fix(compat): cjs/esm interop for dynamic imports (#13792)

This commit fixes CJS/ESM interop in compat mode for dynamically
imported modules.

"ProcState::prepare_module_load" was changed to accept a list
of "graph roots" without associated "module kind". That module kind
was always hardcoded to "ESM" which is not true for CJS/ESM interop -
a CommonJs module might be imported using "import()" function. In
such case the root of the graph should have "CommonJs" module kind
instead of "ESM".

6 files changed, 48 insertions, 11 deletions

diff --git a/cli/main.rs b/cli/main.rs
index 3e49380d9..bb2a0afb3 100644
--- a/cli/main.rs
+++ b/cli/main.rs
@@ -586,7 +586,7 @@ async fn cache_command(
   for file in cache_flags.files {
     let specifier = resolve_url_or_path(&file)?;
     ps.prepare_module_load(
-      vec![(specifier, deno_graph::ModuleKind::Esm)],
+      vec![specifier],
       false,
       lib.clone(),
       Permissions::allow_all(),
diff --git a/cli/module_loader.rs b/cli/module_loader.rs
index b6eb5ca51..afd47c2d4 100644
--- a/cli/module_loader.rs
+++ b/cli/module_loader.rs
@@ -108,7 +108,7 @@ impl ModuleLoader for CliModuleLoader {
 
     async move {
       ps.prepare_module_load(
-        vec![(specifier, deno_graph::ModuleKind::Esm)],
+        vec![specifier],
         is_dynamic,
         lib,
         root_permissions,
diff --git a/cli/proc_state.rs b/cli/proc_state.rs
index 8933cb986..3d5578d32 100644
--- a/cli/proc_state.rs
+++ b/cli/proc_state.rs
@@ -42,6 +42,7 @@ use deno_graph::create_graph;
 use deno_graph::source::CacheInfo;
 use deno_graph::source::LoadFuture;
 use deno_graph::source::Loader;
+use deno_graph::source::ResolveResponse;
 use deno_graph::ModuleKind;
 use deno_graph::Resolved;
 use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel;
@@ -252,13 +253,46 @@ impl ProcState {
   /// emits where necessary or report any module graph / type checking errors.
   pub(crate) async fn prepare_module_load(
     &self,
-    roots: Vec<(ModuleSpecifier, ModuleKind)>,
+    roots: Vec<ModuleSpecifier>,
     is_dynamic: bool,
     lib: emit::TypeLib,
     root_permissions: Permissions,
     dynamic_permissions: Permissions,
     reload_on_watch: bool,
   ) -> Result<(), AnyError> {
+    let maybe_resolver: Option<&dyn deno_graph::source::Resolver> =
+      if let Some(resolver) = &self.maybe_resolver {
+        Some(resolver.as_ref())
+      } else {
+        None
+      };
+
+    // NOTE(@bartlomieju):
+    // Even though `roots` are fully resolved at this point, we are going
+    // to resolve them through `maybe_resolver` to get module kind for the graph
+    // or default to ESM.
+    //
+    // One might argue that this is a code smell, and I would agree. However
+    // due to flux in "Node compatibility" it's not clear where it should be
+    // decided what `ModuleKind` is decided for root specifier.
+    let roots = roots
+      .into_iter()
+      .map(|r| {
+        if let Some(resolver) = &maybe_resolver {
+          let response =
+            resolver.resolve(r.as_str(), &Url::parse("unused:").unwrap());
+          // TODO(bartlomieju): this should be implemented in `deno_graph`
+          match response {
+            ResolveResponse::CommonJs(_) => (r, ModuleKind::CommonJs),
+            ResolveResponse::Err(_) => unreachable!(),
+            _ => (r, ModuleKind::Esm),
+          }
+        } else {
+          (r, ModuleKind::Esm)
+        }
+      })
+      .collect();
+
     // TODO(bartlomieju): this is very make-shift, is there an existing API
     // that we could include it like with "maybe_imports"?
     let roots = if self.flags.compat {
@@ -290,12 +324,6 @@ impl ProcState {
     );
     let maybe_locker = as_maybe_locker(self.lockfile.clone());
     let maybe_imports = self.get_maybe_imports()?;
-    let maybe_resolver: Option<&dyn deno_graph::source::Resolver> =
-      if let Some(resolver) = &self.maybe_resolver {
-        Some(resolver.as_ref())
-      } else {
-        None
-      };
 
     struct ProcStateLoader<'a> {
       inner: &'a mut cache::FetchCacher,
@@ -329,6 +357,7 @@ impl ProcState {
       graph_data: self.graph_data.clone(),
       reload: reload_on_watch,
     };
+
     let graph = create_graph(
       roots.clone(),
       is_dynamic,
diff --git a/cli/tests/integration/compat_tests.rs b/cli/tests/integration/compat_tests.rs
index c8fc1c0a0..5c6a93201 100644
--- a/cli/tests/integration/compat_tests.rs
+++ b/cli/tests/integration/compat_tests.rs
@@ -101,6 +101,12 @@ itest!(cjs_esm_interop {
   output: "compat/import_cjs_from_esm.out",
 });
 
+itest!(cjs_esm_interop_dynamic {
+  args:
+    "run --compat --unstable -A --quiet --no-check compat/import_cjs_from_esm/main_dynamic.mjs",
+  output: "compat/import_cjs_from_esm.out",
+});
+
 #[test]
 fn globals_in_repl() {
   let (out, _err) = util::run_and_collect_output_with_args(
diff --git a/cli/tests/testdata/compat/import_cjs_from_esm/main_dynamic.mjs b/cli/tests/testdata/compat/import_cjs_from_esm/main_dynamic.mjs
new file mode 100644
index 000000000..e94af67a4
--- /dev/null
+++ b/cli/tests/testdata/compat/import_cjs_from_esm/main_dynamic.mjs
@@ -0,0 +1,2 @@
+const url = new URL("./imported.js", import.meta.url);
+await import(url.href);
diff --git a/cli/tools/test.rs b/cli/tools/test.rs
index 61a37c7ff..d3eff1368 100644
--- a/cli/tools/test.rs
+++ b/cli/tools/test.rs
@@ -742,7 +742,7 @@ async fn check_specifiers(
   if !inline_files.is_empty() {
     let specifiers = inline_files
       .iter()
-      .map(|file| (file.specifier.clone(), ModuleKind::Esm))
+      .map(|file| file.specifier.clone())
       .collect();
 
     for file in inline_files {
@@ -764,7 +764,7 @@ async fn check_specifiers(
     .iter()
     .filter_map(|(specifier, mode)| {
       if *mode != TestMode::Documentation {
-        Some((specifier.clone(), ModuleKind::Esm))
+        Some(specifier.clone())
       } else {
         None
       }