FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: mock dual-package with conditional exports · nodejs/node@797ef40 · GitHub

/ node Public

Commit 797ef40

Browse files
maruthang
authored and
aduh95
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 <maruthang4@gmail.com> PR-URL: #62943 Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Jacob Smith <jacob@frende.me>
1 parent 1ad7ca3 commit 797ef40

6 files changed

Lines changed: 173 additions & 0 deletions

File tree

‎lib/internal/test_runner/mock/mock.js‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const {
2020
ReflectConstruct,
2121
ReflectGet,
2222
SafeMap,
23+
StringPrototypeIncludes,
2324
StringPrototypeSlice,
2425
StringPrototypeStartsWith,
2526
} = primordials;
@@ -207,6 +208,7 @@ class MockModuleContext {
207208
baseURL,
208209
cache,
209210
caller,
211+
cjsPath,
210212
format,
211213
fullPath,
212214
moduleExports,
@@ -222,12 +224,25 @@ class MockModuleContext {
222224

223225
sharedState.mockMap.set(baseURL, config);
224226
sharedState.mockMap.set(fullPath, config);
227+
// For dual packages (e.g., a package with a "exports" field that exposes
228+
// both ESM and CJS entry points), the file selected by the ESM resolver
229+
// (used to compute fullPath) may differ from the one selected by CJS
230+
// require(). Register the CJS-resolved path so that require() also picks
231+
// up the mock. See https://github.com/nodejs/node/issues/58231.
232+
if (cjsPath !== null && cjsPath !== fullPath) {
233+
sharedState.mockMap.set(cjsPath, config);
234+
}
225235

226236
this.#sharedState = sharedState;
227237
this.#restore = {
228238
__proto__: null,
229239
baseURL,
230240
cached: fullPath in Module._cache,
241+
cjsPath,
242+
cjsCached: cjsPath !== null && cjsPath !== fullPath &&
243+
cjsPath in Module._cache,
244+
cjsValue: cjsPath !== null && cjsPath !== fullPath ?
245+
Module._cache[cjsPath] : undefined,
231246
format,
232247
fullPath,
233248
value: Module._cache[fullPath],
@@ -257,6 +272,9 @@ class MockModuleContext {
257272
}
258273

259274
delete Module._cache[fullPath];
275+
if (cjsPath !== null && cjsPath !== fullPath) {
276+
delete Module._cache[cjsPath];
277+
}
260278
sharedState.mockExports.set(baseURL, {
261279
__proto__: null,
262280
moduleExports,
@@ -276,6 +294,14 @@ class MockModuleContext {
276294
Module._cache[this.#restore.fullPath] = this.#restore.value;
277295
}
278296

297+
if (this.#restore.cjsPath !== null &&
298+
this.#restore.cjsPath !== this.#restore.fullPath) {
299+
delete Module._cache[this.#restore.cjsPath];
300+
if (this.#restore.cjsCached) {
301+
Module._cache[this.#restore.cjsPath] = this.#restore.cjsValue;
302+
}
303+
}
304+
279305
const mock = mocks.get(this.#restore.baseURL);
280306

281307
if (mock !== undefined) {
@@ -285,6 +311,10 @@ class MockModuleContext {
285311

286312
this.#sharedState.mockMap.delete(this.#restore.baseURL);
287313
this.#sharedState.mockMap.delete(this.#restore.fullPath);
314+
if (this.#restore.cjsPath !== null &&
315+
this.#restore.cjsPath !== this.#restore.fullPath) {
316+
this.#sharedState.mockMap.delete(this.#restore.cjsPath);
317+
}
288318
this.#restore = undefined;
289319
}
290320
}
@@ -680,11 +710,19 @@ class MockTracker {
680710

681711
const fullPath = StringPrototypeStartsWith(url, 'file://') ?
682712
fileURLToPath(url) : null;
713+
// For dual packages, the ESM resolver may return a different file than
714+
// CJS require() would for the same specifier (e.g., when a package's
715+
// "exports" field points to different files for the "import" and
716+
// "require" conditions). Compute the CJS-resolved path so that
717+
// require() of a mocked module also picks up the mock.
718+
// See https://github.com/nodejs/node/issues/58231.
719+
const cjsPath = resolveAsCJS(mockSpecifier, caller, fullPath);
683720
const ctx = new MockModuleContext({
684721
__proto__: null,
685722
baseURL: baseURL.href,
686723
cache,
687724
caller,
725+
cjsPath,
688726
format,
689727
fullPath,
690728
moduleExports,
@@ -987,6 +1025,54 @@ function cjsMockModuleLoad(request, parent, isMain) {
9871025
return modExports;
9881026
}
9891027

1028+
// Resolve `specifier` using CJS resolution rules so that mocks for dual
1029+
// packages (e.g., a package whose "exports" field points to different files
1030+
// for the "import" and "require" conditions) also intercept require().
1031+
// Returns an absolute file path on success, or null when the specifier cannot
1032+
// be resolved as CJS (for example, when the package is ESM-only or when it is
1033+
// a non-file URL such as data: or node:).
1034+
function resolveAsCJS(specifier, callerURL, esmFullPath) {
1035+
if (isBuiltin(specifier) ||
1036+
StringPrototypeStartsWith(specifier, 'node:') ||
1037+
StringPrototypeStartsWith(specifier, 'data:')) {
1038+
return null;
1039+
}
1040+
1041+
let parentPath;
1042+
if (StringPrototypeStartsWith(callerURL, 'file://')) {
1043+
try {
1044+
parentPath = fileURLToPath(callerURL);
1045+
} catch {
1046+
return null;
1047+
}
1048+
} else {
1049+
return null;
1050+
}
1051+
1052+
try {
1053+
const tmpModule = new Module(parentPath, null);
1054+
tmpModule.paths = _nodeModulePaths(parentPath);
1055+
const resolved = _resolveFilename(specifier, tmpModule, false);
1056+
if (typeof resolved !== 'string') {
1057+
return null;
1058+
}
1059+
// If the resolution matches what the ESM resolver picked, there is
1060+
// nothing additional to register.
1061+
if (resolved === esmFullPath) {
1062+
return esmFullPath;
1063+
}
1064+
// If the resolution returned something that is not a filesystem path
1065+
// (e.g., a builtin id without a slash or backslash), ignore it.
1066+
if (!StringPrototypeIncludes(resolved, '/') &&
1067+
!StringPrototypeIncludes(resolved, '\\')) {
1068+
return null;
1069+
}
1070+
return resolved;
1071+
} catch {
1072+
return null;
1073+
}
1074+
}
1075+
9901076
function validateStringOrSymbol(value, name) {
9911077
if (typeof value !== 'string' && typeof value !== 'symbol') {
9921078
throw new ERR_INVALID_ARG_TYPE(name, ['string', 'symbol'], value);

‎test/fixtures/test-runner/mock-nm-dual-pkg.js‎

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.

‎test/parallel/test-runner-mock-dual-package.js‎

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)
Back | FazBrowse Home | New Git URL