From b2f221fe4973c4d45616a537f7c80f14e46959ba Mon Sep 17 00:00:00 2001
From: Gabriel Bota <gabriel.bota@dynatrace.com>
Date: Tue, 21 May 2024 13:54:28 +0200
Subject: [PATCH] further review findings

---
 lib/internal/modules/esm/get_format.js        |  1 -
 test/es-module/test-esm-detect-ambiguous.mjs  |  2 +-
 test/es-module/test-esm-loader-hooks.mjs      | 23 +++++++++++++++++++
 .../test-esm-named-exports-detect-module.js   | 13 -----------
 test/es-module/test-esm-named-exports.js      | 13 -----------
 .../require-esm-throws-with-loaders.js        | 10 ++++++++
 6 files changed, 34 insertions(+), 28 deletions(-)
 delete mode 100644 test/es-module/test-esm-named-exports-detect-module.js
 delete mode 100644 test/es-module/test-esm-named-exports.js
 create mode 100644 test/fixtures/es-modules/require-esm-throws-with-loaders.js

diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js
index f387b388274da6..c73e97f0ebdd56 100644
--- a/lib/internal/modules/esm/get_format.js
+++ b/lib/internal/modules/esm/get_format.js
@@ -23,7 +23,6 @@ const { containsModuleSyntax } = internalBinding('contextify');
 const { getPackageScopeConfig, getPackageType } = require('internal/modules/package_json_reader');
 const { fileURLToPath } = require('internal/url');
 const { readFileSync } = require('fs');
-const { Buffer } = require('buffer');
 const { ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;
 
 const protocolHandlers = {
diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs
index d023b5536dd5c7..7a490dfabcfac5 100644
--- a/test/es-module/test-esm-detect-ambiguous.mjs
+++ b/test/es-module/test-esm-detect-ambiguous.mjs
@@ -168,7 +168,7 @@ describe('--experimental-detect-module', { concurrency: !process.env.TEST_PARALL
       });
     }
 
-    it('should hint format correctly for extensionles modules resolve hook', async () => {
+    it('should hint format correctly for the resolve hook for extensionless modules', async () => {
       let writeSync;
       const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
         '--experimental-detect-module',
diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs
index a049350772aef7..73f5aa30dc8fb6 100644
--- a/test/es-module/test-esm-loader-hooks.mjs
+++ b/test/es-module/test-esm-loader-hooks.mjs
@@ -781,6 +781,29 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => {
     }
   });
 
+  describe('should use ESM loaders ', async () => {
+    for (const { testConfigName, additionalOptions } of [
+      { testConfigName: 'without --experimental-detect-module', additionalOptions: [] },
+      { testConfigName: 'with --experimental-detect-module', additionalOptions: ['--experimental-detect-module'] },
+    ]) {
+      it(testConfigName, async () => {
+        const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
+          ...additionalOptions,
+          '--import',
+          '../es-module-loaders/builtin-named-exports.mjs',
+          fixtures.path('es-modules/require-esm-throws-with-loaders.js'),
+        ], {
+          cwd: fixtures.path('es-modules'),
+        });
+
+        assert.strictEqual(stderr, '');
+        assert.strictEqual(stdout, '');
+        assert.strictEqual(code, 0);
+        assert.strictEqual(signal, null);
+      });
+    }
+  });
+
   it('should support source maps in commonjs translator', async () => {
     const readFile = async () => {};
     const hook = `
diff --git a/test/es-module/test-esm-named-exports-detect-module.js b/test/es-module/test-esm-named-exports-detect-module.js
deleted file mode 100644
index b77db78f7dfd97..00000000000000
--- a/test/es-module/test-esm-named-exports-detect-module.js
+++ /dev/null
@@ -1,13 +0,0 @@
-// Flags: --experimental-detect-module --import ./test/fixtures/es-module-loaders/builtin-named-exports.mjs
-'use strict';
-
-const common = require('../common');
-common.skipIfWorker();
-
-const { readFile, __fromLoader } = require('fs');
-const assert = require('assert');
-
-assert.throws(() => require('../fixtures/es-modules/test-esm-ok.mjs'), { code: 'ERR_REQUIRE_ESM' });
-
-assert(readFile);
-assert(__fromLoader);
diff --git a/test/es-module/test-esm-named-exports.js b/test/es-module/test-esm-named-exports.js
deleted file mode 100644
index 00b7aebbfd1f46..00000000000000
--- a/test/es-module/test-esm-named-exports.js
+++ /dev/null
@@ -1,13 +0,0 @@
-// Flags: --import ./test/fixtures/es-module-loaders/builtin-named-exports.mjs
-'use strict';
-
-const common = require('../common');
-common.skipIfWorker();
-
-const { readFile, __fromLoader } = require('fs');
-const assert = require('assert');
-
-assert.throws(() => require('../fixtures/es-modules/test-esm-ok.mjs'), { code: 'ERR_REQUIRE_ESM' });
-
-assert(readFile);
-assert(__fromLoader);
diff --git a/test/fixtures/es-modules/require-esm-throws-with-loaders.js b/test/fixtures/es-modules/require-esm-throws-with-loaders.js
new file mode 100644
index 00000000000000..d7908ec69e9b5b
--- /dev/null
+++ b/test/fixtures/es-modules/require-esm-throws-with-loaders.js
@@ -0,0 +1,10 @@
+'use strict';
+const fixtures = require('../../common/fixtures');
+const { readFile, __fromLoader } = require('fs');
+const assert = require('assert');
+
+const moduleName = fixtures.path('es-modules/test-esm-ok.mjs');
+assert.throws(() => require(moduleName), { code: 'ERR_REQUIRE_ESM' });
+
+assert(readFile);
+assert(__fromLoader);