feat(cli): better error message when failed to resolve non explicit CJS module from ESM

cli/module_loader.rs

@@ -815,9 +815,15 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
     }
 
     let referrer = self.0.resolve_referrer(referrer)?;
-    let specifier = self.0.inner_resolve(specifier, &referrer)?;
-    ensure_not_jsr_non_jsr_remote_import(&specifier, &referrer)?;
-    Ok(specifier)
+    let resolved_specifier = self.0.inner_resolve(specifier, &referrer)?;
+    // set the raw specifier so that we can use it later
+    self
+      .0
+      .shared
+      .cjs_tracker
+      .set_raw_speficier(&resolved_specifier, specifier);
+    ensure_not_jsr_non_jsr_remote_import(&resolved_specifier, &referrer)?;
+    Ok(resolved_specifier)
   }
 
   fn get_host_defined_options<'s>(

cli/resolver.rs

@@ -1,6 +1,7 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
 use async_trait::async_trait;
+use dashmap::mapref::one::Ref;
 use dashmap::DashMap;
 use dashmap::DashSet;
 use deno_ast::MediaType;
@@ -382,6 +383,49 @@ impl NpmModuleLoader {
           if let Some(referrer) = &maybe_referrer {
             msg.push_str(" imported from ");
             msg.push_str(referrer.as_str());
+
+            // TODO(Hajime-san): Use `with_added_extension` when it becomes stable.
+            //
+            // This extended implementation for `Path` defines an ad-hoc method with the same name,
+            // since `with_added_extension` is currently only available in the nightly version.
+            // This implementation should be replaced when it becomes stable.
+            // https://github.com/rust-lang/rust/issues/127292
+            trait PathExt {
+              fn _with_added_extension(&self, extension: &str) -> PathBuf;
+            }
+
+            impl PathExt for Path {
+              fn _with_added_extension(&self, extension: &str) -> PathBuf {
+                let mut path = self.to_path_buf();
+
+                let new_extension = match self.extension() {
+                  Some(ext) => {
+                    format!("{}.{}", ext.to_string_lossy(), extension)
+                  }
+                  None => extension.to_string(),
+                };
+
+                path.set_extension(new_extension);
+                path
+              }
+            }
+
+            // A package may have CommonJS modules that are not all listed in the package.json exports.
+            // In this case, it cannot be statically resolved when imported from ESM unless you include the extension of the target file.
+            // So provide the user with a helpful error message.
+            let extension = ["js", "cjs"]
+              .iter()
+              .find(|e| file_path._with_added_extension(e).is_file());
+            let raw_specifier = self.cjs_tracker.get_raw_speficier(specifier);
+            if let (Some(raw_specifier), Some(extension)) =
+              (raw_specifier, extension)
+            {
+              msg.push_str("\nDid you mean to import \"");
+              let suggested_specifier = Path::new(raw_specifier.as_str())
+                ._with_added_extension(extension);
+              msg.push_str(&suggested_specifier.to_string_lossy());
+              msg.push_str("\"?");
+            }
           }
           msg
         }
@@ -434,6 +478,8 @@ pub struct CjsTracker {
   pkg_json_resolver: Arc<PackageJsonResolver>,
   unstable_detect_cjs: bool,
   known: DashMap<ModuleSpecifier, ModuleKind>,
+  /// This field is used to store the raw specifier string due to suggest a helpful error message.
+  raw_specifier: DashMap<ModuleSpecifier, String>,
 }
 
 impl CjsTracker {
@@ -447,6 +493,7 @@ impl CjsTracker {
       pkg_json_resolver,
       unstable_detect_cjs: options.unstable_detect_cjs,
       known: Default::default(),
+      raw_specifier: Default::default(),
     }
   }
 
@@ -498,6 +545,23 @@ impl CjsTracker {
     self.get_known_kind_with_is_script(specifier, media_type, None)
   }
 
