module: allow omitting context in synchronous next hooks

lib/internal/modules/customization_hooks.js

@@ -4,6 +4,7 @@ const {
   ArrayPrototypeFindIndex,
   ArrayPrototypePush,
   ArrayPrototypeSplice,
+  ObjectAssign,
   ObjectFreeze,
   StringPrototypeStartsWith,
   Symbol,
@@ -162,19 +163,31 @@ function convertURLToCJSFilename(url) {
  * @param {'load'|'resolve'} name Name of the hook in ModuleHooks.
  * @param {Function} defaultStep The default step in the chain.
  * @param {Function} validate A function that validates and sanitize the result returned by the chain.
- * @returns {Function}
  */
-function buildHooks(hooks, name, defaultStep, validate) {
+function buildHooks(hooks, name, defaultStep, validate, mergedContext) {
   let lastRunIndex = hooks.length;
-  function wrapHook(index, userHook, next) {
-    return function wrappedHook(...args) {
+  /**
+   * Helper function to wrap around invocation of user hook or the default step
+   * in order to fill in missing arguments or check returned results.
+   * Due to the merging of the context, this must be a closure.
+   * @param {number} index Index in the chain. Default step is 0, last added hook is 1,
+   *                       and so on.
+   * @param {Function} userHookOrDefault Either the user hook or the default step to invoke.
+   * @param {Function|undefined} next The next wrapped step. If this is the default step, it's undefined.
+   * @returns {Function} Wrapped hook or default step.
+   */
+  function wrapHook(index, userHookOrDefault, next) {
+    return function nextStep(arg0, context) {
       lastRunIndex = index;
-      const hookResult = userHook(...args, next);
+      if (context && context !== mergedContext) {
+        ObjectAssign(mergedContext, context);
+      }
+      const hookResult = userHookOrDefault(arg0, mergedContext, next);
       if (lastRunIndex > 0 && lastRunIndex === index && !hookResult.shortCircuit) {
         throw new ERR_INVALID_RETURN_PROPERTY_VALUE('true', name, 'shortCircuit',
                                                     hookResult.shortCircuit);
       }
-      return validate(...args, hookResult);
+      return validate(arg0, mergedContext, hookResult);
     };
   }
   const chain = [wrapHook(0, defaultStep)];
@@ -330,7 +343,7 @@ function loadWithHooks(url, originalFormat, importAttributes, conditions, defaul
     return defaultLoad(url, context);
   }
 
-  const runner = buildHooks(loadHooks, 'load', defaultLoad, validateLoad);
+  const runner = buildHooks(loadHooks, 'load', defaultLoad, validateLoad, context);
 
   const result = runner(url, context);
   const { source, format } = result;
@@ -373,7 +386,7 @@ function resolveWithHooks(specifier, parentURL, importAttributes, conditions, de
     return defaultResolve(specifier, context);
   }
 
-  const runner = buildHooks(resolveHooks, 'resolve', defaultResolve, validateResolve);
+  const runner = buildHooks(resolveHooks, 'resolve', defaultResolve, validateResolve, context);
 
   return runner(specifier, context);
 }

test/module-hooks/test-module-hooks-load-context-merged-esm.mjs

@@ -0,0 +1,31 @@
+// Test that the context parameter will be merged in multiple load hooks.
+
+import * as common from '../common/index.mjs';
+import assert from 'node:assert';
+import { registerHooks } from 'node:module';
+
+const hook1 = registerHooks({
+  load: common.mustCall(function(url, context, nextLoad) {
+    assert.strictEqual(context.testProp, 'custom');  // It should be merged from hook 2 and 3.
+    return nextLoad(url, context);
+  }, 1),
+});
+
+const hook2 = registerHooks({
+  load: common.mustCall(function(url, context, nextLoad) {
+    assert.strictEqual(context.testProp, 'custom');  // It should be merged from hook 3.
+    return nextLoad(url);  // Omit the context.
+  }, 1),
+});
+
+const hook3 = registerHooks({
+  load: common.mustCall(function(url, context, nextLoad) {
+    return nextLoad(url, { testProp: 'custom' });  // Add a custom property
+  }, 1),
+});
+
+await import('../fixtures/es-modules/message.mjs');
+
+hook3.deregister();
+hook2.deregister();
+hook1.deregister();

test/module-hooks/test-module-hooks-load-context-merged.js

@@ -0,0 +1,33 @@
+'use strict';
+
+// Test that the context parameter will be merged in multiple load hooks.
+
+const common = require('../common');
+const assert = require('assert');
+const { registerHooks } = require('module');
+
+const hook1 = registerHooks({
+  load: common.mustCall(function(url, context, nextLoad) {
+    assert.strictEqual(context.testProp, 'custom');  // It should be merged from hook 2 and 3.
+    return nextLoad(url, context);
+  }, 1),
+});
+
+const hook2 = registerHooks({
+  load: common.mustCall(function(url, context, nextLoad) {
+    assert.strictEqual(context.testProp, 'custom');  // It should be merged from hook 3.
+    return nextLoad(url);  // Omit the context.
+  }, 1),
+});
+
+const hook3 = registerHooks({
+  load: common.mustCall(function(url, context, nextLoad) {
+    return nextLoad(url, { testProp: 'custom' });  // Add a custom property
+  }, 1),
+});
+
+require('../fixtures/empty.js');
+
+hook3.deregister();
+hook2.deregister();
+hook1.deregister();

test/module-hooks/test-module-hooks-load-context-optional-esm.mjs

@@ -0,0 +1,14 @@
+// Test that the context parameter can be omitted in the nextLoad invocation.
+
+import * as common from '../common/index.mjs';
+import { registerHooks } from 'node:module';
+
+const hook = registerHooks({
+  load: common.mustCall(function(url, context, nextLoad) {
+    return nextLoad(url);
+  }, 1),
+});
+
+await import('../fixtures/es-modules/message.mjs');
+
+hook.deregister();

test/module-hooks/test-module-hooks-load-context-optional.js

@@ -0,0 +1,16 @@
+'use strict';
+
+// Test that the context parameter can be omitted in the nextLoad invocation.
+
+const common = require('../common');
+const { registerHooks } = require('module');
+
+const hook = registerHooks({
+  load: common.mustCall(function(url, context, nextLoad) {
+    return nextLoad(url);
+  }, 1),
+});
+
+require('../fixtures/empty.js');
+
+hook.deregister();

test/module-hooks/test-module-hooks-resolve-context-merged-esm.mjs

@@ -0,0 +1,32 @@
+// Test that the context parameter will be merged in multiple resolve hooks.
+
+import * as common from '../common/index.mjs';
+import assert from 'node:assert';
+import { registerHooks } from 'node:module';
+
+const hook1 = registerHooks({
+  resolve: common.mustCall(function(specifier, context, nextResolve) {
+    assert.strictEqual(context.testProp, 'custom');  // It should be merged from hook 2 and 3.
+    const result = nextResolve(specifier, context);
+    return result;
+  }, 1),
+});
+
+const hook2 = registerHooks({
+  resolve: common.mustCall(function(specifier, context, nextResolve) {
+    assert.strictEqual(context.testProp, 'custom');  // It should be merged from hook 3.
+    return nextResolve(specifier);  // Omit the context.
+  }, 1),
+});
+
+const hook3 = registerHooks({
+  resolve: common.mustCall(function(specifier, context, nextResolve) {
+    return nextResolve(specifier, { testProp: 'custom' });  // Add a custom property
+  }, 1),
+});
+
+await import('../fixtures/es-modules/message.mjs');
+
+hook3.deregister();
+hook2.deregister();
+hook1.deregister();

test/module-hooks/test-module-hooks-resolve-context-merged.js

@@ -0,0 +1,34 @@
+'use strict';
+
+// Test that the context parameter will be merged in multiple resolve hooks.
+
+const common = require('../common');
+const assert = require('assert');
+const { registerHooks } = require('module');
+
+const hook1 = registerHooks({
+  resolve: common.mustCall(function(specifier, context, nextResolve) {
+    assert.strictEqual(context.testProp, 'custom');  // It should be merged from hook 2 and 3.
+    const result = nextResolve(specifier, context);
+    return result;
+  }, 1),
+});
+
+const hook2 = registerHooks({
+  resolve: common.mustCall(function(specifier, context, nextResolve) {
+    assert.strictEqual(context.testProp, 'custom');  // It should be merged from hook 3.
+    return nextResolve(specifier);  // Omit the context.
+  }, 1),
+});
+
+const hook3 = registerHooks({
+  resolve: common.mustCall(function(specifier, context, nextResolve) {
+    return nextResolve(specifier, { testProp: 'custom' });  // Add a custom property
+  }, 1),
+});
+
+require('../fixtures/empty.js');
+
+hook3.deregister();
+hook2.deregister();
+hook1.deregister();

test/module-hooks/test-module-hooks-resolve-context-optional-esm.mjs

@@ -0,0 +1,14 @@
+// Test that the context parameter can be omitted in the nextResolve invocation.
+
+import * as common from '../common/index.mjs';
+import { registerHooks } from 'node:module';
+
+const hook = registerHooks({
+  resolve: common.mustCall(function(specifier, context, nextResolve) {
+    return nextResolve(specifier);
+  }, 1),
+});
+
+await import('../fixtures/es-modules/message.mjs');
+
+hook.deregister();

test/module-hooks/test-module-hooks-resolve-context-optional.js

@@ -0,0 +1,16 @@
+'use strict';
+
+// Test that the context parameter can be omitted in the nextResolve invocation.
+
+const common = require('../common');
+const { registerHooks } = require('module');
+
+const hook = registerHooks({
+  resolve: common.mustCall(function(specifier, context, nextResolve) {
+    return nextResolve(specifier);
+  }, 1),
+});
+
+require('../fixtures/empty.js');
+
+hook.deregister();