From a3a4760831e14307a9499d4e410cf1653b416dc1 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 12 Sep 2022 14:28:51 -0400 Subject: [PATCH] fix(npm): align Deno importing Node cjs with Node esm importing cjs (#15879) --- cli/module_loader.rs | 32 ++---------- cli/node/mod.rs | 51 ++----------------- .../npm/cjs_reexport_collision/main.ts | 2 +- .../npm/esm_import_cjs_default/main.js | 2 +- .../npm/esm_import_cjs_default/main.out | 4 +- 5 files changed, 12 insertions(+), 79 deletions(-) diff --git a/cli/module_loader.rs b/cli/module_loader.rs index e280a8a3a0..d3293c9574 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -4,7 +4,6 @@ use crate::emit::emit_parsed_source; use crate::emit::TsTypeLib; use crate::graph_util::ModuleEntry; use crate::node; -use crate::node::CjsToEsmTranslateKind; use crate::npm::NpmPackageResolver; use crate::proc_state::ProcState; use crate::text_encoding::code_without_source_map; @@ -24,7 +23,6 @@ use deno_core::ModuleType; use deno_core::OpState; use deno_core::SourceMapGetter; use deno_runtime::permissions::Permissions; -use std::borrow::Cow; use std::cell::RefCell; use std::pin::Pin; use std::rc::Rc; @@ -136,28 +134,7 @@ impl CliModuleLoader { maybe_referrer: Option, ) -> Result { let code_source = if self.ps.npm_resolver.in_npm_package(specifier) { - let is_cjs = self.ps.cjs_resolutions.lock().contains(specifier); - let (maybe_translate_kind, load_specifier) = if is_cjs { - let path = specifier.path(); - let mut specifier = specifier.clone(); - if let Some(new_path) = path.strip_suffix(node::CJS_TO_ESM_NODE_SUFFIX) - { - specifier.set_path(new_path); - (Some(CjsToEsmTranslateKind::Node), Cow::Owned(specifier)) - } else if let Some(new_path) = - path.strip_suffix(node::CJS_TO_ESM_DENO_SUFFIX) - { - specifier.set_path(new_path); - (Some(CjsToEsmTranslateKind::Deno), Cow::Owned(specifier)) - } else { - // all cjs code that goes through the loader should have been given a suffix - unreachable!("Unknown cjs specifier: {}", specifier); - } - } else { - (None, Cow::Borrowed(specifier)) - }; - - let file_path = load_specifier.to_file_path().unwrap(); + let file_path = specifier.to_file_path().unwrap(); let code = std::fs::read_to_string(&file_path).with_context(|| { let mut msg = "Unable to load ".to_string(); msg.push_str(&*file_path.to_string_lossy()); @@ -168,19 +145,18 @@ impl CliModuleLoader { msg })?; - let code = if let Some(translate_kind) = maybe_translate_kind { + let code = if self.ps.cjs_resolutions.lock().contains(specifier) { // translate cjs to esm if it's cjs and inject node globals node::translate_cjs_to_esm( &self.ps.file_fetcher, - &load_specifier, + specifier, code, MediaType::Cjs, &self.ps.npm_resolver, - translate_kind, )? } else { // only inject node globals for esm - node::esm_code_with_node_globals(&load_specifier, code)? + node::esm_code_with_node_globals(specifier, code)? }; ModuleCodeSource { code, diff --git a/cli/node/mod.rs b/cli/node/mod.rs index 81a2f10e79..75fdfcd7d5 100644 --- a/cli/node/mod.rs +++ b/cli/node/mod.rs @@ -255,11 +255,6 @@ static NODE_COMPAT_URL: Lazy = Lazy::new(|| { pub static MODULE_ALL_URL: Lazy = Lazy::new(|| NODE_COMPAT_URL.join("node/module_all.ts").unwrap()); -/// Suffix used for the CJS to ESM translation for Node code -pub const CJS_TO_ESM_NODE_SUFFIX: &str = ".node_cjs_to_esm.mjs"; -/// Suffix used for the CJS to ESM translation for Deno code -pub const CJS_TO_ESM_DENO_SUFFIX: &str = ".deno_cjs_to_esm.mjs"; - fn find_builtin_node_module(specifier: &str) -> Option<&NodeModulePolyfill> { SUPPORTED_MODULES.iter().find(|m| m.name == specifier) } @@ -417,15 +412,7 @@ pub fn node_resolve( None => return Ok(None), }; - let resolve_response = match url_to_node_resolution(url, npm_resolver)? { - NodeResolution::CommonJs(mut url) => { - // Use a suffix to say this cjs specifier should be translated from - // cjs to esm for consumption by Node ESM code - url.set_path(&format!("{}{}", url.path(), CJS_TO_ESM_NODE_SUFFIX)); - NodeResolution::CommonJs(url) - } - val => val, - }; + let resolve_response = url_to_node_resolution(url, npm_resolver)?; // TODO(bartlomieju): skipped checking errors for commonJS resolution and // "preserveSymlinksMain"/"preserveSymlinks" options. Ok(Some(resolve_response)) @@ -453,15 +440,7 @@ pub fn node_resolve_npm_reference( })?; let url = ModuleSpecifier::from_file_path(resolved_path).unwrap(); - let resolve_response = match url_to_node_resolution(url, npm_resolver)? { - NodeResolution::CommonJs(mut url) => { - // Use a suffix to say this cjs specifier should be translated from - // cjs to esm for consumption by Deno ESM code - url.set_path(&format!("{}{}", url.path(), CJS_TO_ESM_DENO_SUFFIX)); - NodeResolution::CommonJs(url) - } - val => val, - }; + let resolve_response = url_to_node_resolution(url, npm_resolver)?; // TODO(bartlomieju): skipped checking errors for commonJS resolution and // "preserveSymlinksMain"/"preserveSymlinks" options. Ok(Some(resolve_response)) @@ -722,12 +701,6 @@ fn add_export( } } -#[derive(Clone, Copy, PartialEq, Eq)] -pub enum CjsToEsmTranslateKind { - Node, - Deno, -} - /// Translates given CJS module into ESM. This function will perform static /// analysis on the file to find defined exports and reexports. /// @@ -740,7 +713,6 @@ pub fn translate_cjs_to_esm( code: String, media_type: MediaType, npm_resolver: &GlobalNpmPackageResolver, - translate_kind: CjsToEsmTranslateKind, ) -> Result { fn perform_cjs_analysis( specifier: &str, @@ -767,11 +739,6 @@ pub fn translate_cjs_to_esm( let analysis = perform_cjs_analysis(specifier.as_str(), media_type, code)?; - let root_exports = analysis - .exports - .iter() - .map(|s| s.as_str()) - .collect::>(); let mut all_exports = analysis .exports .iter() @@ -837,16 +804,8 @@ pub fn translate_cjs_to_esm( .replace('\"', "\\\"") )); - let mut had_default = false; for export in &all_exports { - if export.as_str() == "default" { - if translate_kind == CjsToEsmTranslateKind::Deno - && root_exports.contains("__esModule") - { - source.push(format!("export default mod[\"{}\"];", export)); - had_default = true; - } - } else { + if export.as_str() != "default" { add_export( &mut source, export, @@ -856,9 +815,7 @@ pub fn translate_cjs_to_esm( } } - if !had_default { - source.push("export default mod;".to_string()); - } + source.push("export default mod;".to_string()); let translated_source = source.join("\n"); Ok(translated_source) diff --git a/cli/tests/testdata/npm/cjs_reexport_collision/main.ts b/cli/tests/testdata/npm/cjs_reexport_collision/main.ts index 51d9a69e3c..4bfcd89b15 100644 --- a/cli/tests/testdata/npm/cjs_reexport_collision/main.ts +++ b/cli/tests/testdata/npm/cjs_reexport_collision/main.ts @@ -1,2 +1,2 @@ import ReExportCollision from "npm:@denotest/cjs-reexport-collision"; -ReExportCollision.sayHello(); +ReExportCollision.default.sayHello(); diff --git a/cli/tests/testdata/npm/esm_import_cjs_default/main.js b/cli/tests/testdata/npm/esm_import_cjs_default/main.js index 3be3cac5e7..f405a58999 100644 --- a/cli/tests/testdata/npm/esm_import_cjs_default/main.js +++ b/cli/tests/testdata/npm/esm_import_cjs_default/main.js @@ -17,6 +17,6 @@ console.log(esmDefault); console.log(esmNamespace); console.log("==========================="); -console.log(cjsDefault()); +console.log(cjsDefault.default()); console.log(esmDefault()); console.log(MyCjsClass.someStaticMethod()); diff --git a/cli/tests/testdata/npm/esm_import_cjs_default/main.out b/cli/tests/testdata/npm/esm_import_cjs_default/main.out index 3b2b3b006a..4b73328f33 100644 --- a/cli/tests/testdata/npm/esm_import_cjs_default/main.out +++ b/cli/tests/testdata/npm/esm_import_cjs_default/main.out @@ -17,11 +17,11 @@ Module { static method Deno esm importing node cjs =========================== -[Function] +{ default: [Function], named: [Function], MyClass: [Function: MyClass] } Module { MyClass: [Function: MyClass], __esModule: true, - default: [Function], + default: { default: [Function], named: [Function], MyClass: [Function: MyClass] }, named: [Function] } ===========================