refactor(node_resolver): make conditions_from_resolution_mode configurable #27596
Conversation
|
Note change is breaking |
theswerd
changed the title
theswerd
changed the title
…no more memory issues
resolvers/node/resolution.rs
Outdated
| @@ -63,6 +66,30 @@ fn conditions_from_resolution_mode( | |||
| } | |||
| } | |||
|
|
|||
| pub struct ConditionsFromResolutionMode { | |||
There was a problem hiding this comment.
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
changed the title
There was a problem hiding this comment.
After looking at this more, I think we should maybe we should just have everything accept a conditions: &[&str] method, then use resolution_mode.default_conditions() in the CLI code? That way we don't need to box anything and people can supply their own conditions for the specific methods.
By the way, what is the purpose of this change? I don't see it being used anywhere in the CLI.
Happy to make these changes right now. We have our own runtime derived from deno_core + the deno extensions. I want this functionality for it. I also want it in deno itself, but how you choose to configure it is above my pay grade – I'd really like an option like I tried to make this PR minimally, so I can use the configuration how I want as soon as possible in a way that deno can take and run with however they want. |
|
@dsherret made your recommended changes |
…olution_mode' into conditions_from_resolution_mode
|
I've never seen a patch file before this is so cool! I applied it! |
|
Thank you! |
A user was trying to use Hono node server + react-dom, because node server expects
renderToPipeableStream, because thedenoexport maps to browser, it doesn't have that export, instead it hasrenderToReadableStream. I want to be able to configurably choose module priority based on if users are more leaning into deno or node.