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 801ea12

Browse filesBrowse files
guybedfordMylesBorins
guybedford
authored and
MylesBorins
committed
module: self resolve bug fix and esm ordering
PR-URL: #31009 Reviewed-By: Jan Krems <jan.krems@gmail.com>
1 parent 5b4db8e commit 801ea12
Copy full SHA for 801ea12

File tree

Expand file treeCollapse file tree

12 files changed

+106
-101
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

12 files changed

+106
-101
lines changed
Open diff view settings
Collapse file

‎doc/api/esm.md‎

Copy file name to clipboardExpand all lines: doc/api/esm.md
+18-18Lines changed: 18 additions & 18 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,9 @@ _defaultEnv_ is the conditional environment name priority array,
12091209
> 1. If _packageSubpath_ contains any _"."_ or _".."_ segments or percent
12101210
> encoded strings for _"/"_ or _"\\"_, then
12111211
> 1. Throw an _Invalid Specifier_ error.
1212+
> 1. Set _selfUrl_ to the result of
1213+
> **SELF_REFERENCE_RESOLVE**(_packageName_, _packageSubpath_, _parentURL_).
1214+
> 1. If _selfUrl_ isn't empty, return _selfUrl_.
12121215
> 1. If _packageSubpath_ is _undefined_ and _packageName_ is a Node.js builtin
12131216
> module, then
12141217
> 1. Return the string _"node:"_ concatenated with _packageSpecifier_.
@@ -1230,30 +1233,27 @@ _defaultEnv_ is the conditional environment name priority array,
12301233
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_,
12311234
> _packageSubpath_, _pjson.exports_).
12321235
> 1. Return the URL resolution of _packageSubpath_ in _packageURL_.
1233-
> 1. Set _selfUrl_ to the result of
1234-
> **SELF_REFERENCE_RESOLE**(_packageSpecifier_, _parentURL_).
1235-
> 1. If _selfUrl_ isn't empty, return _selfUrl_.
12361236
> 1. Throw a _Module Not Found_ error.
12371237
1238-
**SELF_REFERENCE_RESOLVE**(_specifier_, _parentURL_)
1238+
**SELF_REFERENCE_RESOLVE**(_packageName_, _packageSubpath_, _parentURL_)
12391239
12401240
> 1. Let _packageURL_ be the result of **READ_PACKAGE_SCOPE**(_parentURL_).
12411241
> 1. If _packageURL_ is **null**, then
1242-
> 1. Return an empty result.
1242+
> 1. Return **undefined**.
12431243
> 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_packageURL_).
1244-
> 1. Set _name_ to _pjson.name_.
1245-
> 1. If _name_ is empty, then return an empty result.
1246-
> 1. If _name_ is equal to _specifier_, then
1247-
> 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_).
1248-
> 1. If _specifier_ starts with _name_ followed by "/", then
1249-
> 1. Set _subpath_ to everything after the "/".
1250-
> 1. If _pjson_ is not **null** and _pjson_ has an _"exports"_ key, then
1251-
> 1. Let _exports_ be _pjson.exports_.
1252-
> 1. If _exports_ is not **null** or **undefined**, then
1253-
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, _subpath_,
1254-
> _pjson.exports_).
1255-
> 1. Return the URL resolution of _subpath_ in _packageURL_.
1256-
> 1. Otherwise return an empty result.
1244+
> 1. If _pjson_ does not include an _"exports"_ property, then
1245+
> 1. Return **undefined**.
1246+
> 1. If _pjson.name_ is equal to _packageName_, then
1247+
> 1. If _packageSubpath_ is _undefined_, then
1248+
> 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_).
1249+
> 1. Otherwise,
1250+
> 1. If _pjson_ is not **null** and _pjson_ has an _"exports"_ key, then
1251+
> 1. Let _exports_ be _pjson.exports_.
1252+
> 1. If _exports_ is not **null** or **undefined**, then
1253+
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, _subpath_,
1254+
> _pjson.exports_).
1255+
> 1. Return the URL resolution of _subpath_ in _packageURL_.
1256+
> 1. Otherwise, return **undefined**.
12571257
12581258
**PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_)
12591259
Collapse file

