Skip to content

Commit

Permalink
compat: support compat mode in REPL (#12882)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bartlomieju authored and piscisaureus committed Nov 24, 2021
1 parent ef06f87 commit dd7d8d2
Show file tree
Hide file tree
Showing 12 changed files with 112 additions and 41 deletions.
2 changes: 1 addition & 1 deletion cli/compat/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub(crate) fn err_module_not_found(
typ: &str,
) -> AnyError {
generic_error(format!(
"[ERR_MODULE_NOT_FOUND] Cannot find {} '{}' imported from {}",
"[ERR_MODULE_NOT_FOUND] Cannot find {} \"{}\" imported from \"{}\"",
typ, path, base
))
}
Expand Down
10 changes: 3 additions & 7 deletions cli/compat/esm_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ impl NodeEsmResolver {
maybe_import_map_resolver,
}
}

pub fn as_resolver(&self) -> &dyn Resolver {
self
}
}

impl Resolver for NodeEsmResolver {
Expand Down Expand Up @@ -232,12 +228,12 @@ fn finalize_resolution(
};
if is_dir {
return Err(errors::err_unsupported_dir_import(
&path.display().to_string(),
&to_file_path_string(base),
resolved.as_str(),
base.as_str(),
));
} else if !is_file {
return Err(errors::err_module_not_found(
&path.display().to_string(),
resolved.as_str(),
base.as_str(),
"module",
));
Expand Down
18 changes: 18 additions & 0 deletions cli/compat/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,24 @@ pub(crate) fn load_cjs_module(
Ok(())
}

pub(crate) fn add_global_require(
js_runtime: &mut JsRuntime,
main_module: &str,
) -> Result<(), AnyError> {
let source_code = &format!(
r#"(async function setupGlobalRequire(main) {{
const Module = await import("{}");
const require = Module.createRequire(main);
globalThis.require = require;
}})('{}');"#,
MODULE_URL_STR.as_str(),
escape_for_single_quote_string(main_module),
);

js_runtime.execute_script(&located_script_name!(), source_code)?;
Ok(())
}

