Skip to content

Commit 74bac97

Browse files
committed
Adds tests
1 parent 19b9e1b commit 74bac97

File tree

10 files changed

+182
-37
lines changed

10 files changed

+182
-37
lines changed

doc/api/packages.md

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,15 +1020,15 @@ When a bare specifier is encountered:
10201020

10211021
1. Node.js determines which package contains the importing file by checking
10221022
if the file path is within any package's `path`.
1023-
2. If the importing file is not within any mapped package, standard
1024-
`node_modules` resolution is used.
1023+
2. If the importing file is not within any mapped package, a
1024+
`MODULE_NOT_FOUND` error is thrown.
10251025
3. Node.js searches the importing package's `dependencies` array for an entry
10261026
whose `name` matches the specifier's package name.
10271027
4. If found, the specifier resolves to that dependency's `path`.
10281028
5. If the package exists in the map but is not in `dependencies`, an
10291029
[`ERR_PACKAGE_MAP_ACCESS_DENIED`][] error is thrown.
1030-
6. If the package does not exist in the map at all, standard `node_modules`
1031-
resolution is used as a fallback.
1030+
6. If the package does not exist in the map at all, a
1031+
`MODULE_NOT_FOUND` error is thrown.
10321032

10331033
### Subpath resolution
10341034

