diff --git a/doc/api/esm.md b/doc/api/esm.md index 07a3a71e90f0fd..ac1f154ff3e540 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -242,13 +242,13 @@ throw when an attempt is made to import them: ```js import submodule from 'es-module-package/private-module.js'; -// Throws - Package exports error +// Throws - Module not found ``` > Note: this is not a strong encapsulation as any private modules can still be > loaded by absolute paths. -Folders can also be mapped with package exports as well: +Folders can also be mapped with package exports: ```js @@ -268,8 +268,24 @@ import feature from 'es-module-package/features/x.js'; If a package has no exports, setting `"exports": false` can be used instead of `"exports": {}` to indicate the package does not intend for submodules to be exposed. -This is just a convention that works because `false`, just like `{}`, has no -iterable own properties. + +Any invalid exports entries will be ignored. This includes exports not +starting with `"./"` or a missing trailing `"/"` for directory exports. + +Array fallback support is provided for exports, similarly to import maps +in order to be forward-compatible with fallback workflows in future: + + +```js +{ + "exports": { + "./submodule": ["not:valid", "./submodule.js"] + } +} +``` + +Since `"not:valid"` is not a supported target, `"./submodule.js"` is used +instead as the fallback, as if it were the only target. ## import Specifiers @@ -660,7 +676,7 @@ CommonJS loader. Additional formats such as _"addon"_ can be extended in future updates. In the following algorithms, all subroutine errors are propagated as errors -of these top-level routines. +of these top-level routines unless stated otherwise. _isMain_ is **true** when resolving the Node.js application entry point. @@ -681,6 +697,9 @@ _isMain_ is **true** when resolving the Node.js application entry point. > 1. Note: _specifier_ is now a bare specifier. > 1. Set _resolvedURL_ the result of > **PACKAGE_RESOLVE**(_specifier_, _parentURL_). +> 1. If _resolvedURL_ contains any percent encodings of _"/"_ or _"\\"_ (_"%2f"_ +> and _"%5C"_ respectively), then +> 1. Throw an _Invalid Specifier_ error. > 1. If the file at _resolvedURL_ does not exist, then > 1. Throw a _Module Not Found_ error. > 1. Set _resolvedURL_ to the real path of _resolvedURL_. @@ -735,7 +754,7 @@ _isMain_ is **true** when resolving the Node.js application entry point. > 1. If _pjson_ is **null**, then > 1. Throw a _Module Not Found_ error. > 1. If _pjson.main_ is a String, then -> 1. Let _resolvedMain_ be the concatenation of _packageURL_, "/", and +> 1. Let _resolvedMain_ be the URL resolution of _packageURL_, "/", and > _pjson.main_. > 1. If the file at _resolvedMain_ exists, then > 1. Return _resolvedMain_. @@ -744,8 +763,6 @@ _isMain_ is **true** when resolving the Node.js application entry point. > 1. Let _legacyMainURL_ be the result applying the legacy > **LOAD_AS_DIRECTORY** CommonJS resolver to _packageURL_, throwing a > _Module Not Found_ error for no resolution. -> 1. If _legacyMainURL_ does not end in _".js"_ then, -> 1. Throw an _Unsupported File Extension_ error. > 1. Return _legacyMainURL_. **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, _packagePath_, _exports_) @@ -753,19 +770,42 @@ _isMain_ is **true** when resolving the Node.js application entry point. > 1. Set _packagePath_ to _"./"_ concatenated with _packagePath_. > 1. If _packagePath_ is a key of _exports_, then > 1. Let _target_ be the value of _exports[packagePath]_. -> 1. If _target_ is not a String, continue the loop. -> 1. Return the URL resolution of the concatenation of _packageURL_ and -> _target_. +> 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, +> _""_). > 1. Let _directoryKeys_ be the list of keys of _exports_ ending in > _"/"_, sorted by length descending. > 1. For each key _directory_ in _directoryKeys_, do > 1. If _packagePath_ starts with _directory_, then > 1. Let _target_ be the value of _exports[directory]_. -> 1. If _target_ is not a String, continue the loop. > 1. Let _subpath_ be the substring of _target_ starting at the index > of the length of _directory_. -> 1. Return the URL resolution of the concatenation of _packageURL_, -> _target_ and _subpath_. +> 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, +> _subpath_). +> 1. Throw a _Module Not Found_ error. + +**PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, _subpath_) +> 1. If _target_ is a String, then +> 1. If _target_ does not start with _"./"_, throw a _Module Not Found_ +> error. +> 1. If _subpath_ has non-zero length and _target_ does not end with _"/"_, +> throw a _Module Not Found_ error. +> 1. If _target_ or _subpath_ contain any _"node_modules"_ segments including +> _"node_modules"_ percent-encoding, throw a _Module Not Found_ error. +> 1. Let _resolvedTarget_ be the URL resolution of the concatenation of +> _packageURL_ and _target_. +> 1. If _resolvedTarget_ is contained in _packageURL_, then +> 1. Let _resolved_ be the URL resolution of the concatenation of +> _subpath_ and _resolvedTarget_. +> 1. If _resolved_ is contained in _resolvedTarget_, then +> 1. Return _resolved_. +> 1. Otherwise, if _target_ is an Array, then +> 1. For each item _targetValue_ in _target_, do +> 1. If _targetValue_ is not a String, continue the loop. +> 1. Let _resolved_ be the result of +> **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _targetValue_, +> _subpath_), continuing the loop on abrupt completion. +> 1. Assert: _resolved_ is a String. +> 1. Return _resolved_. > 1. Throw a _Module Not Found_ error. **ESM_FORMAT**(_url_, _isMain_) @@ -788,6 +828,7 @@ _isMain_ is **true** when resolving the Node.js application entry point. **READ_PACKAGE_SCOPE**(_url_) > 1. Let _scopeURL_ be _url_. > 1. While _scopeURL_ is not the file system root, +> 1. If _scopeURL_ ends in a _"node_modules"_ path segment, return **null**. > 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_scopeURL_). > 1. If _pjson_ is not **null**, then > 1. Return _pjson_. diff --git a/doc/api/modules.md b/doc/api/modules.md index bf8209965e9122..7197ef6ae2fdaa 100644 --- a/doc/api/modules.md +++ b/doc/api/modules.md @@ -202,11 +202,12 @@ NODE_MODULES_PATHS(START) 5. return DIRS ``` -If `--experimental-exports` is enabled, -node allows packages loaded via `LOAD_NODE_MODULES` to explicitly declare -which filepaths to expose and how they should be interpreted. -This expands on the control packages already had using the `main` field. -With this feature enabled, the `LOAD_NODE_MODULES` changes as follows: +If `--experimental-exports` is enabled, Node.js allows packages loaded via +`LOAD_NODE_MODULES` to explicitly declare which file paths to expose and how +they should be interpreted. This expands on the control packages already had +using the `main` field. + +With this feature enabled, the `LOAD_NODE_MODULES` changes are: ```txt LOAD_NODE_MODULES(X, START) @@ -224,10 +225,10 @@ RESOLVE_BARE_SPECIFIER(DIR, X) b. If "exports" is null or undefined, GOTO 3. c. Find the longest key in "exports" that the subpath starts with. d. If no such key can be found, throw "not found". - e. If the key matches the subpath entirely, return DIR/name/${exports[key]}. - f. If either the key or exports[key] do not end with a slash (`/`), - throw "not found". - g. Return DIR/name/${exports[key]}${subpath.slice(key.length)}. + e. let RESOLVED_URL = + PACKAGE_EXPORTS_TARGET_RESOLVE(pathToFileURL(DIR/name), exports[key], + subpath.slice(key.length)), as defined in the esm resolver. + f. return fileURLToPath(RESOLVED_URL) 3. return DIR/X ``` diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 95b56e08520a52..0ea8a46855e68a 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -24,6 +24,7 @@ const { JSON, Object, + ObjectPrototype, Reflect, SafeMap, StringPrototype, @@ -348,18 +349,18 @@ function resolveExports(nmPath, request, absoluteRequest) { const basePath = path.resolve(nmPath, name); const pkgExports = readExports(basePath); + const mappingKey = `.${expansion}`; - if (pkgExports != null) { - const mappingKey = `.${expansion}`; - const mapping = pkgExports[mappingKey]; - if (typeof mapping === 'string') { - return fileURLToPath(new URL(mapping, `${pathToFileURL(basePath)}/`)); + if (typeof pkgExports === 'object' && pkgExports !== null) { + if (ObjectPrototype.hasOwnProperty(pkgExports, mappingKey)) { + const mapping = pkgExports[mappingKey]; + return resolveExportsTarget(pathToFileURL(basePath + '/'), mapping, '', + basePath, mappingKey); } let dirMatch = ''; - for (const [candidateKey, candidateValue] of Object.entries(pkgExports)) { + for (const candidateKey of Object.keys(pkgExports)) { if (candidateKey[candidateKey.length - 1] !== '/') continue; - if (candidateValue[candidateValue.length - 1] !== '/') continue; if (candidateKey.length > dirMatch.length && StringPrototype.startsWith(mappingKey, candidateKey)) { dirMatch = candidateKey; @@ -367,15 +368,13 @@ function resolveExports(nmPath, request, absoluteRequest) { } if (dirMatch !== '') { - const dirMapping = pkgExports[dirMatch]; - const remainder = StringPrototype.slice(mappingKey, dirMatch.length); - const expectedPrefix = - new URL(dirMapping, `${pathToFileURL(basePath)}/`); - const resolved = new URL(remainder, expectedPrefix).href; - if (StringPrototype.startsWith(resolved, expectedPrefix.href)) { - return fileURLToPath(resolved); - } + const mapping = pkgExports[dirMatch]; + const subpath = StringPrototype.slice(mappingKey, dirMatch.length); + return resolveExportsTarget(pathToFileURL(basePath + '/'), mapping, + subpath, basePath, mappingKey); } + } + if (pkgExports != null) { // eslint-disable-next-line no-restricted-syntax const e = new Error(`Package exports for '${basePath}' do not define ` + `a '${mappingKey}' subpath`); @@ -387,6 +386,43 @@ function resolveExports(nmPath, request, absoluteRequest) { return path.resolve(nmPath, request); } +function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) { + if (typeof target === 'string') { + if (target.startsWith('./') && + (subpath.length === 0 || target.endsWith('/'))) { + const resolvedTarget = new URL(target, pkgPath); + const pkgPathPath = pkgPath.pathname; + const resolvedTargetPath = resolvedTarget.pathname; + if (StringPrototype.startsWith(resolvedTargetPath, pkgPathPath) && + StringPrototype.indexOf(resolvedTargetPath, '/node_modules/', + pkgPathPath.length - 1) === -1) { + const resolved = new URL(subpath, resolvedTarget); + const resolvedPath = resolved.pathname; + if (StringPrototype.startsWith(resolvedPath, resolvedTargetPath) && + StringPrototype.indexOf(resolvedPath, '/node_modules/', + pkgPathPath.length - 1) === -1) { + return fileURLToPath(resolved); + } + } + } + } else if (Array.isArray(target)) { + for (const targetValue of target) { + if (typeof targetValue !== 'string') continue; + try { + return resolveExportsTarget(pkgPath, targetValue, subpath, basePath, + mappingKey); + } catch (e) { + if (e.code !== 'MODULE_NOT_FOUND') throw e; + } + } + } + // eslint-disable-next-line no-restricted-syntax + const e = new Error(`Package exports for '${basePath}' do not define a ` + + `valid '${mappingKey}' target${subpath ? 'for ' + subpath : ''}`); + e.code = 'MODULE_NOT_FOUND'; + throw e; +} + Module._findPath = function(request, paths, isMain) { const absoluteRequest = path.isAbsolute(request); if (absoluteRequest) { diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 5b33ef261cf69c..83cc9863a62553 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -545,8 +545,8 @@ Maybe GetPackageConfig(Environment* env, if (existing != env->package_json_cache.end()) { const PackageConfig* pcfg = &existing->second; if (pcfg->is_valid == IsValid::No) { - std::string msg = "Invalid JSON in '" + path + - "' imported from " + base.ToFilePath(); + std::string msg = "Invalid JSON in " + path + + " imported from " + base.ToFilePath(); node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str()); return Nothing(); } @@ -579,8 +579,8 @@ Maybe GetPackageConfig(Environment* env, env->package_json_cache.emplace(path, PackageConfig { Exists::Yes, IsValid::No, HasMain::No, "", PackageType::None, Global() }); - std::string msg = "Invalid JSON in '" + path + - "' imported from " + base.ToFilePath(); + std::string msg = "Invalid JSON in " + path + + " imported from " + base.ToFilePath(); node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str()); return Nothing(); } @@ -633,6 +633,12 @@ Maybe GetPackageScopeConfig(Environment* env, const URL& base) { URL pjson_url("./package.json", &resolved); while (true) { + std::string pjson_url_path = pjson_url.path(); + if (pjson_url_path.length() > 25 && + pjson_url_path.substr(pjson_url_path.length() - 25, 25) == + "node_modules/package.json") { + break; + } Maybe pkg_cfg = GetPackageConfig(env, pjson_url.ToFilePath(), base); if (pkg_cfg.IsNothing()) return pkg_cfg; @@ -643,14 +649,13 @@ Maybe GetPackageScopeConfig(Environment* env, // Terminates at root where ../package.json equals ../../package.json // (can't just check "/package.json" for Windows support). - if (pjson_url.path() == last_pjson_url.path()) { - auto entry = env->package_json_cache.emplace(pjson_url.ToFilePath(), - PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "", - PackageType::None, Global() }); - const PackageConfig* pcfg = &entry.first->second; - return Just(pcfg); - } + if (pjson_url.path() == last_pjson_url.path()) break; } + auto entry = env->package_json_cache.emplace(pjson_url.ToFilePath(), + PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "", + PackageType::None, Global() }); + const PackageConfig* pcfg = &entry.first->second; + return Just(pcfg); } /* @@ -750,16 +755,17 @@ Maybe FinalizeResolution(Environment* env, if (!file.IsNothing()) { return file; } - std::string msg = "Cannot find module '" + resolved.path() + - "' imported from " + base.ToFilePath(); + std::string msg = "Cannot find module " + resolved.path() + + " imported from " + base.ToFilePath(); node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); return Nothing(); } const std::string& path = resolved.ToFilePath(); if (CheckDescriptorAtPath(path) != FILE) { - std::string msg = "Cannot find module '" + path + - "' imported from " + base.ToFilePath(); + std::string msg = "Cannot find module " + + (path.length() != 0 ? path : resolved.path()) + + " imported from " + base.ToFilePath(); node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); return Nothing(); } @@ -793,13 +799,94 @@ Maybe PackageMainResolve(Environment* env, } } } - std::string msg = "Cannot find main entry point for '" + - URL(".", pjson_url).ToFilePath() + "' imported from " + + std::string msg = "Cannot find main entry point for " + + URL(".", pjson_url).ToFilePath() + " imported from " + base.ToFilePath(); node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); return Nothing(); } +void ThrowExportsNotFound(Environment* env, + const std::string& subpath, + const URL& pjson_url, + const URL& base) { + const std::string msg = "Package exports for " + + pjson_url.ToFilePath() + " do not define a '" + subpath + + "' subpath, imported from " + base.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); +} + +void ThrowExportsInvalid(Environment* env, + const std::string& subpath, + const std::string& target, + const URL& pjson_url, + const URL& base) { + const std::string msg = "Cannot resolve package exports target '" + target + + "' matched for '" + subpath + "' in " + pjson_url.ToFilePath() + + ", imported from " + base.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); +} + +void ThrowExportsInvalid(Environment* env, + const std::string& subpath, + Local target, + const URL& pjson_url, + const URL& base) { + Local target_string; + if (target->ToString(env->context()).ToLocal(&target_string)) { + Utf8Value target_utf8(env->isolate(), target_string); + std::string target_str(*target_utf8, target_utf8.length()); + if (target->IsArray()) { + target_str = '[' + target_str + ']'; + } + ThrowExportsInvalid(env, subpath, target_str, pjson_url, base); + } +} + +Maybe ResolveExportsTarget(Environment* env, + const std::string& target, + const std::string& subpath, + const std::string& match, + const URL& pjson_url, + const URL& base, + bool throw_invalid = true) { + if (target.substr(0, 2) != "./") { + if (throw_invalid) { + ThrowExportsInvalid(env, match, target, pjson_url, base); + } + return Nothing(); + } + if (subpath.length() > 0 && target.back() != '/') { + if (throw_invalid) { + ThrowExportsInvalid(env, match, target, pjson_url, base); + } + return Nothing(); + } + URL resolved(target, pjson_url); + std::string resolved_path = resolved.path(); + std::string pkg_path = URL(".", pjson_url).path(); + if (resolved_path.find(pkg_path) != 0 || + resolved_path.find("/node_modules/", pkg_path.length() - 1) != + std::string::npos) { + if (throw_invalid) { + ThrowExportsInvalid(env, match, target, pjson_url, base); + } + return Nothing(); + } + if (subpath.length() == 0) return Just(resolved); + URL subpath_resolved(subpath, resolved); + std::string subpath_resolved_path = subpath_resolved.path(); + if (subpath_resolved_path.find(resolved_path) != 0 || + subpath_resolved_path.find("/node_modules/", pkg_path.length() - 1) + != std::string::npos) { + if (throw_invalid) { + ThrowExportsInvalid(env, match, target + subpath, pjson_url, base); + } + return Nothing(); + } + return Just(subpath_resolved); +} + Maybe PackageExportsResolve(Environment* env, const URL& pjson_url, const std::string& pkg_subpath, @@ -809,57 +896,126 @@ Maybe PackageExportsResolve(Environment* env, Isolate* isolate = env->isolate(); Local context = env->context(); Local exports = pcfg.exports.Get(isolate); - if (exports->IsObject()) { - Local exports_obj = exports.As(); - Local subpath = String::NewFromUtf8(isolate, - pkg_subpath.c_str(), v8::NewStringType::kNormal).ToLocalChecked(); + if (!exports->IsObject()) { + ThrowExportsNotFound(env, pkg_subpath, pjson_url, base); + return Nothing(); + } + Local exports_obj = exports.As(); + Local subpath = String::NewFromUtf8(isolate, + pkg_subpath.c_str(), v8::NewStringType::kNormal).ToLocalChecked(); - auto target = exports_obj->Get(context, subpath).ToLocalChecked(); + if (exports_obj->HasOwnProperty(context, subpath).FromJust()) { + Local target = exports_obj->Get(context, subpath).ToLocalChecked(); if (target->IsString()) { Utf8Value target_utf8(isolate, target.As()); - std::string target(*target_utf8, target_utf8.length()); - if (target.substr(0, 2) == "./") { - URL target_url(target, pjson_url); - return FinalizeResolution(env, target_url, base); + std::string target_str(*target_utf8, target_utf8.length()); + Maybe resolved = ResolveExportsTarget(env, target_str, "", + pkg_subpath, pjson_url, base); + if (resolved.IsNothing()) { + ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); + return Nothing(); } + return FinalizeResolution(env, resolved.FromJust(), base); + } else if (target->IsArray()) { + Local target_arr = target.As(); + const uint32_t length = target_arr->Length(); + if (length == 0) { + ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); + return Nothing(); + } + for (uint32_t i = 0; i < length; i++) { + auto target_item = target_arr->Get(context, i).ToLocalChecked(); + if (target_item->IsString()) { + Utf8Value target_utf8(isolate, target_item.As()); + std::string target(*target_utf8, target_utf8.length()); + Maybe resolved = ResolveExportsTarget(env, target, "", + pkg_subpath, pjson_url, base, false); + if (resolved.IsNothing()) continue; + return FinalizeResolution(env, resolved.FromJust(), base); + } + } + auto invalid = target_arr->Get(context, length - 1).ToLocalChecked(); + if (!invalid->IsString()) { + ThrowExportsInvalid(env, pkg_subpath, invalid, pjson_url, base); + return Nothing(); + } + Utf8Value invalid_utf8(isolate, invalid.As()); + std::string invalid_str(*invalid_utf8, invalid_utf8.length()); + Maybe resolved = ResolveExportsTarget(env, invalid_str, "", + pkg_subpath, pjson_url, base); + CHECK(resolved.IsNothing()); + return Nothing(); + } else { + ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); + return Nothing(); } + } - Local best_match; - std::string best_match_str = ""; - Local keys = - exports_obj->GetOwnPropertyNames(context).ToLocalChecked(); - for (uint32_t i = 0; i < keys->Length(); ++i) { - Local key = keys->Get(context, i).ToLocalChecked().As(); - Utf8Value key_utf8(isolate, key); - std::string key_str(*key_utf8, key_utf8.length()); - if (key_str.back() != '/') continue; - if (pkg_subpath.substr(0, key_str.length()) == key_str && - key_str.length() > best_match_str.length()) { - best_match = key; - best_match_str = key_str; - } + Local best_match; + std::string best_match_str = ""; + Local keys = + exports_obj->GetOwnPropertyNames(context).ToLocalChecked(); + for (uint32_t i = 0; i < keys->Length(); ++i) { + Local key = keys->Get(context, i).ToLocalChecked().As(); + Utf8Value key_utf8(isolate, key); + std::string key_str(*key_utf8, key_utf8.length()); + if (key_str.back() != '/') continue; + if (pkg_subpath.substr(0, key_str.length()) == key_str && + key_str.length() > best_match_str.length()) { + best_match = key; + best_match_str = key_str; } + } - if (best_match_str.length() > 0) { - auto target = exports_obj->Get(context, best_match).ToLocalChecked(); - if (target->IsString()) { - Utf8Value target_utf8(isolate, target.As()); - std::string target(*target_utf8, target_utf8.length()); - if (target.back() == '/' && target.substr(0, 2) == "./") { - std::string subpath = pkg_subpath.substr(best_match_str.length()); - URL url_prefix(target, pjson_url); - URL target_url(subpath, url_prefix); - if (target_url.path().find(url_prefix.path()) == 0) { - return FinalizeResolution(env, target_url, base); - } + if (best_match_str.length() > 0) { + auto target = exports_obj->Get(context, best_match).ToLocalChecked(); + std::string subpath = pkg_subpath.substr(best_match_str.length()); + if (target->IsString()) { + Utf8Value target_utf8(isolate, target.As()); + std::string target(*target_utf8, target_utf8.length()); + Maybe resolved = ResolveExportsTarget(env, target, subpath, + pkg_subpath, pjson_url, base); + if (resolved.IsNothing()) { + ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); + return Nothing(); + } + return FinalizeResolution(env, URL(subpath, resolved.FromJust()), base); + } else if (target->IsArray()) { + Local target_arr = target.As(); + const uint32_t length = target_arr->Length(); + if (length == 0) { + ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); + return Nothing(); + } + for (uint32_t i = 0; i < length; i++) { + auto target_item = target_arr->Get(context, i).ToLocalChecked(); + if (target_item->IsString()) { + Utf8Value target_utf8(isolate, target_item.As()); + std::string target_str(*target_utf8, target_utf8.length()); + Maybe resolved = ResolveExportsTarget(env, target_str, subpath, + pkg_subpath, pjson_url, base, false); + if (resolved.IsNothing()) continue; + return FinalizeResolution(env, resolved.FromJust(), base); } } + auto invalid = target_arr->Get(context, length - 1).ToLocalChecked(); + if (!invalid->IsString()) { + ThrowExportsInvalid(env, pkg_subpath, invalid, pjson_url, base); + return Nothing(); + } + Utf8Value invalid_utf8(isolate, invalid.As()); + std::string invalid_str(*invalid_utf8, invalid_utf8.length()); + Maybe resolved = ResolveExportsTarget(env, invalid_str, subpath, + pkg_subpath, pjson_url, base); + CHECK(resolved.IsNothing()); + return Nothing(); + } else { + ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); + return Nothing(); } } - std::string msg = "Package exports for '" + - URL(".", pjson_url).ToFilePath() + "' do not define a '" + pkg_subpath + - "' subpath, imported from " + base.ToFilePath(); - node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + + ThrowExportsNotFound(env, pkg_subpath, pjson_url, base); return Nothing(); } diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index 6c42d3b64ead40..4f772521588898 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -17,6 +17,9 @@ import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs'; ['pkgexports/space', { default: 'encoded path' }], // Verifying that normal packages still work with exports turned on. isRequire ? ['baz/index', { default: 'eye catcher' }] : [null], + // Fallbacks + ['pkgexports/fallbackdir/asdf.js', { default: 'asdf' }], + ['pkgexports/fallbackfile', { default: 'asdf' }], ]); for (const [validSpecifier, expected] of validSpecifiers) { if (validSpecifier === null) continue; @@ -27,20 +30,56 @@ import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs'; })); } - // There's no such export - so there's nothing to do. - loadFixture('pkgexports/missing').catch(mustCall((err) => { - strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND'); - assertStartsWith(err.message, 'Package exports'); - assertIncludes(err.message, 'do not define a \'./missing\' subpath'); - })); + const undefinedExports = new Map([ + // There's no such export - so there's nothing to do. + ['pkgexports/missing', './missing'], + // The file exists but isn't exported. The exports is a number which counts + // as a non-null value without any properties, just like `{}`. + ['pkgexports-number/hidden.js', './hidden.js'], + ]); - // The file exists but isn't exported. The exports is a number which counts - // as a non-null value without any properties, just like `{}`. - loadFixture('pkgexports-number/hidden.js').catch(mustCall((err) => { - strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND'); - assertStartsWith(err.message, 'Package exports'); - assertIncludes(err.message, 'do not define a \'./hidden.js\' subpath'); - })); + const invalidExports = new Map([ + // Even though 'pkgexports/sub/asdf.js' works, alternate "path-like" + // variants do not to prevent confusion and accidental loopholes. + ['pkgexports/sub/./../asdf.js', './sub/./../asdf.js'], + // This path steps back inside the package but goes through an exports + // target that escapes the package, so we still catch that as invalid + ['pkgexports/belowdir/pkgexports/asdf.js', './belowdir/pkgexports/asdf.js'], + // This target file steps below the package + ['pkgexports/belowfile', './belowfile'], + // Directory mappings require a trailing / to work + ['pkgexports/missingtrailer/x', './missingtrailer/x'], + // Invalid target handling + ['pkgexports/null', './null'], + ['pkgexports/invalid1', './invalid1'], + ['pkgexports/invalid2', './invalid2'], + ['pkgexports/invalid3', './invalid3'], + ['pkgexports/invalid4', './invalid4'], + // Missing / invalid fallbacks + ['pkgexports/nofallback1', './nofallback1'], + ['pkgexports/nofallback2', './nofallback2'], + // Reaching into nested node_modules + ['pkgexports/nodemodules', './nodemodules'], + ]); + + for (const [specifier, subpath] of undefinedExports) { + loadFixture(specifier).catch(mustCall((err) => { + strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND'); + assertStartsWith(err.message, 'Package exports'); + assertIncludes(err.message, `do not define a '${subpath}' subpath`); + })); + } + + for (const [specifier, subpath] of invalidExports) { + loadFixture(specifier).catch(mustCall((err) => { + strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND'); + assertStartsWith(err.message, (isRequire ? 'Package exports' : + 'Cannot resolve')); + assertIncludes(err.message, isRequire ? + `do not define a valid '${subpath}' subpath` : + `matched for '${subpath}'`); + })); + } // There's no main field so we won't find anything when importing the name. // The fact that "." is mapped is ignored, it's not a valid main config. @@ -54,26 +93,19 @@ import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs'; } })); - // Even though 'pkgexports/sub/asdf.js' works, alternate "path-like" variants - // do not to prevent confusion and accidental loopholes. - loadFixture('pkgexports/sub/./../asdf.js').catch(mustCall((err) => { + // Covering out bases - not a file is still not a file after dir mapping. + loadFixture('pkgexports/sub/not-a-file.js').catch(mustCall((err) => { strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND'); - assertStartsWith(err.message, 'Package exports'); - assertIncludes(err.message, - 'do not define a \'./sub/./../asdf.js\' subpath'); + // ESM returns a full file path + assertStartsWith(err.message, isRequire ? + 'Cannot find module \'pkgexports/sub/not-a-file.js\'' : + 'Cannot find module'); })); - // Covering out bases - not a file is still not a file after dir mapping. - loadFixture('pkgexports/sub/not-a-file.js').catch(mustCall((err) => { - if (isRequire) { - strictEqual(err.code, 'MODULE_NOT_FOUND'); - assertStartsWith(err.message, - 'Cannot find module \'pkgexports/sub/not-a-file.js\''); - } else { - strictEqual(err.code, 'ERR_MODULE_NOT_FOUND'); - // ESM currently returns a full file path - assertStartsWith(err.message, 'Cannot find module'); - } + // THe use of %2F escapes in paths fails loading + loadFixture('pkgexports/sub/..%2F..%2Fbar.js').catch(mustCall((err) => { + strictEqual(err.code, isRequire ? 'ERR_INVALID_FILE_URL_PATH' : + 'ERR_MODULE_NOT_FOUND'); })); }); diff --git a/test/es-module/test-esm-scope-node-modules.mjs b/test/es-module/test-esm-scope-node-modules.mjs new file mode 100644 index 00000000000000..8358da5c765288 --- /dev/null +++ b/test/es-module/test-esm-scope-node-modules.mjs @@ -0,0 +1,10 @@ +// Flags: --experimental-modules +import '../common/index.mjs'; +import cjs from '../fixtures/baz.js'; +import { message } from '../fixtures/es-modules/message.mjs'; +import assert from 'assert'; + +// Assert we loaded esm dependency as ".js" in this mode +assert.strictEqual(message, 'A message'); +// Assert we loaded CommonJS dependency +assert.strictEqual(cjs, 'perhaps I work'); diff --git a/test/fixtures/es-modules/package-type-module/index.js b/test/fixtures/es-modules/package-type-module/index.js index 12aba970ef279f..e8f4db3e164302 100644 --- a/test/fixtures/es-modules/package-type-module/index.js +++ b/test/fixtures/es-modules/package-type-module/index.js @@ -1,3 +1,4 @@ +import 'dep/dep.js'; const identifier = 'package-type-module'; console.log(identifier); export default identifier; diff --git a/test/fixtures/es-modules/package-type-module/node_modules/dep/dep.js b/test/fixtures/es-modules/package-type-module/node_modules/dep/dep.js new file mode 100644 index 00000000000000..7b04e8b3808e04 --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/node_modules/dep/dep.js @@ -0,0 +1,2 @@ +// No package.json -> should still be CommonJS as it is in node_modules +module.exports = 42; diff --git a/test/fixtures/node_modules/pkgexports/node_modules/internalpkg/x.js b/test/fixtures/node_modules/pkgexports/node_modules/internalpkg/x.js new file mode 100644 index 00000000000000..888cae37af95c5 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports/node_modules/internalpkg/x.js @@ -0,0 +1 @@ +module.exports = 42; diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index 97f07da85e2a61..2b190521e5f987 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -3,6 +3,19 @@ ".": "./asdf.js", "./space": "./sp%20ce.js", "./valid-cjs": "./asdf.js", - "./sub/": "./" + "./sub/": "./", + "./belowdir/": "../belowdir/", + "./belowfile": "../belowfile", + "./missingtrailer/": ".", + "./null": null, + "./invalid1": {}, + "./invalid2": 1234, + "./invalid3": "", + "./invalid4": {}, + "./fallbackdir/": [[], null, {}, "builtin:x/", "./fallbackfile", "./"], + "./fallbackfile": [[], null, {}, "builtin:x", "./asdf.js"], + "./nofallback1": [], + "./nofallback2": [null, {}, "builtin:x"], + "./nodemodules": "./node_modules/internalpkg/x.js" } }