Skip to content

Commit 8c4a8e4

Browse files
committed
test_runner: mock dual-package with conditional exports
When `mock.module()` targets a package whose `exports` field maps `import` and `require` to different files, the ESM resolver and the CJS resolver disagree on the resolved path. Only the ESM path was registered in `mockMap`, so `require()` of the mocked specifier bypassed the mock and loaded the real CJS module. Resolve the specifier through `Module._resolveFilename` from the caller's directory in addition to the existing ESM resolution. When the two paths differ, register the CJS path as a second key in `mockMap` and invalidate `Module._cache[cjsPath]`, restoring it on `restore()`. Single-resolution packages keep their existing behavior. Fixes: #58231 Signed-off-by: Maruthan G <[email protected]>
1 parent 4744070 commit 8c4a8e4

6 files changed

Lines changed: 175 additions & 0 deletions

File tree

lib/internal/test_runner/mock/mock.js

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const {
1616
ReflectConstruct,
1717
ReflectGet,
1818
SafeMap,
19+
StringPrototypeIncludes,
1920
StringPrototypeSlice,
2021
StringPrototypeStartsWith,
2122
} = primordials;
@@ -196,6 +197,7 @@ class MockModuleContext {
196197
baseURL,
197198
cache,
198199
caller,
200+
cjsPath,
199201
format,
200202
fullPath,
201203
moduleExports,
@@ -211,12 +213,25 @@ class MockModuleContext {
211213

212214
sharedState.mockMap.set(baseURL, config);
213215
sharedState.mockMap.set(fullPath, config);
216+
// For dual packages (e.g., a package with a "exports" field that exposes
217+
// both ESM and CJS entry points), the file selected by the ESM resolver
218+
// (used to compute fullPath) may differ from the one selected by CJS
219+
// require(). Register the CJS-resolved path so that require() also picks
220+
// up the mock. See https://github.com/nodejs/node/issues/58231.
221+
if (cjsPath !== null && cjsPath !== fullPath) {
222+
sharedState.mockMap.set(cjsPath, config);
223+
}
214224

215225
this.#sharedState = sharedState;
216226
this.#restore = {
217227
__proto__: null,
218228
baseURL,
219229
cached: fullPath in Module._cache,
230+
cjsPath,
231+
cjsCached: cjsPath !== null && cjsPath !== fullPath &&
232+
cjsPath in Module._cache,
233+
cjsValue: cjsPath !== null && cjsPath !== fullPath ?
234+
Module._cache[cjsPath] : undefined,
220235
format,
221236
fullPath,
222237
value: Module._cache[fullPath],
@@ -246,6 +261,9 @@ class MockModuleContext {
246261
}
247262

248263
delete Module._cache[fullPath];
264+
if (cjsPath !== null && cjsPath !== fullPath) {
265+
delete Module._cache[cjsPath];
266+
}
249267
sharedState.mockExports.set(baseURL, {
250268
__proto__: null,
251269
moduleExports,
@@ -265,6 +283,14 @@ class MockModuleContext {
265283
Module._cache[this.#restore.fullPath] = this.#restore.value;
266284
}
267285

286+
if (this.#restore.cjsPath !== null &&
287+
this.#restore.cjsPath !== this.#restore.fullPath) {
288+
delete Module._cache[this.#restore.cjsPath];
289+
if (this.#restore.cjsCached) {
290+
Module._cache[this.#restore.cjsPath] = this.#restore.cjsValue;
291+
}
292+
}
293+
268294
const mock = mocks.get(this.#restore.baseURL);
269295

270296
if (mock !== undefined) {
@@ -274,6 +300,10 @@ class MockModuleContext {
274300

275301
this.#sharedState.mockMap.delete(this.#restore.baseURL);
276302
this.#sharedState.mockMap.delete(this.#restore.fullPath);
303+
if (this.#restore.cjsPath !== null &&
304+
this.#restore.cjsPath !== this.#restore.fullPath) {
305+
this.#sharedState.mockMap.delete(this.#restore.cjsPath);
306+
}
277307
this.#restore = undefined;
278308
}
279309
}
@@ -669,11 +699,19 @@ class MockTracker {
669699

670700
const fullPath = StringPrototypeStartsWith(url, 'file://') ?
671701
fileURLToPath(url) : null;
702+
// For dual packages, the ESM resolver may return a different file than
703+
// CJS require() would for the same specifier (e.g., when a package's
704+
// "exports" field points to different files for the "import" and
705+
// "require" conditions). Compute the CJS-resolved path so that
706+
// require() of a mocked module also picks up the mock.
707+
// See https://github.com/nodejs/node/issues/58231.
708+
const cjsPath = resolveAsCJS(mockSpecifier, caller, fullPath);
672709
const ctx = new MockModuleContext({
673710
__proto__: null,
674711
baseURL: baseURL.href,
675712
cache,
676713
caller,
714+
cjsPath,
677715
format,
678716
fullPath,
679717
moduleExports,
@@ -960,6 +998,56 @@ function cjsMockModuleLoad(request, parent, isMain) {
960998
return modExports;
961999
}
9621000

1001+
// Resolve `specifier` using CJS resolution rules so that mocks for dual
1002+
// packages (e.g., a package whose "exports" field points to different files
1003+
// for the "import" and "require" conditions) also intercept require().
1004+
// Returns an absolute file path on success, or null when the specifier cannot
1005+
// be resolved as CJS (for example, when the package is ESM-only or when it is
1006+
// a non-file URL such as data:, http:, or node:).
1007+
function resolveAsCJS(specifier, callerURL, esmFullPath) {
1008+
if (isBuiltin(specifier) ||
1009+
StringPrototypeStartsWith(specifier, 'node:') ||
1010+
StringPrototypeStartsWith(specifier, 'data:') ||
1011+
StringPrototypeStartsWith(specifier, 'http:') ||
1012+
StringPrototypeStartsWith(specifier, 'https:')) {
1013+
return null;
1014+
}
1015+
1016+
let parentPath;
1017+
if (StringPrototypeStartsWith(callerURL, 'file://')) {
1018+
try {
1019+
parentPath = fileURLToPath(callerURL);
1020+
} catch {
1021+
return null;
1022+
}
1023+
} else {
1024+
return null;
1025+
}
1026+
1027+
try {
1028+
const tmpModule = new Module(parentPath, null);
1029+
tmpModule.paths = _nodeModulePaths(parentPath);
1030+
const resolved = _resolveFilename(specifier, tmpModule, false);
1031+
if (typeof resolved !== 'string') {
1032+
return null;
1033+
}
1034+
// If the resolution matches what the ESM resolver picked, there is
1035+
// nothing additional to register.
1036+
if (resolved === esmFullPath) {
1037+
return esmFullPath;
1038+
}
1039+
// If the resolution returned something that is not a filesystem path
1040+
// (e.g., a builtin id without a slash or backslash), ignore it.
1041+
if (!StringPrototypeIncludes(resolved, '/') &&
1042+
!StringPrototypeIncludes(resolved, '\\')) {
1043+
return null;
1044+
}
1045+
return resolved;
1046+
} catch {
1047+
return null;
1048+
}
1049+
}
1050+
9631051
function validateStringOrSymbol(value, name) {
9641052
if (typeof value !== 'string' && typeof value !== 'symbol') {
9651053
throw new ERR_INVALID_ARG_TYPE(name, ['string', 'symbol'], value);
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict';
2+
const assert = require('node:assert');
3+
const { test } = require('node:test');
4+
const fixture = 'dual-pkg-with-exports';
5+
6+
test('mock node_modules dual package with conditional exports', async (t) => {
7+
const mock = t.mock.module(fixture, {
8+
namedExports: { add(x, y) { return 1 + x + y; }, flavor: 'mocked' },
9+
});
10+
11+
// CJS require should pick up the mock even though the package's "exports"
12+
// field maps the "require" condition to a different file than "import".
13+
const cjsImpl = require(fixture);
14+
assert.strictEqual(cjsImpl.add(4, 5), 10);
15+
assert.strictEqual(cjsImpl.flavor, 'mocked');
16+
17+
// ESM dynamic import should also pick up the mock.
18+
const esmImpl = await import(fixture);
19+
assert.strictEqual(esmImpl.add(4, 5), 10);
20+
assert.strictEqual(esmImpl.flavor, 'mocked');
21+
22+
mock.restore();
23+
24+
// After restore, both module systems should see the original exports.
25+
const restoredCjs = require(fixture);
26+
assert.strictEqual(restoredCjs.add(4, 5), 9);
27+
assert.strictEqual(restoredCjs.flavor, 'cjs');
28+
29+
const restoredEsm = await import(fixture);
30+
assert.strictEqual(restoredEsm.add(4, 5), 9);
31+
assert.strictEqual(restoredEsm.flavor, 'esm');
32+
});

test/fixtures/test-runner/node_modules/dual-pkg-with-exports/index.cjs

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/test-runner/node_modules/dual-pkg-with-exports/index.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/test-runner/node_modules/dual-pkg-with-exports/package.json

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
const common = require('../common');
3+
const { isMainThread } = require('worker_threads');
4+
5+
if (!isMainThread) {
6+
common.skip('registering customization hooks in Workers does not work');
7+
}
8+
9+
const fixtures = require('../common/fixtures');
10+
const assert = require('node:assert');
11+
const { test } = require('node:test');
12+
13+
// Regression test for https://github.com/nodejs/node/issues/58231
14+
// When a dual package exposes both ESM and CJS entry points via the
15+
// "exports" field with "import"/"require" conditions, the ESM resolver
16+
// picks one file (e.g. index.js) and CJS require() picks another
17+
// (e.g. index.cjs). mock.module() must intercept both so that require()
18+
// of the mocked module does not return the original CJS file.
19+
test('mock.module intercepts dual package require with conditional exports',
20+
async () => {
21+
const cwd = fixtures.path('test-runner');
22+
const fixture = fixtures.path('test-runner', 'mock-nm-dual-pkg.js');
23+
const args = ['--experimental-test-module-mocks', fixture];
24+
const {
25+
code,
26+
stdout,
27+
signal,
28+
} = await common.spawnPromisified(process.execPath, args, { cwd });
29+
30+
assert.strictEqual(signal, null);
31+
assert.strictEqual(code, 0,
32+
'child process exited with non-zero status\n' +
33+
`stdout:\n${stdout}`);
34+
assert.match(stdout, /pass 1/);
35+
assert.match(stdout, /fail 0/);
36+
});

0 commit comments

Comments
 (0)