Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d9084d2

Browse filesBrowse files
hybristtargos
hybrist
authored and
targos
committed
module: unify package exports test for CJS and ESM
Refs: nodejs/modules#358 PR-URL: #28831 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent 5f07f49 commit d9084d2
Copy full SHA for d9084d2

File tree

Expand file treeCollapse file tree

6 files changed

+102
-90
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+102
-90
lines changed
Open diff view settings
Collapse file

‎src/module_wrap.cc‎

Copy file name to clipboardExpand all lines: src/module_wrap.cc
+5-2Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -847,8 +847,11 @@ Maybe<URL> PackageExportsResolve(Environment* env,
847847
std::string target(*target_utf8, target_utf8.length());
848848
if (target.back() == '/' && target.substr(0, 2) == "./") {
849849
std::string subpath = pkg_subpath.substr(best_match_str.length());
850-
URL target_url(target + subpath, pjson_url);
851-
return FinalizeResolution(env, target_url, base);
850+
URL url_prefix(target, pjson_url);
851+
URL target_url(subpath, url_prefix);
852+
if (target_url.path().find(url_prefix.path()) == 0) {
853+
return FinalizeResolution(env, target_url, base);
854+
}
852855
}
853856
}
854857
}
Collapse file

‎test/es-module/test-esm-exports.mjs‎

Copy file name to clipboard
+85-26Lines changed: 85 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,88 @@
11
// Flags: --experimental-modules
22