fn escape_for_single_quote_string(text: &str) -> String {
text.replace(r"\", r"\\").replace("'", r"\'")
}
1 change: 1 addition & 0 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,7 @@ async fn run_repl(flags: Flags, repl_flags: ReplFlags) -> Result<(), AnyError> {
create_main_worker(&ps, main_module.clone(), permissions, None);
if flags.compat {
worker.execute_side_module(&compat::GLOBAL_URL).await?;
compat::add_global_require(&mut worker.js_runtime, main_module.as_str())?;
}
worker.run_event_loop(false).await?;

Expand Down
84 changes: 52 additions & 32 deletions cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ pub struct Inner {
pub broadcast_channel: InMemoryBroadcastChannel,
pub shared_array_buffer_store: SharedArrayBufferStore,
pub compiled_wasm_module_store: CompiledWasmModuleStore,
maybe_resolver: Option<Arc<dyn deno_graph::source::Resolver + Send + Sync>>,
}

impl Deref for ProcState {
Expand Down Expand Up @@ -313,6 +314,34 @@ impl ProcState {
.clone()
.or_else(|| env::var("DENO_UNSTABLE_COVERAGE_DIR").ok());

// FIXME(bartlomieju): `NodeEsmResolver` is not aware of JSX resolver
// created below
let node_resolver = NodeEsmResolver::new(
maybe_import_map.clone().map(ImportMapResolver::new),
);
let maybe_import_map_resolver =
maybe_import_map.clone().map(ImportMapResolver::new);
let maybe_jsx_resolver = maybe_config_file
.as_ref()
.map(|cf| {
cf.to_maybe_jsx_import_source_module()
.map(|im| JsxResolver::new(im, maybe_import_map_resolver.clone()))
})
.flatten();
let maybe_resolver: Option<
Arc<dyn deno_graph::source::Resolver + Send + Sync>,
> = if flags.compat {
Some(Arc::new(node_resolver))
} else if let Some(jsx_resolver) = maybe_jsx_resolver {
// the JSX resolver offloads to the import map if present, otherwise uses
// the default Deno explicit import resolution.
Some(Arc::new(jsx_resolver))
} else if let Some(import_map_resolver) = maybe_import_map_resolver {
Some(Arc::new(import_map_resolver))
} else {
None
};

Ok(ProcState(Arc::new(Inner {
dir,
coverage_dir,
Expand All @@ -328,6 +357,7 @@ impl ProcState {
broadcast_channel,
shared_array_buffer_store,
compiled_wasm_module_store,
maybe_resolver,
})))
}

Expand Down Expand Up @@ -395,30 +425,12 @@ impl ProcState {
);
let maybe_locker = as_maybe_locker(self.lockfile.clone());
let maybe_imports = self.get_maybe_imports()?;
let node_resolver = NodeEsmResolver::new(
self.maybe_import_map.clone().map(ImportMapResolver::new),
);
let maybe_import_map_resolver =
self.maybe_import_map.clone().map(ImportMapResolver::new);
let maybe_jsx_resolver = self
.maybe_config_file
.as_ref()
.map(|cf| {
cf.to_maybe_jsx_import_source_module()
.map(|im| JsxResolver::new(im, maybe_import_map_resolver.clone()))
})
.flatten();
let maybe_resolver = if self.flags.compat {
Some(node_resolver.as_resolver())
} else if maybe_jsx_resolver.is_some() {
// the JSX resolver offloads to the import map if present, otherwise uses
// the default Deno explicit import resolution.
maybe_jsx_resolver.as_ref().map(|jr| jr.as_resolver())
} else {
maybe_import_map_resolver
.as_ref()
.map(|im| im.as_resolver())
};
let maybe_resolver: Option<&dyn deno_graph::source::Resolver> =
if let Some(resolver) = &self.maybe_resolver {
Some(resolver.as_ref())
} else {
None
};
let graph = create_graph(
roots.clone(),
is_dynamic,
Expand Down Expand Up @@ -637,18 +649,26 @@ impl ProcState {
}
}

// FIXME(bartlomieju): hacky way to provide compatibility with repl
// 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()
};
if let Some(import_map) = &self.maybe_import_map {
import_map
.resolve(specifier, referrer)
.map_err(|err| err.into())

let maybe_resolver: Option<&dyn deno_graph::source::Resolver> =
if let Some(resolver) = &self.maybe_resolver {
Some(resolver.as_ref())
} else {
None
};
if let Some(resolver) = &maybe_resolver {
resolver.resolve(specifier, &referrer)
} else {
deno_core::resolve_import(specifier, referrer).map_err(|err| err.into())
deno_core::resolve_import(specifier, referrer.as_str())
.map_err(|err| err.into())
}
}

Expand Down
20 changes: 20 additions & 0 deletions cli/tests/integration/compat_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ itest!(compat_dyn_import_rejects_with_node_compatible_error {
envs: vec![("DENO_NODE_COMPAT_URL".to_string(), std_file_url())],
});

itest!(import_esm_from_cjs {
args:
"run --compat --unstable -A --quiet compat/import_esm_from_cjs/index.js",
output_str: Some("function\n"),
});

#[test]
fn globals_in_repl() {
let (out, _err) = util::run_and_collect_output_with_args(
Expand All @@ -58,6 +64,20 @@ fn globals_in_repl() {
assert!(out.contains("true"));
}

#[test]
fn require_in_repl() {
let (out, _err) = util::run_and_collect_output_with_args(
true,
vec!["repl", "--compat", "--unstable", "--quiet"],
Some(vec![
"const foo = require('./compat/import_esm_from_cjs/index');",
]),
None,
false,
);
assert!(out.contains("function"));
}

#[test]
fn node_compat_url() {
let (out, err) = util::run_and_collect_output_with_args(
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/testdata/compat/dyn_import_reject.out
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
TypeError: Cannot load module "file:///[WILDCARD]/testdata/compat/foobar.js".
TypeError: [ERR_MODULE_NOT_FOUND] Cannot find module "file://[WILDCARD]/testdata/compat/foobar.js" imported from "file://[WILDCARD]/testdata/compat/dyn_import_reject.js"
ERR_MODULE_NOT_FOUND
1 change: 1 addition & 0 deletions cli/tests/testdata/compat/import_esm_from_cjs/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require("pure-cjs");

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit dd7d8d2

Please sign in to comment.