+  pub fn get_raw_speficier(
+    &self,
+    specifier: &ModuleSpecifier,
+  ) -> Option<Ref<'_, Url, String>> {
+    self.raw_specifier.get(specifier)
+  }
+
+  pub fn set_raw_speficier(
+    &self,
+    specifier: &ModuleSpecifier,
+    raw_specifier: &str,
+  ) {
+    self
+      .raw_specifier
+      .insert(specifier.clone(), raw_specifier.to_string());
+  }
+
   fn get_known_kind_with_is_script(
     &self,
     specifier: &ModuleSpecifier,

tests/registry/npm/@denotest/cjs-non-explicit-file/1.0.0/index.js

@@ -0,0 +1 @@
+module.exports = {};

tests/registry/npm/@denotest/cjs-non-explicit-file/1.0.0/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "@denotest/cjs-non-explicit-file",
+  "version": "1.0.0",
+  "main": "index.js"
+}

tests/registry/npm/@denotest/cjs-non-explicit-file/1.0.0/server.js

@@ -0,0 +1 @@
+module.exports = require("./server.node");

tests/registry/npm/@denotest/cjs-non-explicit-file/1.0.0/server.node.js

@@ -0,0 +1,3 @@
+module.exports["server"] = function () {
+  return 1;
+};

tests/specs/npm/sub_paths_cjs_import_faillure/__test__.jsonc

@@ -0,0 +1,15 @@
+{
+  "steps": [{
+    "args": "run --quiet sub_paths_cjs_import_faillure/main1.js",
+    "output": "sub_paths_cjs_import_faillure/main1.out",
+    "exitCode": 1
+  }, {
+    "args": "run --quiet sub_paths_cjs_import_faillure/main2.js",
+    "output": "sub_paths_cjs_import_faillure/main2.out",
+    "exitCode": 1
+  }, {
+    "args": "run --quiet sub_paths_cjs_import_faillure/main3.js",
+    "output": "sub_paths_cjs_import_faillure/main3.out",
+    "exitCode": 1
+  }]
+}

tests/specs/npm/sub_paths_cjs_import_faillure/sub_paths_cjs_import_faillure/main1.js

@@ -0,0 +1,2 @@
+import Server from "npm:@denotest/[email protected]/server";
+console.log(Server);

tests/specs/npm/sub_paths_cjs_import_faillure/sub_paths_cjs_import_faillure/main1.out

@@ -0,0 +1,5 @@
+error: Unable to load [WILDCARD]server imported from [WILDCARD]main1.js
+Did you mean to import "npm:@denotest/[email protected]/server.js"?
+
+Caused by:
+[WILDCARD]

tests/specs/npm/sub_paths_cjs_import_faillure/sub_paths_cjs_import_faillure/main2.js

@@ -0,0 +1,2 @@
+import Server from "npm:@denotest/[email protected]/server.node";
+console.log(Server);

tests/specs/npm/sub_paths_cjs_import_faillure/sub_paths_cjs_import_faillure/main2.out

@@ -0,0 +1,5 @@
+error: Unable to load [WILDCARD]server.node imported from [WILDCARD]main2.js
+Did you mean to import "npm:@denotest/[email protected]/server.node.js"?
+
+Caused by:
+[WILDCARD]

tests/specs/npm/sub_paths_cjs_import_faillure/sub_paths_cjs_import_faillure/main3.js

@@ -0,0 +1,3 @@
+// deno-fmt-ignore
+const Server = await import("npm:@denotest/[email protected]/server");
+console.log(Server);

tests/specs/npm/sub_paths_cjs_import_faillure/sub_paths_cjs_import_faillure/main3.out

@@ -0,0 +1,7 @@
+error: Uncaught (in promise) TypeError: Unable to load [WILDCARD]server imported from [WILDCARD]main3.js
+Did you mean to import "npm:@denotest/[email protected]/server.js"?
+  Caused by:
+[WILDCARD]
+const Server = await import("npm:@denotest/[email protected]/server");
+               ^
+    at async [WILDCARD]main3.js:[WILDCARD]