33
import { mustCall } from '../common/index.mjs';
4-
import { ok, strictEqual } from 'assert';
5-
6-
import { asdf, asdf2, space } from '../fixtures/pkgexports.mjs';
7-
import {
8-
loadMissing,
9-
loadFromNumber,
10-
loadDot,
11-
} from '../fixtures/pkgexports-missing.mjs';
12-
13-
strictEqual(asdf, 'asdf');
14-
strictEqual(asdf2, 'asdf');
15-
strictEqual(space, 'encoded path');
16-
17-
loadMissing().catch(mustCall((err) => {
18-
ok(err.message.toString().startsWith('Package exports'));
19-
ok(err.message.toString().indexOf('do not define a \'./missing\' subpath'));
20-
}));
21-
22-
loadFromNumber().catch(mustCall((err) => {
23-
ok(err.message.toString().startsWith('Package exports'));
24-
ok(err.message.toString().indexOf('do not define a \'./missing\' subpath'));
25-
}));
26-
27-
loadDot().catch(mustCall((err) => {
28-
ok(err.message.toString().startsWith('Cannot find main entry point'));
29-
}));
4+
import { ok, deepStrictEqual, strictEqual } from 'assert';
5+
6+
import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
7+
8+
[requireFixture, importFixture].forEach((loadFixture) => {
9+
const isRequire = loadFixture === requireFixture;
10+
11+
const validSpecifiers = new Map([
12+
// A simple mapping of a path.
13+
['pkgexports/valid-cjs', { default: 'asdf' }],
14+
// A directory mapping, pointing to the package root.
15+
['pkgexports/sub/asdf.js', { default: 'asdf' }],
16+
// A mapping pointing to a file that needs special encoding (%20) in URLs.
17+
['pkgexports/space', { default: 'encoded path' }],
18+
// Verifying that normal packages still work with exports turned on.
19+
isRequire ? ['baz/index', { default: 'eye catcher' }] : [null],
20+
]);
21+
for (const [validSpecifier, expected] of validSpecifiers) {
22+
if (validSpecifier === null) continue;
23+
24+
loadFixture(validSpecifier)
25+
.then(mustCall((actual) => {
26+
deepStrictEqual({ ...actual }, expected);
27+
}));
28+
}
29+
30+
// There's no such export - so there's nothing to do.
31+
loadFixture('pkgexports/missing').catch(mustCall((err) => {
32+
strictEqual(err.code, 'ERR_PATH_NOT_EXPORTED');
33+
assertStartsWith(err.message, 'Package exports');
34+
assertIncludes(err.message, 'do not define a \'./missing\' subpath');
35+
}));
36+
37+
// The file exists but isn't exported. The exports is a number which counts
38+
// as a non-null value without any properties, just like `{}`.
39+
loadFixture('pkgexports-number/hidden.js').catch(mustCall((err) => {
40+
strictEqual(err.code, 'ERR_PATH_NOT_EXPORTED');
41+
assertStartsWith(err.message, 'Package exports');
42+
assertIncludes(err.message, 'do not define a \'./hidden.js\' subpath');
43+
}));
44+
45+
// There's no main field so we won't find anything when importing the name.
46+
// The fact that "." is mapped is ignored, it's not a valid main config.
47+
loadFixture('pkgexports').catch(mustCall((err) => {
48+
if (isRequire) {
49+
strictEqual(err.code, 'MODULE_NOT_FOUND');
50+
assertStartsWith(err.message, 'Cannot find module \'pkgexports\'');
51+
} else {
52+
strictEqual(err.code, 'ERR_MODULE_NOT_FOUND');
53+
assertStartsWith(err.message, 'Cannot find main entry point');
54+
}
55+
}));
56+
57+
// Even though 'pkgexports/sub/asdf.js' works, alternate "path-like" variants
58+
// do not to prevent confusion and accidental loopholes.
59+
loadFixture('pkgexports/sub/./../asdf.js').catch(mustCall((err) => {
60+
strictEqual(err.code, 'ERR_PATH_NOT_EXPORTED');
61+
assertStartsWith(err.message, 'Package exports');
62+
assertIncludes(err.message,
63+
'do not define a \'./sub/./../asdf.js\' subpath');
64+
}));
65+
66+
// Covering out bases - not a file is still not a file after dir mapping.
67+
loadFixture('pkgexports/sub/not-a-file.js').catch(mustCall((err) => {
68+
if (isRequire) {
69+
strictEqual(err.code, 'MODULE_NOT_FOUND');
70+
assertStartsWith(err.message,
71+
'Cannot find module \'pkgexports/sub/not-a-file.js\'');
72+
} else {
73+
strictEqual(err.code, 'ERR_MODULE_NOT_FOUND');
74+
// ESM currently returns a full file path
75+
assertStartsWith(err.message, 'Cannot find module');
76+
}
77+
}));
78+
});
79+
80+
function assertStartsWith(actual, expected) {
81+
const start = actual.toString().substr(0, expected.length);
82+
strictEqual(start, expected);
83+
}
84+
85+
function assertIncludes(actual, expected) {
86+
ok(actual.toString().indexOf(expected),
87+
`${JSON.stringify(actual)} includes ${JSON.stringify(expected)}`);
88+
}
Collapse file

‎test/fixtures/node_modules/pkgexports/package.json‎

Copy file name to clipboardExpand all lines: test/fixtures/node_modules/pkgexports/package.json
-1Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/pkgexports-missing.mjs‎

Copy file name to clipboardExpand all lines: test/fixtures/pkgexports-missing.mjs
-11Lines changed: 0 additions & 11 deletions
This file was deleted.
Collapse file

‎test/fixtures/pkgexports.mjs‎

Copy file name to clipboard
+12-3Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1-
export { default as asdf } from 'pkgexports/asdf';
2-
export { default as asdf2 } from 'pkgexports/sub/asdf.js';
3-
export { default as space } from 'pkgexports/space';
1+
import { fileURLToPath } from 'url';
2+
import { createRequire } from 'module';
3+
4+
const rawRequire = createRequire(fileURLToPath(import.meta.url));
5+
6+
export async function requireFixture(specifier) {
7+
return { default: rawRequire(specifier ) };
8+
}
9+
10+
export function importFixture(specifier) {
11+
return import(specifier);
12+
}
Collapse file

‎test/parallel/test-module-package-exports.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-module-package-exports.js
-47Lines changed: 0 additions & 47 deletions
This file was deleted.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.