Commit 7347d34
module: fixing url change in load sync hook chain
Fixes: #56376
PR-URL: #56402
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>1 parent eb3148f commit 7347d34
3 files changed
Lines changed: 56 additions & 1 deletion
File tree
- lib/internal/modules/cjs
- test/module-hooks
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1144,7 +1144,7 @@ function getDefaultLoad(url, filename) { | |||
| 1144 | 1144 | return function defaultLoad(urlFromHook, context) { | |
| 1145 | 1145 | // If the url is the same as the original one, save the conversion. | |
| 1146 | 1146 | const isLoadingOriginalModule = (urlFromHook === url); | |
| 1147 | - const filenameFromHook = isLoadingOriginalModule ? filename : convertURLToCJSFilename(url); | ||
| 1147 | + const filenameFromHook = isLoadingOriginalModule ? filename : convertURLToCJSFilename(urlFromHook); | ||
| 1148 | 1148 | const source = defaultLoadImpl(filenameFromHook, context.format); | |
| 1149 | 1149 | // Format from context is directly returned, because format detection should only be | |
| 1150 | 1150 | // done after the entire load chain is completed. | |
Lines changed: 25 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,25 @@ | |||
| 1 | + import { mustCall } from '../common/index.mjs'; | ||
| 2 | + import assert from 'node:assert'; | ||
| 3 | + import { registerHooks } from 'node:module'; | ||
| 4 | + import { fileURL } from '../common/fixtures.mjs'; | ||
| 5 | + | ||
| 6 | + // This tests shows the url parameter in `load` can be changed in the `nextLoad` call | ||
| 7 | + // It changes `foo` package name into `redirected-fs` and then loads `redirected-fs` | ||
| 8 | + | ||
| 9 | + const hook = registerHooks({ | ||
| 10 | + resolve(specifier, context, nextResolve) { | ||
| 11 | + assert.strictEqual(specifier, 'foo'); | ||
| 12 | + return { | ||
| 13 | + url: 'foo://bar', | ||
| 14 | + shortCircuit: true, | ||
| 15 | + }; | ||
| 16 | + }, | ||
| 17 | + load: mustCall(function load(url, context, nextLoad) { | ||
| 18 | + assert.strictEqual(url, 'foo://bar'); | ||
| 19 | + return nextLoad(fileURL('module-hooks', 'redirected-fs.js').href, context); | ||
| 20 | + }), | ||
| 21 | + }); | ||
| 22 | + | ||
| 23 | + assert.strictEqual((await import('foo')).exports_for_test, 'redirected fs'); | ||
| 24 | + | ||
| 25 | + hook.deregister(); | ||
Lines changed: 30 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,30 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + | ||
| 3 | + const common = require('../common'); | ||
| 4 | + const assert = require('assert'); | ||
| 5 | + const { registerHooks } = require('module'); | ||
| 6 | + const fixtures = require('../common/fixtures'); | ||
| 7 | + | ||
| 8 | + // This tests shows the url parameter in `load` can be changed in the `nextLoad` call | ||
| 9 | + // It changes `foo` package name into `redirected-fs` and then loads `redirected-fs` | ||
| 10 | + | ||
| 11 | + const hook = registerHooks({ | ||
| 12 | + resolve(specifier, context, nextResolve) { | ||
| 13 | + assert.strictEqual(specifier, 'foo'); | ||
| 14 | + return { | ||
| 15 | + url: 'foo://bar', | ||
| 16 | + shortCircuit: true, | ||
| 17 | + }; | ||
| 18 | + }, | ||
| 19 | + load: common.mustCall(function load(url, context, nextLoad) { | ||
| 20 | + assert.strictEqual(url, 'foo://bar'); | ||
| 21 | + return nextLoad( | ||
| 22 | + fixtures.fileURL('module-hooks', 'redirected-fs.js').href, | ||
| 23 | + context, | ||
| 24 | + ); | ||
| 25 | + }), | ||
| 26 | + }); | ||
| 27 | + | ||
| 28 | + assert.strictEqual(require('foo').exports_for_test, 'redirected fs'); | ||
| 29 | + | ||
| 30 | + hook.deregister(); | ||
0 commit comments