esm: do not give wrong hints when detecting file format

PR-URL: #50314 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
nodejs · Oct 24, 2023 · 998feda · 998feda
1 parent 269e268
commit 998feda
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 15 deletions.
diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js
@@ -110,8 +110,10 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
       default: { // The user did not pass `--experimental-default-type`.
         // `source` is undefined when this is called from `defaultResolve`;
         // but this gets called again from `defaultLoad`/`defaultLoadSync`.
-        if (source && getOptionValue('--experimental-detect-module')) {
-          return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
+        if (getOptionValue('--experimental-detect-module')) {
+          return source ?
+            (containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs') :
+            null;
         }
         return 'commonjs';
       }
@@ -136,9 +138,13 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
         return 'commonjs';
       }
       default: { // The user did not pass `--experimental-default-type`.
-        if (source && getOptionValue('--experimental-detect-module') &&
-          getFormatOfExtensionlessFile(url) === 'module') {
-          return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
+        if (getOptionValue('--experimental-detect-module')) {
+          if (!source) { return null; }
+          const format = getFormatOfExtensionlessFile(url);
+          if (format === 'module') {
+            return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
+          }
+          return format;
         }
         return 'commonjs';
       }

diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js
@@ -105,7 +105,7 @@ function getSourceSync(url, context) {
  * @param {LoadContext} context
  * @returns {LoadReturn}
  */
-async function defaultLoad(url, context = { __proto__: null }) {
+async function defaultLoad(url, context = kEmptyObject) {
   let responseURL = url;
   let {
     importAttributes,
@@ -129,18 +129,22 @@ async function defaultLoad(url, context = { __proto__: null }) {
 
   if (urlInstance.protocol === 'node:') {
     source = null;
-  } else if (source == null) {
-    ({ responseURL, source } = await getSource(urlInstance, context));
-    context.source = source;
-  }
+    format ??= 'builtin';
+  } else {
+    let contextToPass = context;
+    if (source == null) {
+      ({ responseURL, source } = await getSource(urlInstance, context));
+      contextToPass = { __proto__: context, source };
+    }
 
-  if (format == null || format === 'commonjs') {
     // Now that we have the source for the module, run `defaultGetFormat` again in case we detect ESM syntax.
-    format = await defaultGetFormat(urlInstance, context);
-  }
+    format ??= await defaultGetFormat(urlInstance, contextToPass);
 
-  if (format === 'commonjs') {
-    source = null; // Let the CommonJS loader handle it (for now)
+    if (format === 'commonjs' && contextToPass !== context) {
+      // For backward compatibility reasons, we need to discard the source in
+      // order for the CJS loader to re-fetch it.
+      source = null;
+    }
   }
 
   validateAttributes(url, format, importAttributes);

diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs
@@ -152,6 +152,29 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
         strictEqual(signal, null);
       });
     }
+
+    it('should not hint wrong format in resolve hook', async () => {
+      let writeSync;
+      const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
+        '--experimental-detect-module',
+        '--no-warnings',
+        '--loader',
+        `data:text/javascript,import { writeSync } from "node:fs"; export ${encodeURIComponent(
+          async function resolve(s, c, next) {
+            const result = await next(s, c);
+            writeSync(1, result.format + '\n');
+            return result;
+          }
+        )}`,
+        fixtures.path('es-modules/package-without-type/noext-esm'),
+      ]);
+
+      strictEqual(stderr, '');
+      strictEqual(stdout, 'null\nexecuted\n');
+      strictEqual(code, 0);
+      strictEqual(signal, null);
+
+    });
   });
 
   describe('file input in a "type": "commonjs" package', { concurrency: true }, () => {