From 65307fc0f2a06ddd7736e88f2ce68fcf0b5e9a85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Nov 2021 01:20:36 +0100 Subject: [PATCH 1/9] compat: support compat mode in REPL --- cli/proc_state.rs | 57 ++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/cli/proc_state.rs b/cli/proc_state.rs index c8e3968d202f88..1889898d747675 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -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>, } impl Deref for ProcState { @@ -313,6 +314,32 @@ 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> = 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, @@ -328,6 +355,7 @@ impl ProcState { broadcast_channel, shared_array_buffer_store, compiled_wasm_module_store, + maybe_resolver, }))) } @@ -395,29 +423,10 @@ 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()) + let maybe_resolver = if let Some(resolver) = self.maybe_resolver { + Some(resolver.as_ref()) } else { - maybe_import_map_resolver - .as_ref() - .map(|im| im.as_resolver()) + None }; let graph = create_graph( roots.clone(), @@ -637,7 +646,9 @@ 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 } else { From 53818d8ab0d7350e2474df7b3a6146c50aa59af9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Nov 2021 01:48:21 +0100 Subject: [PATCH 2/9] fmt --- cli/proc_state.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 1889898d747675..7c77c52f7ba6db 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -328,7 +328,9 @@ impl ProcState { .map(|im| JsxResolver::new(im, maybe_import_map_resolver.clone())) }) .flatten(); - let maybe_resolver: Option> = if flags.compat { + let maybe_resolver: Option< + Arc, + > = 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 From da7db0c53369ba7d5275caff298b8aa6eb0aaa21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Nov 2021 03:13:56 +0100 Subject: [PATCH 3/9] use ProcState::maybe_resolver in fallback --- cli/compat/esm_resolver.rs | 4 ---- cli/proc_state.rs | 31 ++++++++++++++++++++----------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index 980e44b32b658d..b8776863f0635f 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -24,10 +24,6 @@ impl NodeEsmResolver { maybe_import_map_resolver, } } - - pub fn as_resolver(&self) -> &dyn Resolver { - self - } } impl Resolver for NodeEsmResolver { diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 7c77c52f7ba6db..9968bc3bd1bbf4 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -425,11 +425,12 @@ impl ProcState { ); let maybe_locker = as_maybe_locker(self.lockfile.clone()); let maybe_imports = self.get_maybe_imports()?; - let maybe_resolver = if let Some(resolver) = self.maybe_resolver { - Some(resolver.as_ref()) - } else { - None - }; + 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, @@ -652,16 +653,24 @@ impl ProcState { // 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) + + 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) .map_err(|err| err.into()) } 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()) } } From 4680c0a7d559658e296f0e4f57b74fc56ce261b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Nov 2021 03:22:48 +0100 Subject: [PATCH 4/9] add global 'require' in REPL --- cli/compat/mod.rs | 18 ++++++++++++++++++ cli/main.rs | 1 + cli/proc_state.rs | 4 +--- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 01d84d65a5a93d..41443968f8b703 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -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"\'") } diff --git a/cli/main.rs b/cli/main.rs index 33895ec750fbb9..87fe28461163e3 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -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?; diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 9968bc3bd1bbf4..7615b9d4641e82 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -665,9 +665,7 @@ impl ProcState { None }; if let Some(resolver) = &maybe_resolver { - resolver - .resolve(specifier, &referrer) - .map_err(|err| err.into()) + resolver.resolve(specifier, &referrer) } else { deno_core::resolve_import(specifier, referrer.as_str()) .map_err(|err| err.into()) From 4f581d120002791107688f127c7fb7a6e3583a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Nov 2021 03:45:21 +0100 Subject: [PATCH 5/9] add tests --- cli/tests/integration/compat_tests.rs | 22 ++++++++++++++++++- .../testdata/compat/dyn_import_reject.out | 2 +- .../compat/import_esm_from_cjs/index.js | 1 + .../node_modules/pure-cjs/index.js | 4 ++++ .../node_modules/pure-cjs/package.json | 4 ++++ .../node_modules/pure-esm/index.js | 2 ++ .../node_modules/pure-esm/package.json | 5 +++++ 7 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 cli/tests/testdata/compat/import_esm_from_cjs/index.js create mode 100644 cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-cjs/index.js create mode 100644 cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-cjs/package.json create mode 100644 cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-esm/index.js create mode 100644 cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-esm/package.json diff --git a/cli/tests/integration/compat_tests.rs b/cli/tests/integration/compat_tests.rs index e15a19b8a31ae4..09bf06710a895d 100644 --- a/cli/tests/integration/compat_tests.rs +++ b/cli/tests/integration/compat_tests.rs @@ -28,6 +28,12 @@ itest!(compat_dyn_import_rejects_with_node_compatible_error { output: "compat/dyn_import_reject.out", }); +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( @@ -40,6 +46,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( @@ -54,5 +74,5 @@ fn node_compat_url() { ); assert!(out.is_empty()); assert!(!err.is_empty()); - assert!(err.contains("file:///non_existent/node/global.ts")); + assert!(err.contains("non_existent/node/global.ts")); } diff --git a/cli/tests/testdata/compat/dyn_import_reject.out b/cli/tests/testdata/compat/dyn_import_reject.out index 6d78135b27f9c8..27fc7b2f11747e 100644 --- a/cli/tests/testdata/compat/dyn_import_reject.out +++ b/cli/tests/testdata/compat/dyn_import_reject.out @@ -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 diff --git a/cli/tests/testdata/compat/import_esm_from_cjs/index.js b/cli/tests/testdata/compat/import_esm_from_cjs/index.js new file mode 100644 index 00000000000000..4ba03e1045128e --- /dev/null +++ b/cli/tests/testdata/compat/import_esm_from_cjs/index.js @@ -0,0 +1 @@ +require("pure-cjs"); diff --git a/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-cjs/index.js b/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-cjs/index.js new file mode 100644 index 00000000000000..35f7c37744d4a4 --- /dev/null +++ b/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-cjs/index.js @@ -0,0 +1,4 @@ +async function run() { + const _result = await import('pure-esm'); +} +run() diff --git a/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-cjs/package.json b/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-cjs/package.json new file mode 100644 index 00000000000000..e854fd992679aa --- /dev/null +++ b/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-cjs/package.json @@ -0,0 +1,4 @@ +{ + "name": "pure-cjs", + "main": "./index.js" +} diff --git a/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-esm/index.js b/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-esm/index.js new file mode 100644 index 00000000000000..898097cb5a7bf7 --- /dev/null +++ b/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-esm/index.js @@ -0,0 +1,2 @@ +import fs from 'node:fs'; +console.log(typeof fs.chmod); diff --git a/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-esm/package.json b/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-esm/package.json new file mode 100644 index 00000000000000..a373d3ad986d21 --- /dev/null +++ b/cli/tests/testdata/compat/import_esm_from_cjs/node_modules/pure-esm/package.json @@ -0,0 +1,5 @@ +{ + "name": "pure-esm", + "type": "module", + "main": "./index.js" +} From 34d9ae514e941678719f18e80c57a0cd590b90e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Nov 2021 04:15:39 +0100 Subject: [PATCH 6/9] update out file --- cli/tests/testdata/compat/dyn_import_reject.out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tests/testdata/compat/dyn_import_reject.out b/cli/tests/testdata/compat/dyn_import_reject.out index 27fc7b2f11747e..63acd4a7819764 100644 --- a/cli/tests/testdata/compat/dyn_import_reject.out +++ b/cli/tests/testdata/compat/dyn_import_reject.out @@ -1,2 +1,2 @@ -TypeError: [ERR_MODULE_NOT_FOUND] Cannot find module 'file:///[WILDCARD]/testdata/compat/foobar.js' imported from file:///[WILDCARD]/testdata/compat/dyn_import_reject.js +TypeError: [ERR_MODULE_NOT_FOUND] Cannot find module '[WILDCARD]/testdata/compat/foobar.js' imported from file:///[WILDCARD]/testdata/compat/dyn_import_reject.js ERR_MODULE_NOT_FOUND From 086b42de112985e7773380d2c57a4a5dd067af90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Nov 2021 04:32:35 +0100 Subject: [PATCH 7/9] reset CI From 1c6fd04cfc0da395e683e3a930c391a5dfc9c28f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Nov 2021 04:48:21 +0100 Subject: [PATCH 8/9] reset CI From a08995293368fb93cc15cab80e0bd795c59d294a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 24 Nov 2021 12:05:54 +0100 Subject: [PATCH 9/9] unify error output --- cli/compat/errors.rs | 2 +- cli/compat/esm_resolver.rs | 6 +++--- cli/tests/integration/compat_tests.rs | 2 +- cli/tests/testdata/compat/dyn_import_reject.out | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/compat/errors.rs b/cli/compat/errors.rs index 3a44b2c88fdc61..e9cc31a0db1a2d 100644 --- a/cli/compat/errors.rs +++ b/cli/compat/errors.rs @@ -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 )) } diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index b8776863f0635f..5787c9e4857d17 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -228,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", )); diff --git a/cli/tests/integration/compat_tests.rs b/cli/tests/integration/compat_tests.rs index 09bf06710a895d..7888822b0e07de 100644 --- a/cli/tests/integration/compat_tests.rs +++ b/cli/tests/integration/compat_tests.rs @@ -74,5 +74,5 @@ fn node_compat_url() { ); assert!(out.is_empty()); assert!(!err.is_empty()); - assert!(err.contains("non_existent/node/global.ts")); + assert!(err.contains("file:///non_existent/node/global.ts")); } diff --git a/cli/tests/testdata/compat/dyn_import_reject.out b/cli/tests/testdata/compat/dyn_import_reject.out index 63acd4a7819764..1324c8aeb229f8 100644 --- a/cli/tests/testdata/compat/dyn_import_reject.out +++ b/cli/tests/testdata/compat/dyn_import_reject.out @@ -1,2 +1,2 @@ -TypeError: [ERR_MODULE_NOT_FOUND] Cannot find module '[WILDCARD]/testdata/compat/foobar.js' imported from file:///[WILDCARD]/testdata/compat/dyn_import_reject.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