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

module: self resolve bug fix and esm ordering · nodejs/node@bd047e8 · GitHub

/ node Public

Commit bd047e8

Browse files
guybedford
authored and
BridgeAR
committed
module: self resolve bug fix and esm ordering
PR-URL: #31009 Reviewed-By: Jan Krems <jan.krems@gmail.com>
1 parent 4f32bbb commit bd047e8

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
@@ -1207,6 +1207,9 @@ _defaultEnv_ is the conditional environment name priority array,
12071207
> 1. If _packageSubpath_ contains any _"."_ or _".."_ segments or percent
12081208
> encoded strings for _"/"_ or _"\\"_, then
12091209
> 1. Throw an _Invalid Specifier_ error.
1210+
> 1. Set _selfUrl_ to the result of
1211+
> **SELF_REFERENCE_RESOLVE**(_packageName_, _packageSubpath_, _parentURL_).
1212+
> 1. If _selfUrl_ isn't empty, return _selfUrl_.
12101213
> 1. If _packageSubpath_ is _undefined_ and _packageName_ is a Node.js builtin
12111214
> module, then
12121215
> 1. Return the string _"node:"_ concatenated with _packageSpecifier_.
@@ -1228,30 +1231,27 @@ _defaultEnv_ is the conditional environment name priority array,
12281231
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_,
12291232
> _packageSubpath_, _pjson.exports_).
12301233
> 1. Return the URL resolution of _packageSubpath_ in _packageURL_.
1231-
> 1. Set _selfUrl_ to the result of
1232-
> **SELF_REFERENCE_RESOLE**(_packageSpecifier_, _parentURL_).
1233-
> 1. If _selfUrl_ isn't empty, return _selfUrl_.
12341234
> 1. Throw a _Module Not Found_ error.
12351235
1236-
**SELF_REFERENCE_RESOLVE**(_specifier_, _parentURL_)
1236+
**SELF_REFERENCE_RESOLVE**(_packageName_, _packageSubpath_, _parentURL_)
12371237
12381238
> 1. Let _packageURL_ be the result of **READ_PACKAGE_SCOPE**(_parentURL_).
12391239
> 1. If _packageURL_ is **null**, then
1240-
> 1. Return an empty result.
1240+
> 1. Return **undefined**.
12411241
> 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_packageURL_).
1242-
> 1. Set _name_ to _pjson.name_.
1243-
> 1. If _name_ is empty, then return an empty result.
1244-
> 1. If _name_ is equal to _specifier_, then
1245-
> 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_).
1246-
> 1. If _specifier_ starts with _name_ followed by "/", then
1247-
> 1. Set _subpath_ to everything after the "/".
1248-
> 1. If _pjson_ is not **null** and _pjson_ has an _"exports"_ key, then
1249-
> 1. Let _exports_ be _pjson.exports_.
1250-
> 1. If _exports_ is not **null** or **undefined**, then
1251-
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, _subpath_,
1252-
> _pjson.exports_).
1253-
> 1. Return the URL resolution of _subpath_ in _packageURL_.
1254-
> 1. Otherwise return an empty result.
1242+
> 1. If _pjson_ does not include an _"exports"_ property, then
1243+
> 1. Return **undefined**.
1244+
> 1. If _pjson.name_ is equal to _packageName_, then
1245+
> 1. If _packageSubpath_ is _undefined_, then
1246+
> 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_).
1247+
> 1. Otherwise,
1248+
> 1. If _pjson_ is not **null** and _pjson_ has an _"exports"_ key, then
1249+
> 1. Let _exports_ be _pjson.exports_.
1250+
> 1. If _exports_ is not **null** or **undefined**, then
1251+
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, _subpath_,
1252+
> _pjson.exports_).
1253+
> 1. Return the URL resolution of _subpath_ in _packageURL_.
1254+
> 1. Otherwise, return **undefined**.
12551255
12561256
**PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_)
12571257

