esm: fix loader hooks accepting too many arguments

lib/internal/modules/esm/loader.js

@@ -148,29 +148,29 @@ function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
   }
 
   return ObjectDefineProperty(
-    async (...args) => {
+    async (arg0, context) => {
       // Update only when hook is invoked to avoid fingering the wrong filePath
       meta.hookErrIdentifier = `${hookFilePath} '${hookName}'`;
 
-      validateArgs(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, args);
+      validateArgs(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, [arg0, context]);
 
       const outputErrIdentifier = `${chain[generatedHookIndex].url} '${hookName}' hook's ${nextHookName}()`;
 
       // Set when next<HookName> is actually called, not just generated.
       if (generatedHookIndex === 0) { meta.chainFinished = true; }
 
-      // `context` is an optional argument that only needs to be passed when changed
-      switch (args.length) {
-        case 1: // It was omitted, so supply the cached value
-          ArrayPrototypePush(args, meta.context);
-          break;
-        case 2: // Overrides were supplied, so update cached value
-          ObjectAssign(meta.context, args[1]);
-          break;
+      const argsToApply = [arg0];
+
+      if (context) { // `context` has already been validated, so no fancy check needed.
+        argsToApply[1] = context;
+        ObjectAssign(meta.context, context);
+      } else {
+        argsToApply[1] = meta.context;
       }
 
-      ArrayPrototypePush(args, nextNextHook);
-      const output = await ReflectApply(hook, undefined, args);
+      argsToApply[2] = nextNextHook;
+
+      const output = await ReflectApply(hook, undefined, argsToApply);
 
       validateOutput(outputErrIdentifier, output);
 

test/es-module/test-esm-loader-chaining.mjs

@@ -101,6 +101,28 @@ describe('ESM: loader chaining', { concurrency: true }, () => {
     assert.strictEqual(code, 0);
   });
 
+  it('should accept only the correct arguments', async () => {
+    const { stdout } = await spawnPromisified(
+      execPath,
+      [
+        '--loader',
+        fixtures.fileURL('es-module-loaders', 'loader-log-args.mjs'),
+        '--loader',
+        fixtures.fileURL('es-module-loaders', 'loader-with-too-many-args.mjs'),
+        ...commonArgs,
+      ],
+      { encoding: 'utf8' },
+    );
+
+    assert.match(stdout, /resolve arg count: 3/);
+    assert.match(stdout, /specifier: 'node:fs'/);
+    assert.match(stdout, /next: \[AsyncFunction: nextResolve\]/);
+
+    assert.match(stdout, /load arg count: 3/);
+    assert.match(stdout, /url: 'node:fs'/);
+    assert.match(stdout, /next: \[AsyncFunction: nextLoad\]/);
+  });
+
   it('should result in proper output from multiple changes in resolve hooks', async () => {
     const { code, stderr, stdout } = await spawnPromisified(
       execPath,

test/fixtures/es-module-loaders/loader-log-args.mjs

@@ -0,0 +1,28 @@
+export async function resolve(...args) {
+  console.log(`resolve arg count: ${args.length}`);
+  console.log({
+    specifier: args[0],
+    context: args[1],
+    next: args[2],
+  });
+
+  return {
+    shortCircuit: true,
+    url: args[0],
+  };
+}
+
+export async function load(...args) {
+  console.log(`load arg count: ${args.length}`);
+  console.log({
+    url: args[0],
+    context: args[1],
+    next: args[2],
+  });
+
+  return {
+    format: 'module',
+    source: '',
+    shortCircuit: true,
+  };
+}

test/fixtures/es-module-loaders/loader-with-too-many-args.mjs

@@ -0,0 +1,7 @@
+export async function resolve(specifier, context, next) {
+  return next(specifier, context, 'resolve-extra-arg');
+}
+
+export async function load(url, context, next) {
+  return next(url, context, 'load-extra-arg');
+}