feat(node): load ES modules defined as CJS (#22945)

Changes the behaviour in Deno to just always load ES modules in npm
packages even if they're defined as CJS.

Closes #22818

cli/cache/node.rs

@@ -1,10 +1,11 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-use deno_ast::CjsAnalysis;
 use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_runtime::deno_webstorage::rusqlite::params;
 
+use crate::node::CliCjsAnalysis;
+
 use super::cache_db::CacheDB;
 use super::cache_db::CacheDBConfiguration;
 use super::cache_db::CacheFailure;
@@ -59,7 +60,7 @@ impl NodeAnalysisCache {
     &self,
     specifier: &str,
     expected_source_hash: &str,
-  ) -> Option<CjsAnalysis> {
+  ) -> Option<CliCjsAnalysis> {
     Self::ensure_ok(
       self.inner.get_cjs_analysis(specifier, expected_source_hash),
     )
@@ -69,7 +70,7 @@ impl NodeAnalysisCache {
     &self,
     specifier: &str,
     source_hash: &str,
-    cjs_analysis: &CjsAnalysis,
+    cjs_analysis: &CliCjsAnalysis,
   ) {
     Self::ensure_ok(self.inner.set_cjs_analysis(
       specifier,
@@ -93,7 +94,7 @@ impl NodeAnalysisCacheInner {
     &self,
     specifier: &str,
     expected_source_hash: &str,
-  ) -> Result<Option<CjsAnalysis>, AnyError> {
+  ) -> Result<Option<CliCjsAnalysis>, AnyError> {
     let query = "
       SELECT
         data
@@ -118,7 +119,7 @@ impl NodeAnalysisCacheInner {
     &self,
     specifier: &str,
     source_hash: &str,
-    cjs_analysis: &CjsAnalysis,
+    cjs_analysis: &CliCjsAnalysis,
   ) -> Result<(), AnyError> {
     let sql = "
       INSERT OR REPLACE INTO
@@ -147,7 +148,7 @@ mod test {
     let cache = NodeAnalysisCacheInner::new(conn);
 
     assert!(cache.get_cjs_analysis("file.js", "2").unwrap().is_none());
-    let cjs_analysis = CjsAnalysis {
+    let cjs_analysis = CliCjsAnalysis::Cjs {
       exports: vec!["export1".to_string()],
       reexports: vec!["re-export1".to_string()],
     };
@@ -157,8 +158,7 @@ mod test {
     assert!(cache.get_cjs_analysis("file.js", "3").unwrap().is_none()); // different hash
     let actual_cjs_analysis =
       cache.get_cjs_analysis("file.js", "2").unwrap().unwrap();
-    assert_eq!(actual_cjs_analysis.exports, cjs_analysis.exports);
-    assert_eq!(actual_cjs_analysis.reexports, cjs_analysis.reexports);
+    assert_eq!(actual_cjs_analysis, cjs_analysis);
 
     // adding when already exists should not cause issue
     cache
@@ -170,8 +170,7 @@ mod test {
     let cache = NodeAnalysisCacheInner::new(conn);
     let actual_analysis =
       cache.get_cjs_analysis("file.js", "2").unwrap().unwrap();
-    assert_eq!(actual_analysis.exports, cjs_analysis.exports);
-    assert_eq!(actual_analysis.reexports, cjs_analysis.reexports);
+    assert_eq!(actual_analysis, cjs_analysis);
 
     // now changing the cli version should clear it
     let conn = cache.conn.recreate_with_version("2.0.0");

cli/node.rs

@@ -1,15 +1,15 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-use std::borrow::Cow;
-
-use deno_ast::CjsAnalysis;
 use deno_ast::MediaType;
 use deno_ast::ModuleSpecifier;
 use deno_core::error::AnyError;
 use deno_runtime::deno_fs;
 use deno_runtime::deno_node::analyze::CjsAnalysis as ExtNodeCjsAnalysis;
+use deno_runtime::deno_node::analyze::CjsAnalysisExports;
 use deno_runtime::deno_node::analyze::CjsCodeAnalyzer;
 use deno_runtime::deno_node::analyze::NodeCodeTranslator;
+use serde::Deserialize;
+use serde::Serialize;
 
 use crate::cache::NodeAnalysisCache;
 use crate::util::fs::canonicalize_path_maybe_not_exists;
@@ -35,6 +35,17 @@ pub fn resolve_specifier_into_node_modules(
     .unwrap_or_else(|| specifier.clone())
 }
 
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub enum CliCjsAnalysis {
+  /// The module was found to be an ES module.
+  Esm,
+  /// The module was CJS.
+  Cjs {
+    exports: Vec<String>,
+    reexports: Vec<String>,
+  },
+}
+
 pub struct CliCjsCodeAnalyzer {
   cache: NodeAnalysisCache,
   fs: deno_fs::FileSystemRc,
@@ -49,7 +60,7 @@ impl CliCjsCodeAnalyzer {
     &self,
     specifier: &ModuleSpecifier,
     source: &str,
-  ) -> Result<CjsAnalysis, AnyError> {
+  ) -> Result<CliCjsAnalysis, AnyError> {
     let source_hash = NodeAnalysisCache::compute_source_hash(source);
     if let Some(analysis) = self
       .cache
@@ -60,13 +71,13 @@ impl CliCjsCodeAnalyzer {
 
     let media_type = MediaType::from_specifier(specifier);
     if media_type == MediaType::Json {
-      return Ok(CjsAnalysis {
+      return Ok(CliCjsAnalysis::Cjs {
         exports: vec![],
         reexports: vec![],
       });
     }
 
-    let parsed_source = deno_ast::parse_script(deno_ast::ParseParams {
+    let parsed_source = deno_ast::parse_program(deno_ast::ParseParams {
       specifier: specifier.clone(),
       text_info: deno_ast::SourceTextInfo::new(source.into()),
       media_type,
@@ -74,7 +85,15 @@ impl CliCjsCodeAnalyzer {
       scope_analysis: false,
       maybe_syntax: None,
     })?;
-    let analysis = parsed_source.analyze_cjs();
+    let analysis = if parsed_source.is_script() {
+      let analysis = parsed_source.analyze_cjs();
+      CliCjsAnalysis::Cjs {
+        exports: analysis.exports,
+        reexports: analysis.reexports,
+      }
+    } else {
+      CliCjsAnalysis::Esm
+    };
     self
       .cache
       .set_cjs_analysis(specifier.as_str(), &source_hash, &analysis);
@@ -87,20 +106,23 @@ impl CjsCodeAnalyzer for CliCjsCodeAnalyzer {
   fn analyze_cjs(
     &self,
     specifier: &ModuleSpecifier,
-    source: Option<&str>,
+    source: Option<String>,
   ) -> Result<ExtNodeCjsAnalysis, AnyError> {
     let source = match source {
-      Some(source) => Cow::Borrowed(source),
-      None => Cow::Owned(
-        self
-          .fs
-          .read_text_file_sync(&specifier.to_file_path().unwrap())?,
-      ),
+      Some(source) => source,
+      None => self
+        .fs
+        .read_text_file_sync(&specifier.to_file_path().unwrap())?,
     };
     let analysis = self.inner_cjs_analysis(specifier, &source)?;
-    Ok(ExtNodeCjsAnalysis {
-      exports: analysis.exports,
-      reexports: analysis.reexports,
-    })
+    match analysis {
+      CliCjsAnalysis::Esm => Ok(ExtNodeCjsAnalysis::Esm(source)),
+      CliCjsAnalysis::Cjs { exports, reexports } => {
+        Ok(ExtNodeCjsAnalysis::Cjs(CjsAnalysisExports {
+          exports,
+          reexports,
+        }))
+      }
+    }
   }
 }

cli/resolver.rs

@@ -334,7 +334,7 @@ impl NpmModuleLoader {
       // translate cjs to esm if it's cjs and inject node globals
       self.node_code_translator.translate_cjs_to_esm(
         specifier,
-        Some(code.as_str()),
+        Some(code),
         permissions,
       )?
     } else {

ext/node/analyze.rs

@@ -5,6 +5,7 @@ use std::collections::VecDeque;
 use std::path::Path;
 use std::path::PathBuf;
 
+use deno_core::anyhow;
 use deno_core::anyhow::Context;
 use deno_core::ModuleSpecifier;
 use once_cell::sync::Lazy;
@@ -21,7 +22,15 @@ use crate::PackageJson;
 use crate::PathClean;
 
 #[derive(Debug, Clone)]
-pub struct CjsAnalysis {
+pub enum CjsAnalysis {
+  /// File was found to be an ES module and the translator should
+  /// load the code as ESM.
+  Esm(String),
+  Cjs(CjsAnalysisExports),
+}
+
+#[derive(Debug, Clone)]
+pub struct CjsAnalysisExports {
   pub exports: Vec<String>,
   pub reexports: Vec<String>,
 }
@@ -38,7 +47,7 @@ pub trait CjsCodeAnalyzer {
   fn analyze_cjs(
     &self,
     specifier: &ModuleSpecifier,
-    maybe_source: Option<&str>,
+    maybe_source: Option<String>,
   ) -> Result<CjsAnalysis, AnyError>;
 }
 
@@ -73,7 +82,7 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
   pub fn translate_cjs_to_esm(
     &self,
     specifier: &ModuleSpecifier,
-    source: Option<&str>,
+    source: Option<String>,
     permissions: &dyn NodePermissions,
   ) -> Result<String, AnyError> {
     let mut temp_var_count = 0;
@@ -81,6 +90,11 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
 
     let analysis = self.cjs_code_analyzer.analyze_cjs(specifier, source)?;
 
+    let analysis = match analysis {
+      CjsAnalysis::Esm(source) => return Ok(source),
+      CjsAnalysis::Cjs(analysis) => analysis,
+    };
+
     let mut source = vec![
       r#"import {createRequire as __internalCreateRequire} from "node:module";
       const require = __internalCreateRequire(import.meta.url);"#
@@ -127,6 +141,17 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
             reexport, reexport_specifier, referrer
           )
         })?;
+      let analysis = match analysis {
+        CjsAnalysis::Esm(_) => {
+          // todo(dsherret): support this once supporting requiring ES modules
+          return Err(anyhow::anyhow!(
+            "Cannot require ES module '{}' from '{}'",
+            reexport_specifier,
+            specifier
+          ));
+        }
+        CjsAnalysis::Cjs(analysis) => analysis,
+      };
 
       for reexport in analysis.reexports {
         reexports_to_handle.push_back((reexport, reexport_specifier.clone()));

ext/node/package_json.rs

@@ -93,10 +93,6 @@ impl PackageJson {
       ),
     };
 
-    if source.trim().is_empty() {
-      return Ok(PackageJson::empty(path));
-    }
-
     let package_json = Self::load_from_string(path, source)?;
     CACHE.with(|cache| {
       cache
@@ -110,6 +106,10 @@ impl PackageJson {
     path: PathBuf,
     source: String,
   ) -> Result<PackageJson, AnyError> {
+    if source.trim().is_empty() {
+      return Ok(PackageJson::empty(path));
+    }
+
     let package_json: Value = serde_json::from_str(&source).map_err(|err| {
       anyhow::anyhow!(
         "malformed package.json: {}\n    at {}",

tests/specs/node/detect_es_module_defined_as_cjs/__test__.jsonc

@@ -0,0 +1,10 @@
+{
+  "steps": [{
+    "args": "run main.ts",
+    "output": "main.out"
+  }, {
+    // ensure it works with cache
+    "args": "run main.ts",
+    "output": "main.out"
+  }]
+}

tests/specs/node/detect_es_module_defined_as_cjs/deno.json

@@ -0,0 +1,5 @@
+{
+  "unstable": [
+    "byonm"
+  ]
+}

tests/specs/node/detect_es_module_defined_as_cjs/main.out

@@ -0,0 +1 @@
+3

tests/specs/node/detect_es_module_defined_as_cjs/main.ts

@@ -0,0 +1,3 @@
+import { add } from "package";
+
+console.log(add(1, 2));

tests/specs/node/detect_es_module_defined_as_cjs/node_modules/package/index.js

@@ -0,0 +1,3 @@
+export function add(a, b) {
+  return a + b;
+}

tests/specs/node/detect_es_module_defined_as_cjs/node_modules/package/package.json

@@ -0,0 +1,3 @@
+{
+  "name": "package"
+}