compat: support compat mode in REPL #12882
Conversation
|
Ref #12684 as it's somewhat needed to address this problem |
bartlomieju
changed the title
bartlomieju
changed the title
| // FIXME(bartlomieju): this is a hacky way to provide compatibility with REPL | ||
| // and `Deno.core.evalContext` API. Ideally we should always have a referrer filled | ||
| // but sadly that's not the case due to missing APIs in V8. | ||
| let referrer = if referrer.is_empty() && self.flags.repl { | ||
| deno_core::DUMMY_SPECIFIER | ||
| deno_core::resolve_url_or_path("./$deno$repl.ts").unwrap() | ||
| } else { | ||
| referrer | ||
| deno_core::resolve_url_or_path(referrer).unwrap() | ||
| }; |
There was a problem hiding this comment.
I'm not sure if I'm happy about this solution, but it's required because deno_graph::source::Resolver requires &Url type for referrer arg
I kicked can down the road - this still needs to be addressed, but with this PR I think it's a bit more obvious how to tackle this "aggregate resolver" situation. |
| let maybe_resolver: Option<&dyn deno_graph::source::Resolver> = | ||
| if let Some(resolver) = &self.maybe_resolver { | ||
| Some(resolver.as_ref()) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
Longhand version of saying this?
| let maybe_resolver: Option<&dyn deno_graph::source::Resolver> = | |
| if let Some(resolver) = &self.maybe_resolver { | |
| Some(resolver.as_ref()) | |
| } else { | |
| None | |
| }; | |
| let maybe_resolver: Option<&dyn deno_graph::source::Resolver> = | |
| self.maybe_resolver.as_ref(); |
(Or maybe .as_deref(). Likewise around line 661.)
There was a problem hiding this comment.
I tried that before but it doesn't work:
error[E0308]: mismatched types
--> cli/proc_state.rs:429:7
|
428 | let maybe_resolver: Option<&dyn deno_graph::source::Resolver> =
| ----------------------------------------- expected due to this
429 | self.maybe_resolver.as_ref();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected trait object `dyn Resolver`, found struct `Arc`
|
= note: expected enum `std::option::Option<&dyn Resolver>`
found enum `std::option::Option<&Arc<dyn Resolver + std::marker::Send + std::marker::Sync>>`
I had to add additional trait boundaries (Send + Sync) to the dyn Resolver and found that it's the only method to get it past type checking.
| } else { | ||
| None | ||
| }; | ||
| if let Some(resolver) = &maybe_resolver { |
There was a problem hiding this comment.
It's not wholly clear to me why it needs to be converted to &dyn Resolver first.
There was a problem hiding this comment.
I'm not sure what you mean by converting - but we're using dyn Resolver to have a single interface from deno_graph; that can be used in prepare_module_load method and resolve methods on the ProcState.
This commit introduces "ProcState::maybe_resolver" field, which stores a single instance of resolver for the whole lifetime of the process, instead of creating these resolvers for each creation of module graph. As a result, this resolver can be used in fallback case where graph is not constructed (REPL, loading modules using "require") unifying resolution logic.
This commit introduces "ProcState::maybe_resolver" field, which
stores a single instance of resolver for the whole lifetime of the
process, instead of creating these resolvers for each creation
of module graph. As a result, this resolver can be used in fallback
case where graph is not constructed (REPL, loading modules using
"require") unifying resolution logic.
Closes #12628
Closes #12872