Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(node_resolver): make conditions_from_resolution_mode configurable #27596

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

theswerd
Copy link
Contributor

@theswerd theswerd commented Jan 9, 2025

A user was trying to use Hono node server + react-dom, because node server expects renderToPipeableStream, because the deno export maps to browser, it doesn't have that export, instead it has renderToReadableStream. I want to be able to configurably choose module priority based on if users are more leaning into deno or node.

@theswerd theswerd marked this pull request as draft January 9, 2025 06:27
@theswerd
Copy link
Contributor Author

theswerd commented Jan 9, 2025

Note change is breaking

@theswerd theswerd marked this pull request as ready for review January 9, 2025 06:33
@theswerd theswerd changed the title Feat Made conditions_from_resolution_mode work feat Made conditions_from_resolution_mode work Jan 9, 2025
@theswerd theswerd changed the title feat Made conditions_from_resolution_mode work feat Made conditions_from_resolution_mode configurable Jan 9, 2025
@theswerd theswerd marked this pull request as draft January 9, 2025 06:51
@theswerd theswerd marked this pull request as ready for review January 9, 2025 07:00
@@ -63,6 +66,30 @@ fn conditions_from_resolution_mode(
}
}

pub struct ConditionsFromResolutionMode {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should just be: pub struct ConditionsFromResolutionMode(Option<Box<dyn Fn(ResolutionMode) -> &'static [&'static str]>>) with a default implementation of ConditionsFromResolutionMode(None) that will instead use deno_conditions_from_resolution_mode? That way the other code can just do ConditionsFromResolutionMode::default() instead of needing to know about deno_conditions_from_resolution_mode

@dsherret dsherret changed the title feat Made conditions_from_resolution_mode configurable refactor(node_resolver): make conditions_from_resolution_mode configurable Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants