Skip to content

Commit 801ea12

Browse files
guybedfordMylesBorins
guybedford
authored and
MylesBorins
committed
module: self resolve bug fix and esm ordering
PR-URL: #31009 Reviewed-By: Jan Krems <[email protected]>
1 parent 5b4db8e commit 801ea12

12 files changed

Lines changed: 106 additions & 101 deletions

File tree

‎doc/api/esm.md‎

Lines changed: 18 additions & 18 deletions
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

‎doc/api/modules.md‎

Lines changed: 7 additions & 6 deletions
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

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

Lines 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

‎src/module_wrap.cc‎

Lines 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());

‎src/node_file.cc‎

Lines 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\""))) {

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

Lines 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;

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

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

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

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

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

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

Lines 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)