From 3b867e42563edd053ae7ef9bbb339dbba25e552d Mon Sep 17 00:00:00 2001
From: Antoine du Hamel <duhamelantoine1995@gmail.com>
Date: Mon, 23 Oct 2023 18:01:59 +0200
Subject: [PATCH] esm: do not give wrong hints when detecting file format

PR-URL: https://github.com/nodejs/node/pull/50314
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
---
 lib/internal/modules/esm/get_format.js       | 16 +++++++++----
 lib/internal/modules/esm/load.js             | 24 ++++++++++++--------
 test/es-module/test-esm-detect-ambiguous.mjs | 23 +++++++++++++++++++
 3 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js
index ee50bea472758d..1931688e85d05e 100644
--- 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
index f706b5b808da27..6f9b73abd8a761 100644
--- 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
index 61629965518a82..34c5f17f007c8f 100644
--- 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 }, () => {