Skip to content

Commit

Permalink
module: fixing url change in load sync hook chain
Browse files Browse the repository at this point in the history
Fixes: #56376
PR-URL: #56402
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
awto authored and jasnell committed Jan 22, 2025
1 parent 01dd7c4 commit 9ec7bed
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -1144,7 +1144,7 @@ function getDefaultLoad(url, filename) {
return function defaultLoad(urlFromHook, context) {
// If the url is the same as the original one, save the conversion.
const isLoadingOriginalModule = (urlFromHook === url);
const filenameFromHook = isLoadingOriginalModule ? filename : convertURLToCJSFilename(url);
const filenameFromHook = isLoadingOriginalModule ? filename : convertURLToCJSFilename(urlFromHook);
const source = defaultLoadImpl(filenameFromHook, context.format);
// Format from context is directly returned, because format detection should only be
// done after the entire load chain is completed.
Expand Down
25 changes: 25 additions & 0 deletions test/module-hooks/test-module-hooks-load-url-change-import.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { mustCall } from '../common/index.mjs';
import assert from 'node:assert';
import { registerHooks } from 'node:module';
import { fileURL } from '../common/fixtures.mjs';

// This tests shows the url parameter in `load` can be changed in the `nextLoad` call
// It changes `foo` package name into `redirected-fs` and then loads `redirected-fs`

const hook = registerHooks({
resolve(specifier, context, nextResolve) {
assert.strictEqual(specifier, 'foo');
return {
url: 'foo://bar',
shortCircuit: true,
};
},
load: mustCall(function load(url, context, nextLoad) {
assert.strictEqual(url, 'foo://bar');
return nextLoad(fileURL('module-hooks', 'redirected-fs.js').href, context);
}),
});

assert.strictEqual((await import('foo')).exports_for_test, 'redirected fs');

hook.deregister();
30 changes: 30 additions & 0 deletions test/module-hooks/test-module-hooks-load-url-change-require.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const { registerHooks } = require('module');
const fixtures = require('../common/fixtures');

// This tests shows the url parameter in `load` can be changed in the `nextLoad` call
// It changes `foo` package name into `redirected-fs` and then loads `redirected-fs`

const hook = registerHooks({
resolve(specifier, context, nextResolve) {
assert.strictEqual(specifier, 'foo');
return {
url: 'foo://bar',
shortCircuit: true,
};
},
load: common.mustCall(function load(url, context, nextLoad) {
assert.strictEqual(url, 'foo://bar');
return nextLoad(
fixtures.fileURL('module-hooks', 'redirected-fs.js').href,
context,
);
}),
});

assert.strictEqual(require('foo').exports_for_test, 'redirected fs');

hook.deregister();

0 comments on commit 9ec7bed

Please sign in to comment.