@@ -1094,16 +1094,15 @@ const utils = require('@myorg/utils');
10941094
import utils from '@myorg/utils';
10951095
```
10961096

1097-
### Fallback behavior
1097+
### Interaction with other resolution
10981098

1099-
Package maps do not replace `node_modules` resolution entirely. Resolution
1100-
falls back to standard behavior when:
1099+
Package maps only apply to bare specifiers that are not Node.js builtin
1100+
modules. The following cases are not affected by package maps and continue
1101+
to use standard resolution:
11011102

1102-
* The importing file is not within any package defined in the map.
1103-
* The specifier's package name is not found in any package's `name` field.
1104-
* The specifier is a relative path (`./` or `../`).
1105-
* The specifier is an absolute path or URL.
1106-
* The specifier refers to a Node.js builtin module (`node:fs`, etc.).
1103+
* Relative paths (`./` or `../`).
1104+
* Absolute paths or URLs.
1105+
* Node.js builtin modules (`node:fs`, etc.).
11071106

11081107
### Limitations
11091108

lib/internal/modules/cjs/loader.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -665,8 +665,15 @@ function tryPackageMapResolveCJS(request, parent, conditions) {
665665
const parentPath = parent?.filename ?? process.cwd() + path.sep;
666666
const mapped = packageMapResolve(request, parentPath);
667667

668-
// Not in package map - fall back to standard resolution
669-
if (mapped === undefined) { return undefined; }
668+
if (mapped === undefined) {
669+
const requireStack = getRequireStack(parent);
670+
const message = getRequireStackMessage(request, requireStack);
671+
// eslint-disable-next-line no-restricted-syntax
672+
const err = new Error(message);
673+
err.code = 'MODULE_NOT_FOUND';
674+
err.requireStack = requireStack;
675+
throw err;
676+
}
670677

671678
const { packagePath, subpath } = mapped;
672679

lib/internal/modules/package_map.js

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ const {
77
SafeSet,
88
StringPrototypeIndexOf,
99
StringPrototypeSlice,
10-
StringPrototypeStartsWith,
1110
} = primordials;
1211

1312
const assert = require('internal/assert');
1413
const { getLazy } = require('internal/util');
15-
const { resolve: pathResolve, dirname, sep } = require('path');
14+
const { resolve: pathResolve, dirname } = require('path');
1615

1716
const getPackageMapPath = getLazy(() =>
1817
require('internal/options').getOptionValue('--experimental-package-map'),
@@ -117,7 +116,7 @@ class PackageMap {
117116

118117
// Walk up to find containing package
119118
let checkPath = filePath;
120-
while (isPathContainedIn(checkPath, this.#basePath)) {
119+
while (true) {
121120
const keys = this.#pathToKeys.get(checkPath);
122121
if (keys && keys.size > 0) {
123122
const key = keys.values().next().value;
@@ -144,7 +143,7 @@ class PackageMap {
144143
resolve(specifier, parentPath) {
145144
const parentKey = this.#getKeyForPath(parentPath);
146145

147-
// Parent not in map - fall back to standard resolution
146+
// Parent not in map - let the caller throw the appropriate error
148147
if (parentKey === null) { return undefined; }
149148

150149
// Check cache
@@ -189,7 +188,7 @@ class PackageMap {
189188
this.#configPath,
190189
);
191190
}
192-
// Package not in map at all - fall back to standard resolution
191+
// Package not in map at all - let the caller throw the appropriate error
193192
return undefined;
194193
}
195194

@@ -198,17 +197,6 @@ class PackageMap {
198197
}
199198
}
200199

201-
/**
202-
* Check if a file path is contained within a folder path.
203-
* Handles edge cases like /foo/bar not matching /foo/barbaz.
204-
* @param {string} file - The file path to check
205-
* @param {string} folder - The folder path to check against
206-
* @returns {boolean}
207-
*/
208-
function isPathContainedIn(file, folder) {
209-
return file === folder || StringPrototypeStartsWith(file, folder + sep);
210-
}
211-
212200
/**
213201
* Parse a package specifier into name and subpath.
214202
* @param {string} specifier

test/es-module/test-esm-package-map.mjs

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ describe('ESM: --experimental-package-map', () => {
8484

8585
// =========== Fallback Behavior ===========
8686

87-
describe('fallback to standard resolution', () => {
87+
describe('resolution boundaries', () => {
8888
it('falls back for builtin modules', async () => {
8989
const { code, stdout, stderr } = await spawnPromisified(execPath, [
9090
'--experimental-package-map', packageMapPath,
@@ -97,17 +97,16 @@ describe('ESM: --experimental-package-map', () => {
9797
assert.match(stdout, /function/);
9898
});
9999

100-
it('falls back when parent not in map', async () => {
100+
it('throws when parent not in map', async () => {
101101
const { code, stderr } = await spawnPromisified(execPath, [
102102
'--experimental-package-map', packageMapPath,
103103
'--input-type=module',
104104
'--eval',
105105
`import dep from 'dep-a'; console.log(dep);`,
106106
], { cwd: '/tmp' }); // Not in any mapped package
107107

108-
// Should fall back to standard resolution (which will fail)
109108
assert.notStrictEqual(code, 0);
110-
assert.match(stderr, /Cannot find package/);
109+
assert.match(stderr, /ERR_MODULE_NOT_FOUND/);
111110
});
112111
});
113112

@@ -244,4 +243,56 @@ describe('ESM: --experimental-package-map', () => {
244243
assert.match(stdout, /pkg-value/);
245244
});
246245
});
246+
247+
// =========== External Package Paths ===========
248+
249+
describe('external package paths', () => {
250+
it('resolves packages outside the package map directory via relative paths', async () => {
251+
const { code, stdout, stderr } = await spawnPromisified(execPath, [
252+
'--experimental-package-map',
253+
fixtures.path('package-map/nested-project/package-map-external-deps.json'),
254+
'--input-type=module',
255+
'--eval',
256+
`import dep from 'dep-a'; console.log(dep);`,
257+
], { cwd: fixtures.path('package-map/nested-project/src') });
258+
259+
assert.strictEqual(code, 0, stderr);
260+
assert.match(stdout, /dep-a-value/);
261+
});
262+
});
263+
264+
// =========== Longest Path Wins ===========
265+
266+
describe('longest path wins', () => {
267+
const longestPathMap = fixtures.path('package-map/package-map-longest-path.json');
268+
269+
it('resolves nested package using its own dependencies, not the parent', async () => {
270+
// Inner lives at ./root/node_modules/inner which is inside root's
271+
// path (./root). The longest matching path should win, so code in
272+
// inner should resolve dep-a (inner's dep), not be treated as root.
273+
const { code, stdout, stderr } = await spawnPromisified(execPath, [
274+
'--experimental-package-map', longestPathMap,
275+
'--input-type=module',
276+
'--eval',
277+
`import inner from 'inner'; console.log(inner);`,
278+
], { cwd: fixtures.path('package-map/root') });
279+
280+
assert.strictEqual(code, 0, stderr);
281+
assert.match(stdout, /inner using dep-a-value/);
282+
});
283+
284+
it('denies access to nested package deps from parent package', async () => {
285+
// Root does not list dep-a in its dependencies, so importing it
286+
// from root should fail even though inner (nested inside root) can.
287+
const { code, stderr } = await spawnPromisified(execPath, [
288+
'--experimental-package-map', longestPathMap,
289+
'--input-type=module',
290+
'--eval',
291+
`import dep from 'dep-a';`,
292+
], { cwd: fixtures.path('package-map/root') });
293+
294+
assert.notStrictEqual(code, 0);
295+
assert.match(stderr, /ERR_PACKAGE_MAP_ACCESS_DENIED/);
296+
});
297+
});
247298
});
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"packages": {
3+
"app": {
4+
"name": "app",
5+
"path": "./src",
6+
"dependencies": ["dep-a"]
7+
},
8+
"dep-a": {
9+
"name": "dep-a",
10+
"path": "../dep-a",
11+
"dependencies": []
12+
}
13+
}
14+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"packages": {
3+
"root": {
4+
"name": "root",
5+
"path": "./root",
6+
"dependencies": ["inner"]
7+
},
8+
"inner": {
9+
"name": "inner",
10+
"path": "./root/node_modules/inner",
11+
"dependencies": ["dep-a"]
12+
},
13+
"dep-a": {
14+
"name": "dep-a",
15+
"path": "./dep-a",
16+
"dependencies": []
17+
}
18+
}
19+
}

test/fixtures/package-map/root/node_modules/inner/index.cjs

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/package-map/root/node_modules/inner/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/package-map/root/node_modules/inner/package.json

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

test/parallel/test-require-package-map.js

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ describe('CJS: --experimental-package-map', () => {
7070
});
7171
});
7272

73-
describe('fallback behavior', () => {
73+
describe('resolution boundaries', () => {
7474
it('falls back for builtin modules', () => {
7575
const { status, stdout, stderr } = spawnSync(process.execPath, [
7676
'--experimental-package-map', packageMapPath,
@@ -85,7 +85,7 @@ describe('CJS: --experimental-package-map', () => {
8585
assert.match(stdout, /function/);
8686
});
8787

88-
it('falls back when parent not in map', () => {
88+
it('throws when parent not in map', () => {
8989
const { status, stderr } = spawnSync(process.execPath, [
9090
'--experimental-package-map', packageMapPath,
9191
'-e',
@@ -95,7 +95,6 @@ describe('CJS: --experimental-package-map', () => {
9595
encoding: 'utf8',
9696
});
9797

98-
// Should fall back to standard resolution (which will fail)
9998
assert.notStrictEqual(status, 0);
10099
assert.match(stderr, /Cannot find module/);
101100
});
@@ -182,4 +181,58 @@ describe('CJS: --experimental-package-map', () => {
182181
assert.match(stdout, /pkg-value/);
183182
});
184183
});
184+
185+
describe('external package paths', () => {
186+
it('resolves packages outside the package map directory via relative paths', () => {
187+
const { status, stdout, stderr } = spawnSync(process.execPath, [
188+
'--experimental-package-map',
189+
fixtures.path('package-map/nested-project/package-map-external-deps.json'),
190+
'-e',
191+
`const dep = require('dep-a'); console.log(dep.default);`,
192+
], {
193+
cwd: fixtures.path('package-map/nested-project/src'),
194+
encoding: 'utf8',
195+
});
196+
197+
assert.strictEqual(status, 0, stderr);
198+
assert.match(stdout, /dep-a-value/);
199+
});
200+
});
201+
202+
describe('longest path wins', () => {
203+
const longestPathMap = fixtures.path('package-map/package-map-longest-path.json');
204+
205+
it('resolves nested package using its own dependencies, not the parent', () => {
206+
// Inner lives at ./root/node_modules/inner which is inside root's
207+
// path (./root). The longest matching path should win, so code in
208+
// inner should resolve dep-a (inner's dep), not be treated as root.
209+
const { status, stdout, stderr } = spawnSync(process.execPath, [
210+
'--experimental-package-map', longestPathMap,
211+
'-e',
212+
`const inner = require('inner'); console.log(inner.default);`,
213+
], {
214+
cwd: fixtures.path('package-map/root'),
215+
encoding: 'utf8',
216+
});
217+
218+
assert.strictEqual(status, 0, stderr);
219+
assert.match(stdout, /inner using dep-a-value/);
220+
});
221+
222+
it('denies access to nested package deps from parent package', () => {
223+
// Root does not list dep-a in its dependencies, so requiring it
224+
// from root should fail even though inner (nested inside root) can.
225+
const { status, stderr } = spawnSync(process.execPath, [
226+
'--experimental-package-map', longestPathMap,
227+
'-e',
228+
`require('dep-a');`,
229+
], {
230+
cwd: fixtures.path('package-map/root'),
231+
encoding: 'utf8',
232+
});
233+
234+
assert.notStrictEqual(status, 0);
235+
assert.match(stderr, /ERR_PACKAGE_MAP_ACCESS_DENIED/);
236+
});
237+
});
185238
});

0 commit comments

Comments
 (0)