‎doc/api/modules.md‎

Copy file name to clipboardExpand all lines: doc/api/modules.md
+7-6Lines changed: 7 additions & 6 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,9 @@ require(X) from module at path Y
160160
a. LOAD_AS_FILE(Y + X)
161161
b. LOAD_AS_DIRECTORY(Y + X)
162162
c. THROW "not found"
163-
4. LOAD_NODE_MODULES(X, dirname(Y))
164-
5. LOAD_SELF_REFERENCE(X, dirname(Y))
165-
6. THROW "not found"
163+
4. LOAD_SELF_REFERENCE(X, dirname(Y))
164+
5. LOAD_NODE_MODULES(X, dirname(Y))
165+
7. THROW "not found"
166166
167167
LOAD_AS_FILE(X)
168168
1. If X is a file, load X as JavaScript text. STOP
@@ -205,9 +205,10 @@ NODE_MODULES_PATHS(START)
205205
206206
LOAD_SELF_REFERENCE(X, START)
207207
1. Find the closest package scope to START.
208-
2. If no scope was found, throw "not found".
209-
3. If the name in `package.json` isn't a prefix of X, throw "not found".
210-
4. Otherwise, resolve the remainder of X relative to this package as if it
208+
2. If no scope was found, return.
209+
3. If the `package.json` has no "exports", return.
210+
4. If the name in `package.json` isn't a prefix of X, throw "not found".
211+
5. Otherwise, resolve the remainder of X relative to this package as if it
211212
was loaded via `LOAD_NODE_MODULES` with a name in `package.json`.
212213
```
213214

Collapse file

‎lib/internal/modules/cjs/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js
+39-34Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -420,13 +420,13 @@ function resolveBasePath(basePath, exts, isMain, trailingSlash, request) {
420420
return filename;
421421
}
422422

423-
function trySelf(paths, exts, isMain, trailingSlash, request) {
423+
function trySelf(parentPath, isMain, request) {
424424
if (!experimentalSelf) {
425425
return false;
426426
}
427427

428-
const { data: pkg, path: basePath } = readPackageScope(paths[0]);
429-
if (!pkg) return false;
428+
const { data: pkg, path: basePath } = readPackageScope(parentPath) || {};
429+
if (!pkg || 'exports' in pkg === false) return false;
430430
if (typeof pkg.name !== 'string') return false;
431431

432432
let expansion;
@@ -438,12 +438,15 @@ function trySelf(paths, exts, isMain, trailingSlash, request) {
438438
return false;
439439
}
440440

441-
if (exts === undefined)
442-
exts = Object.keys(Module._extensions);
443-
444-
if (expansion) {
445-
// Use exports
446-
const fromExports = applyExports(basePath, expansion);
441+
const exts = Object.keys(Module._extensions);
442+
const fromExports = applyExports(basePath, expansion);
443+
// Use exports
444+
if (fromExports) {
445+
let trailingSlash = request.length > 0 &&
446+
request.charCodeAt(request.length - 1) === CHAR_FORWARD_SLASH;
447+
if (!trailingSlash) {
448+
trailingSlash = /(?:^|\/)\.?\.$/.test(request);
449+
}
447450
return resolveBasePath(fromExports, exts, isMain, trailingSlash, request);
448451
} else {
449452
// Use main field
@@ -691,13 +694,6 @@ Module._findPath = function(request, paths, isMain) {
691694
}
692695
}
693696

694-
const selfFilename = trySelf(paths, exts, isMain, trailingSlash, request);
695-
if (selfFilename) {
696-
emitExperimentalWarning('Package name self resolution');
697-
Module._pathCache[cacheKey] = selfFilename;
698-
return selfFilename;
699-
}
700-
701697
return false;
702698
};
703699

@@ -941,26 +937,35 @@ Module._resolveFilename = function(request, parent, isMain, options) {
941937
paths = Module._resolveLookupPaths(request, parent);
942938
}
943939

944-
// Look up the filename first, since that's the cache key.
945-
const filename = Module._findPath(request, paths, isMain);
946-
if (!filename) {
947-
const requireStack = [];
948-
for (let cursor = parent;
949-
cursor;
950-
cursor = cursor.parent) {
951-
requireStack.push(cursor.filename || cursor.id);
952-
}
953-
let message = `Cannot find module '${request}'`;
954-
if (requireStack.length > 0) {
955-
message = message + '\nRequire stack:\n- ' + requireStack.join('\n- ');
940+
if (parent && parent.filename) {
941+
const filename = trySelf(parent.filename, isMain, request);
942+
if (filename) {
943+
emitExperimentalWarning('Package name self resolution');
944+
const cacheKey = request + '\x00' +
945+
(paths.length === 1 ? paths[0] : paths.join('\x00'));
946+
Module._pathCache[cacheKey] = filename;
947+
return filename;
956948
}
957-
// eslint-disable-next-line no-restricted-syntax
958-
const err = new Error(message);
959-
err.code = 'MODULE_NOT_FOUND';
960-
err.requireStack = requireStack;
961-
throw err;
962949
}
963-
return filename;
950+
951+
// Look up the filename first, since that's the cache key.
952+
const filename = Module._findPath(request, paths, isMain, false);
953+
if (filename) return filename;
954+
const requireStack = [];
955+
for (let cursor = parent;
956+
cursor;
957+
cursor = cursor.parent) {
958+
requireStack.push(cursor.filename || cursor.id);
959+
}
960+
let message = `Cannot find module '${request}'`;
961+
if (requireStack.length > 0) {
962+
message = message + '\nRequire stack:\n- ' + requireStack.join('\n- ');
963+
}
964+
// eslint-disable-next-line no-restricted-syntax
965+
const err = new Error(message);
966+
err.code = 'MODULE_NOT_FOUND';
967+
err.requireStack = requireStack;
968+
throw err;
964969
};
965970

966971

Collapse file

‎src/module_wrap.cc‎

Copy file name to clipboardExpand all lines: src/module_wrap.cc
+14-35Lines changed: 14 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,12 +1150,12 @@ Maybe<URL> PackageExportsResolve(Environment* env,
11501150
}
11511151

11521152
Maybe<URL> ResolveSelf(Environment* env,
1153-
const std::string& specifier,
1153+
const std::string& pkg_name,
1154+
const std::string& pkg_subpath,
11541155
const URL& base) {
11551156
if (!env->options()->experimental_resolve_self) {
11561157
return Nothing<URL>();
11571158
}
1158-
11591159
const PackageConfig* pcfg;
11601160
if (GetPackageScopeConfig(env, base, base).To(&pcfg) &&
11611161
pcfg->exists == Exists::Yes) {
@@ -1171,34 +1171,12 @@ Maybe<URL> ResolveSelf(Environment* env,
11711171
found_pjson = true;
11721172
}
11731173
}
1174-
1175-
if (!found_pjson) {
1176-
return Nothing<URL>();
1177-
}
1178-
1179-
// "If specifier starts with pcfg name"
1180-
std::string subpath;
1181-
if (specifier.rfind(pcfg->name, 0)) {
1182-
// We know now: specifier is either equal to name or longer.
1183-
if (specifier == subpath) {
1184-
subpath = "";
1185-
} else if (specifier[pcfg->name.length()] == '/') {
1186-
// Return everything after the slash
1187-
subpath = "." + specifier.substr(pcfg->name.length() + 1);
1188-
} else {
1189-
// The specifier is neither the name of the package nor a subpath of it
1190-
return Nothing<URL>();
1191-
}
1192-
}
1193-
1194-
if (found_pjson && !subpath.length()) {
1174+
if (!found_pjson || pcfg->name != pkg_name) return Nothing<URL>();
1175+
if (pcfg->exports.IsEmpty()) return Nothing<URL>();
1176+
if (!pkg_subpath.length()) {
11951177
return PackageMainResolve(env, pjson_url, *pcfg, base);
1196-
} else if (found_pjson) {
1197-
if (!pcfg->exports.IsEmpty()) {
1198-
return PackageExportsResolve(env, pjson_url, subpath, *pcfg, base);
1199-
} else {
1200-
return FinalizeResolution(env, URL(subpath, pjson_url), base);
1201-
}
1178+
} else {
1179+
return PackageExportsResolve(env, pjson_url, pkg_subpath, *pcfg, base);
12021180
}
12031181
}
12041182

@@ -1245,6 +1223,13 @@ Maybe<URL> PackageResolve(Environment* env,
12451223
} else {
12461224
pkg_subpath = "." + specifier.substr(sep_index);
12471225
}
1226+
1227+
Maybe<URL> self_url = ResolveSelf(env, pkg_name, pkg_subpath, base);
1228+
if (self_url.IsJust()) {
1229+
ProcessEmitExperimentalWarning(env, "Package name self resolution");
1230+
return self_url;
1231+
}
1232+
12481233
URL pjson_url("./node_modules/" + pkg_name + "/package.json", &base);
12491234
std::string pjson_path = pjson_url.ToFilePath();
12501235
std::string last_path;
@@ -1278,12 +1263,6 @@ Maybe<URL> PackageResolve(Environment* env,
12781263
// Cross-platform root check.
12791264
} while (pjson_path.length() != last_path.length());
12801265

1281-
Maybe<URL> self_url = ResolveSelf(env, specifier, base);
1282-
if (self_url.IsJust()) {
1283-
ProcessEmitExperimentalWarning(env, "Package name self resolution");
1284-
return self_url;
1285-
}
1286-
12871266
std::string msg = "Cannot find package '" + pkg_name +
12881267
"' imported from " + base.ToFilePath();
12891268
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
Collapse file

‎src/node_file.cc‎

Copy file name to clipboardExpand all lines: src/node_file.cc
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,7 @@ static void InternalModuleReadJSON(const FunctionCallbackInfo<Value>& args) {
783783

784784
const size_t size = offset - start;
785785
if (size == 0 || (
786+
size == SearchString(&chars[start], size, "\"name\"") &&
786787
size == SearchString(&chars[start], size, "\"main\"") &&
787788
size == SearchString(&chars[start], size, "\"exports\"") &&
788789
size == SearchString(&chars[start], size, "\"type\""))) {
Collapse file

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

Copy file name to clipboardExpand all lines: test/es-module/test-esm-exports.mjs
+6-3Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
2323
// Fallbacks
2424
['pkgexports/fallbackdir/asdf.js', { default: 'asdf' }],
2525
['pkgexports/fallbackfile', { default: 'asdf' }],
26-
// Dot main
27-
['pkgexports', { default: 'asdf' }],
2826
// Conditional split for require
2927
['pkgexports/condition', isRequire ? { default: 'encoded path' } :
3028
{ default: 'asdf' }],
@@ -33,6 +31,11 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
3331
// Conditional object exports sugar
3432
['pkgexports-sugar2', isRequire ? { default: 'not-exported' } :
3533
{ default: 'main' }],
34+
// Resolve self
35+
['pkgexports/resolve-self', isRequire ?
36+
{ default: 'self-cjs' } : { default: 'self-mjs' }],
37+
// Resolve self sugar
38+
['pkgexports-sugar', { default: 'main' }]
3639
]);
3740

3841
for (const [validSpecifier, expected] of validSpecifiers) {
@@ -140,7 +143,7 @@ const { requireFromInside, importFromInside } = fromInside;
140143
// A file not visible from outside of the package
141144
['../not-exported.js', { default: 'not-exported' }],
142145
// Part of the public interface
143-
['@pkgexports/name/valid-cjs', { default: 'asdf' }],
146+
['pkgexports/valid-cjs', { default: 'asdf' }],
144147
]);
145148
for (const [validSpecifier, expected] of validSpecifiers) {
146149
if (validSpecifier === null) continue;
Collapse file

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

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

‎test/fixtures/node_modules/pkgexports-sugar/main.js‎

Copy file name to clipboardExpand all lines: test/fixtures/node_modules/pkgexports-sugar/main.js
+9Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

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

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

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

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

0 commit comments

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