GeezerVison is showing github.com — module: self resolve bug fix and esm ordering · nodejs/node@801ea12
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file 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 number | Diff line number | Diff line change |
|---|
@@ -1209,6 +1209,9 @@ _defaultEnv_ is the conditional environment name priority array, | 1209 | 1209 | > 1. If _packageSubpath_ contains any _"."_ or _".."_ segments or percent | 1210 | 1210 | > encoded strings for _"/"_ or _"\\"_, then | 1211 | 1211 | > 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_. | 1212 | 1215 | > 1. If _packageSubpath_ is _undefined_ and _packageName_ is a Node.js builtin | 1213 | 1216 | > module, then | 1214 | 1217 | > 1. Return the string _"node:"_ concatenated with _packageSpecifier_. | @@ -1230,30 +1233,27 @@ _defaultEnv_ is the conditional environment name priority array, | 1230 | 1233 | > 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, | 1231 | 1234 | > _packageSubpath_, _pjson.exports_). | 1232 | 1235 | > 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_. | 1236 | 1236 | > 1. Throw a _Module Not Found_ error. | 1237 | 1237 |
| 1238 | | -**SELF_REFERENCE_RESOLVE**(_specifier_, _parentURL_) | | 1238 | +**SELF_REFERENCE_RESOLVE**(_packageName_, _packageSubpath_, _parentURL_) | 1239 | 1239 |
| 1240 | 1240 | > 1. Let _packageURL_ be the result of **READ_PACKAGE_SCOPE**(_parentURL_). | 1241 | 1241 | > 1. If _packageURL_ is **null**, then | 1242 | | -> 1. Return an empty result. | | 1242 | +> 1. Return **undefined**. | 1243 | 1243 | > 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**. | 1257 | 1257 |
| 1258 | 1258 | **PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_) | 1259 | 1259 |
| |
Collapse file 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 number | Diff line number | Diff line change |
|---|
@@ -160,9 +160,9 @@ require(X) from module at path Y | 160 | 160 | a. LOAD_AS_FILE(Y + X) | 161 | 161 | b. LOAD_AS_DIRECTORY(Y + X) | 162 | 162 | 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" | 166 | 166 |
| 167 | 167 | LOAD_AS_FILE(X) | 168 | 168 | 1. If X is a file, load X as JavaScript text. STOP | @@ -205,9 +205,10 @@ NODE_MODULES_PATHS(START) | 205 | 205 |
| 206 | 206 | LOAD_SELF_REFERENCE(X, START) | 207 | 207 | 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 | 211 | 212 | was loaded via `LOAD_NODE_MODULES` with a name in `package.json`. | 212 | 213 | ``` | 213 | 214 |
| |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js +39-34Lines changed: 39 additions & 34 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -420,13 +420,13 @@ function resolveBasePath(basePath, exts, isMain, trailingSlash, request) { | 420 | 420 | return filename; | 421 | 421 | } | 422 | 422 |
| 423 | | -function trySelf(paths, exts, isMain, trailingSlash, request) { | | 423 | +function trySelf(parentPath, isMain, request) { | 424 | 424 | if (!experimentalSelf) { | 425 | 425 | return false; | 426 | 426 | } | 427 | 427 |
| 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; | 430 | 430 | if (typeof pkg.name !== 'string') return false; | 431 | 431 |
| 432 | 432 | let expansion; | @@ -438,12 +438,15 @@ function trySelf(paths, exts, isMain, trailingSlash, request) { | 438 | 438 | return false; | 439 | 439 | } | 440 | 440 |
| 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 | + } | 447 | 450 | return resolveBasePath(fromExports, exts, isMain, trailingSlash, request); | 448 | 451 | } else { | 449 | 452 | // Use main field | @@ -691,13 +694,6 @@ Module._findPath = function(request, paths, isMain) { | 691 | 694 | } | 692 | 695 | } | 693 | 696 |
| 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 | | - | 701 | 697 | return false; | 702 | 698 | }; | 703 | 699 |
| @@ -941,26 +937,35 @@ Module._resolveFilename = function(request, parent, isMain, options) { | 941 | 937 | paths = Module._resolveLookupPaths(request, parent); | 942 | 938 | } | 943 | 939 |
| 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; | 956 | 948 | } | 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; | 962 | 949 | } | 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; | 964 | 969 | }; | 965 | 970 |
| 966 | 971 |
| |
Collapse file Copy file name to clipboardExpand all lines: src/module_wrap.cc +14-35Lines changed: 14 additions & 35 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1150,12 +1150,12 @@ Maybe<URL> PackageExportsResolve(Environment* env, | 1150 | 1150 | } | 1151 | 1151 |
| 1152 | 1152 | Maybe<URL> ResolveSelf(Environment* env, | 1153 | | - const std::string& specifier, | | 1153 | + const std::string& pkg_name, | | 1154 | + const std::string& pkg_subpath, | 1154 | 1155 | const URL& base) { | 1155 | 1156 | if (!env->options()->experimental_resolve_self) { | 1156 | 1157 | return Nothing<URL>(); | 1157 | 1158 | } | 1158 | | - | 1159 | 1159 | const PackageConfig* pcfg; | 1160 | 1160 | if (GetPackageScopeConfig(env, base, base).To(&pcfg) && | 1161 | 1161 | pcfg->exists == Exists::Yes) { | @@ -1171,34 +1171,12 @@ Maybe<URL> ResolveSelf(Environment* env, | 1171 | 1171 | found_pjson = true; | 1172 | 1172 | } | 1173 | 1173 | } | 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()) { | 1195 | 1177 | 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); | 1202 | 1180 | } | 1203 | 1181 | } | 1204 | 1182 |
| @@ -1245,6 +1223,13 @@ Maybe<URL> PackageResolve(Environment* env, | 1245 | 1223 | } else { | 1246 | 1224 | pkg_subpath = "." + specifier.substr(sep_index); | 1247 | 1225 | } | | 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 | + | 1248 | 1233 | URL pjson_url("./node_modules/" + pkg_name + "/package.json", &base); | 1249 | 1234 | std::string pjson_path = pjson_url.ToFilePath(); | 1250 | 1235 | std::string last_path; | @@ -1278,12 +1263,6 @@ Maybe<URL> PackageResolve(Environment* env, | 1278 | 1263 | // Cross-platform root check. | 1279 | 1264 | } while (pjson_path.length() != last_path.length()); | 1280 | 1265 |
| 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 | | - | 1287 | 1266 | std::string msg = "Cannot find package '" + pkg_name + | 1288 | 1267 | "' imported from " + base.ToFilePath(); | 1289 | 1268 | node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); | |
Collapse file Copy file name to clipboardExpand all lines: src/node_file.cc +1Lines changed: 1 addition & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -783,6 +783,7 @@ static void InternalModuleReadJSON(const FunctionCallbackInfo<Value>& args) { | 783 | 783 |
| 784 | 784 | const size_t size = offset - start; | 785 | 785 | if (size == 0 || ( | | 786 | + size == SearchString(&chars[start], size, "\"name\"") && | 786 | 787 | size == SearchString(&chars[start], size, "\"main\"") && | 787 | 788 | size == SearchString(&chars[start], size, "\"exports\"") && | 788 | 789 | size == SearchString(&chars[start], size, "\"type\""))) { | |
Collapse file Copy file name to clipboardExpand all lines: test/es-module/test-esm-exports.mjs +6-3Lines changed: 6 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -23,8 +23,6 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; | 23 | 23 | // Fallbacks | 24 | 24 | ['pkgexports/fallbackdir/asdf.js', { default: 'asdf' }], | 25 | 25 | ['pkgexports/fallbackfile', { default: 'asdf' }], | 26 | | - // Dot main | 27 | | - ['pkgexports', { default: 'asdf' }], | 28 | 26 | // Conditional split for require | 29 | 27 | ['pkgexports/condition', isRequire ? { default: 'encoded path' } : | 30 | 28 | { default: 'asdf' }], | @@ -33,6 +31,11 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; | 33 | 31 | // Conditional object exports sugar | 34 | 32 | ['pkgexports-sugar2', isRequire ? { default: 'not-exported' } : | 35 | 33 | { 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' }] | 36 | 39 | ]); | 37 | 40 |
| 38 | 41 | for (const [validSpecifier, expected] of validSpecifiers) { | @@ -140,7 +143,7 @@ const { requireFromInside, importFromInside } = fromInside; | 140 | 143 | // A file not visible from outside of the package | 141 | 144 | ['../not-exported.js', { default: 'not-exported' }], | 142 | 145 | // Part of the public interface | 143 | | - ['@pkgexports/name/valid-cjs', { default: 'asdf' }], | | 146 | + ['pkgexports/valid-cjs', { default: 'asdf' }], | 144 | 147 | ]); | 145 | 148 | for (const [validSpecifier, expected] of validSpecifiers) { | 146 | 149 | if (validSpecifier === null) continue; | |
Collapse file Copy file name to clipboardExpand all lines: test/fixtures/node_modules/pkgexports-main/package.json +1-1Lines changed: 1 addition & 1 deletion Collapse file Copy file name to clipboardExpand all lines: test/fixtures/node_modules/pkgexports-sugar/main.js +9Lines changed: 9 additions & 0 deletions Collapse file Copy file name to clipboardExpand all lines: test/fixtures/node_modules/pkgexports-sugar/package.json +1Lines changed: 1 addition & 0 deletions Collapse file Copy file name to clipboardExpand all lines: test/fixtures/node_modules/pkgexports/package.json +6-3Lines changed: 6 additions & 3 deletions
|