‎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
@@ -428,13 +428,13 @@ function resolveBasePath(basePath, exts, isMain, trailingSlash, request) {
428428
return filename;
429429
}
430430

431-
function trySelf(paths, exts, isMain, trailingSlash, request) {
431+
function trySelf(parentPath, isMain, request) {
432432
if (!experimentalSelf) {
433433
return false;
434434
}
435435

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

440440
let expansion;
@@ -446,12 +446,15 @@ function trySelf(paths, exts, isMain, trailingSlash, request) {
446446
return false;
447447
}
448448

449-
if (exts === undefined)
450-
exts = ObjectKeys(Module._extensions);
451-
452-
if (expansion) {
453-
// Use exports
454-
const fromExports = applyExports(basePath, expansion);
449+
const exts = ObjectKeys(Module._extensions);
450+
const fromExports = applyExports(basePath, expansion);
451+
// Use exports
452+
if (fromExports) {
453+
let trailingSlash = request.length > 0 &&
454+
request.charCodeAt(request.length - 1) === CHAR_FORWARD_SLASH;
455+
if (!trailingSlash) {
456+
trailingSlash = /(?:^|\/)\.?\.$/.test(request);
457+
}
455458
return resolveBasePath(fromExports, exts, isMain, trailingSlash, request);
456459
} else {
457460
// Use main field
@@ -699,13 +702,6 @@ Module._findPath = function(request, paths, isMain) {
699702
}
700703
}
701704

702-
const selfFilename = trySelf(paths, exts, isMain, trailingSlash, request);
703-
if (selfFilename) {
704-
emitExperimentalWarning('Package name self resolution');
705-
Module._pathCache[cacheKey] = selfFilename;
706-
return selfFilename;
707-
}
708-
709705
return false;
710706
};
711707

@@ -949,26 +945,35 @@ Module._resolveFilename = function(request, parent, isMain, options) {
949945
paths = Module._resolveLookupPaths(request, parent);
950946
}
951947

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

974979

‎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
@@ -829,6 +829,7 @@ static void InternalModuleReadJSON(const FunctionCallbackInfo<Value>& args) {
829829

830830
const size_t size = offset - start;
831831
if (size == 0 || (
832+
size == SearchString(&chars[start], size, "\"name\"") &&
832833
size == SearchString(&chars[start], size, "\"main\"") &&
833834
size == SearchString(&chars[start], size, "\"exports\"") &&
834835
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
@@ -22,8 +22,6 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
2222
// Fallbacks
2323
['pkgexports/fallbackdir/asdf.js', { default: 'asdf' }],
2424
['pkgexports/fallbackfile', { default: 'asdf' }],
25-
// Dot main
26-
['pkgexports', { default: 'asdf' }],
2725
// Conditional split for require
2826
['pkgexports/condition', isRequire ? { default: 'encoded path' } :
2927
{ default: 'asdf' }],
@@ -32,6 +30,11 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
3230
// Conditional object exports sugar
3331
['pkgexports-sugar2', isRequire ? { default: 'not-exported' } :
3432
{ default: 'main' }],
33+
// Resolve self
34+
['pkgexports/resolve-self', isRequire ?
35+
{ default: 'self-cjs' } : { default: 'self-mjs' }],
36+
// Resolve self sugar
37+
['pkgexports-sugar', { default: 'main' }]
3538
]);
3639

3740
for (const [validSpecifier, expected] of validSpecifiers) {
@@ -139,7 +142,7 @@ const { requireFromInside, importFromInside } = fromInside;
139142
// A file not visible from outside of the package
140143
['../not-exported.js', { default: 'not-exported' }],
141144
// Part of the public interface
142-
['@pkgexports/name/valid-cjs', { default: 'asdf' }],
145+
['pkgexports/valid-cjs', { default: 'asdf' }],
143146
]);
144147
for (const [validSpecifier, expected] of validSpecifiers) {
145148
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)
Back | FazBrowse Home | New Git URL