From e3d517aad972cd13cc72e4f10c4ce1bd6f4532cb Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 1 Nov 2019 17:24:36 -0400 Subject: [PATCH 01/13] doc: include --experimental-resolve-self in manpage --- doc/node.1 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/node.1 b/doc/node.1 index e9b7855b9be483..30d63b216dfd8b 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -119,6 +119,9 @@ Enable experimental JSON interop support for the ES Module loader. .It Fl -experimental-modules Enable experimental ES module support and caching modules. . +.It Fl -experimental-resolve-self +Enable experimental support for a package to load itself. +. .It Fl -experimental-policy Use the specified file as a security policy. . From 8f2d04e16f02ed1061045e6ab772602a13898f75 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 13 Oct 2019 19:27:39 -0400 Subject: [PATCH 02/13] module: exports conditions, --experimental-require-target --- doc/api/cli.md | 10 + doc/api/esm.md | 109 +++++-- doc/api/modules.md | 11 +- doc/node.1 | 3 + lib/internal/modules/cjs/loader.js | 82 ++++- src/env.h | 3 + src/module_wrap.cc | 279 +++++++++--------- src/node_options.cc | 4 + src/node_options.h | 1 + test/es-module/test-esm-exports.mjs | 9 +- .../node_modules/pkgexports-direct/asdf.js | 1 + .../pkgexports-direct/package.json | 6 + .../node_modules/pkgexports-direct/sp ce.js | 3 + .../node_modules/pkgexports/package.json | 5 +- 14 files changed, 328 insertions(+), 198 deletions(-) create mode 100644 test/fixtures/node_modules/pkgexports-direct/asdf.js create mode 100644 test/fixtures/node_modules/pkgexports-direct/package.json create mode 100644 test/fixtures/node_modules/pkgexports-direct/sp ce.js diff --git a/doc/api/cli.md b/doc/api/cli.md index fa46a00aff8f0f..e370579249f56e 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -205,6 +205,14 @@ added: v11.8.0 Enable experimental diagnostic report feature. +### `--experimental-require-target + + +Enable experimental support for a `"require"` conditional package export target. +See [Conditional Exports][] for more information. + ### `--experimental-resolve-self` ```js -// ./node_modules/es-module-package/package.json { "exports": { - ".": "./main.js" + "./submodule": ["not:valid", "./submodule.js"] } } ``` -where the "." indicates loading the package without any subpath. Exports will -always override any existing `"main"` value for both CommonJS and -ES module packages. +Since `"not:valid"` is not a supported target, `"./submodule.js"` is used +instead as the fallback, as if it were the only target. + +Defining a `"."` export will define the main entry point for the package, +and will always take precedence over the `"main"` field in the `package.json`. -For packages with only a main entry point, an `"exports"` value of just -a string is also supported: +This allows defining a different entry point for Node.js versions that support +ECMAScript modules and versions that don't, for example: ```js -// ./node_modules/es-module-package/package.json { - "exports": "./main.js" + "main": "./main-legacy.cjs", + "exports": { + ".": "./main-modern.cjs" + } } ``` -Any invalid exports entries will be ignored. This includes exports not -starting with `"./"` or a missing trailing `"/"` for directory exports. +#### Conditional Exports -Array fallback support is provided for exports, similarly to import maps -in order to be forward-compatible with fallback workflows in future: +Conditional exports provide a way to map to different paths depending on +certain conditions. They are supported for both CommonJS and ES module imports. + +For example, a package that wants to provide different ES module exports for +Node.js and the browser can be written: ```js +// ./node_modules/pkg/package.json { + "type": "module", + "main": "./index.js", "exports": { - "./submodule": ["not:valid", "./submodule.js"] + "./feature": { + "browser": "./feature-browser.js", + "default": "./feature-default.js" + } } } ``` -Since `"not:valid"` is not a supported target, `"./submodule.js"` is used -instead as the fallback, as if it were the only target. +When an exports target condition object is provided like the above, the first +matching key in the object that is supported by the current environment will +be resolved. + +When resolving the `"."` export, if no matching target is found, the `"main"` +will be used as the final fallback. + +The conditions supported in Node.js are matched in the following order: + +1. `"require"` - matched when the package is loaded via `require()`. + This is currently only supported behind the `--experimental-require-target` + flag. +2. `"node"` - matched for any Node.js environment. Can be a CommonJS or ES + module file. +3. `"default"` - the generic fallback that will always match if no other + more specific condition is matched first. Can be a CommonJS or ES module + file. + +Using the `"require"` condition it is possible to define a package that will +have a different exported value for CommonJS and ES modules, which can be a +hazard in that it can result in having two separate instances of the same +package in use in an application, which can cause a number of bugs. + +Other conditions such as `"browser"`, `"electron"`, `"deno"`, `"react-native"` +etc. could be defined in other runtimes or tools. ## import Specifiers @@ -806,6 +844,9 @@ of these top-level routines unless stated otherwise. _isMain_ is **true** when resolving the Node.js application entry point. +_defaultEnv_ is the conditional environment name priority array, +`["node", "default"]`. +
Resolver algorithm specification @@ -905,14 +946,14 @@ _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.exports_ is not **null** or **undefined**, then -> 1. If _pjson.exports_ is a String or Array, then +> 1. If _pjson.exports_ is a String or Array, or an Object whose first key +> does not start with _"."_, then +> 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, +> _pjson.exports_, _""_). +> 1. If _pjson.exports_ is an Object containing a _"."_ property, then +> 1. Let _mainExport_ be the _"."_ property in _pjson.exports_. > 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, -> _pjson.exports_, "")_. -> 1. If _pjson.exports is an Object, then -> 1. If _pjson.exports_ contains a _"."_ property, then -> 1. Let _mainExport_ be the _"."_ property in _pjson.exports_. -> 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, -> _mainExport_, "")_. +> _mainExport_, _""_). > 1. If _pjson.main_ is a String, then > 1. Let _resolvedMain_ be the URL resolution of _packageURL_, "/", and > _pjson.main_. @@ -932,7 +973,7 @@ _isMain_ is **true** when resolving the Node.js application entry point. > 1. If _packagePath_ is a key of _exports_, then > 1. Let _target_ be the value of _exports\[packagePath\]_. > 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, -> _""_). +> _""_, _defaultEnv_). > 1. Let _directoryKeys_ be the list of keys of _exports_ ending in > _"/"_, sorted by length descending. > 1. For each key _directory_ in _directoryKeys_, do @@ -941,10 +982,10 @@ _isMain_ is **true** when resolving the Node.js application entry point. > 1. Let _subpath_ be the substring of _target_ starting at the index > of the length of _directory_. > 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, -> _subpath_). +> _subpath_, _defaultEnv_). > 1. Throw a _Module Not Found_ error. -**PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, _subpath_) +**PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, _subpath_, _env_) > 1. If _target_ is a String, then > 1. If _target_ does not start with _"./"_, throw a _Module Not Found_ @@ -960,12 +1001,20 @@ _isMain_ is **true** when resolving the Node.js application entry point. > _subpath_ and _resolvedTarget_. > 1. If _resolved_ is contained in _resolvedTarget_, then > 1. Return _resolved_. +> 1. Otherwise, if _target_ is a non-null Object, then +> 1. If _target_ has an object key matching one of the names in _env_, then +> 1. Let _targetValue_ be the corresponding value of the first object key +> of _target_ in _env_. +> 1. Let _resolved_ be the result of **PACKAGE_EXPORTS_TARGET_RESOLVE** +> (_packageURL_, _targetValue_, _subpath_, _env_). +> 1. Assert: _resolved_ is a String. +> 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. If _targetValue_ is an Array, continue the loop. > 1. Let _resolved_ be the result of > **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _targetValue_, -> _subpath_), continuing the loop on abrupt completion. +> _subpath_, _env_), continuing the loop on abrupt completion. > 1. Assert: _resolved_ is a String. > 1. Return _resolved_. > 1. Throw a _Module Not Found_ error. @@ -1045,7 +1094,7 @@ success! [`import`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import [`module.createRequire()`]: modules.html#modules_module_createrequire_filename [`module.syncBuiltinESMExports()`]: modules.html#modules_module_syncbuiltinesmexports -[dynamic instantiate hook]: #esm_dynamic_instantiate_hook [package exports]: #esm_package_exports +[dynamic instantiate hook]: #esm_dynamic_instantiate_hook [special scheme]: https://url.spec.whatwg.org/#special-scheme [the official standard format]: https://tc39.github.io/ecma262/#sec-modules diff --git a/doc/api/modules.md b/doc/api/modules.md index 8715218b32c8a4..cf6005deb16722 100644 --- a/doc/api/modules.md +++ b/doc/api/modules.md @@ -232,12 +232,15 @@ RESOLVE_BARE_SPECIFIER(DIR, X) 2. If X matches this pattern and DIR/name/package.json is a file: a. Parse DIR/name/package.json, and look for "exports" field. 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. let RESOLVED_URL = + c. If "exports" is a string, or object whose first key does not start with + ".", treat it as having that value as its "." object property. + d. If subpath is "." and "exports" does not have a "." entry, GOTO 3. + e. Find the longest key in "exports" that the subpath starts with. + f. If no such key can be found, throw "not found". + g. 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) + h. return fileURLToPath(RESOLVED_URL) 3. return DIR/X ``` diff --git a/doc/node.1 b/doc/node.1 index 30d63b216dfd8b..c24bb95f0336ff 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -119,6 +119,9 @@ Enable experimental JSON interop support for the ES Module loader. .It Fl -experimental-modules Enable experimental ES module support and caching modules. . +.It Fl -experimental-require-target +Enable experimental support for a "require" conditional export. +. .It Fl -experimental-resolve-self Enable experimental support for a package to load itself. . diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 8cec27e6c4229b..bfc5d47ad84e5e 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -59,6 +59,8 @@ const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); const experimentalModules = getOptionValue('--experimental-modules'); const experimentalSelf = getOptionValue('--experimental-resolve-self'); +const experimentalRequireTarget = + getOptionValue('--experimental-require-target'); const manifest = getOptionValue('--experimental-policy') ? require('internal/process/policy').manifest : null; @@ -441,7 +443,6 @@ function trySelf(paths, exts, isMain, trailingSlash, request) { if (expansion) { // Use exports const fromExports = applyExports(basePath, expansion); - if (!fromExports) return false; return resolveBasePath(fromExports, exts, isMain, trailingSlash, request); } else { // Use main field @@ -450,16 +451,23 @@ function trySelf(paths, exts, isMain, trailingSlash, request) { } function applyExports(basePath, expansion) { - const pkgExports = readPackageExports(basePath); const mappingKey = `.${expansion}`; - if (typeof pkgExports === 'object' && pkgExports !== null) { + const pkgExports = readPackageExports(basePath); + if (pkgExports === undefined || pkgExports === null || !experimentalModules) + return path.resolve(basePath, mappingKey); + + if (typeof pkgExports === 'object' && firstKeyStartsWithDot(pkgExports)) { if (ObjectPrototype.hasOwnProperty(pkgExports, mappingKey)) { const mapping = pkgExports[mappingKey]; return resolveExportsTarget(pathToFileURL(basePath + '/'), mapping, '', basePath, mappingKey); } + // Fallback to CJS main lookup when no main export is defined + if (mappingKey === '.') + return basePath; + let dirMatch = ''; for (const candidateKey of Object.keys(pkgExports)) { if (candidateKey[candidateKey.length - 1] !== '/') continue; @@ -476,19 +484,20 @@ function applyExports(basePath, expansion) { subpath, basePath, mappingKey); } } - if (mappingKey === '.' && typeof pkgExports === 'string') { + if ((mappingKey === '.' && typeof pkgExports === 'string') || + (typeof pkgExports === 'object' && !firstKeyStartsWithDot(pkgExports))) { return resolveExportsTarget(pathToFileURL(basePath + '/'), pkgExports, '', 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`); - e.code = 'MODULE_NOT_FOUND'; - throw e; - } + // Fallback to CJS main lookup when no main export is defined + if (mappingKey === '.') + return basePath; - return path.resolve(basePath, mappingKey); + // eslint-disable-next-line no-restricted-syntax + const e = new Error(`Package exports for '${basePath}' do not define ` + + `a '${mappingKey}' subpath`); + e.code = 'MODULE_NOT_FOUND'; + throw e; } // This only applies to requests of a specific form: @@ -511,6 +520,15 @@ function resolveExports(nmPath, request, absoluteRequest) { return path.resolve(nmPath, request); } +function firstKeyStartsWithDot(exports) { + for (const p in exports) { + if (!ObjectPrototype.hasOwnProperty(exports, p)) + continue; + return p[0] === '.'; + } + return false; +} + function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) { if (typeof target === 'string') { if (target.startsWith('./') && @@ -532,7 +550,7 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) { } } else if (Array.isArray(target)) { for (const targetValue of target) { - if (typeof targetValue !== 'string') continue; + if (Array.isArray(targetValue)) continue; try { return resolveExportsTarget(pkgPath, targetValue, subpath, basePath, mappingKey); @@ -540,10 +558,42 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) { if (e.code !== 'MODULE_NOT_FOUND') throw e; } } + } else if (typeof target === 'object' && target !== null) { + if (experimentalRequireTarget && + ObjectPrototype.hasOwnProperty(target, 'require')) { + try { + return resolveExportsTarget(pkgPath, target.require, subpath, + basePath, mappingKey); + } catch (e) { + if (e.code !== 'MODULE_NOT_FOUND') throw e; + } + } + if (ObjectPrototype.hasOwnProperty(target, 'node')) { + try { + return resolveExportsTarget(pkgPath, target.node, subpath, + basePath, mappingKey); + } catch (e) { + if (e.code !== 'MODULE_NOT_FOUND') throw e; + } + } + if (ObjectPrototype.hasOwnProperty(target, 'default')) { + try { + return resolveExportsTarget(pkgPath, target.default, subpath, + basePath, mappingKey); + } catch (e) { + if (e.code !== 'MODULE_NOT_FOUND') throw e; + } + } + } + let e; + if (mappingKey !== '.') { + // eslint-disable-next-line no-restricted-syntax + e = new Error(`Package exports for '${basePath}' do not define a ` + + `valid '${mappingKey}' target${subpath ? ' for ' + subpath : ''}`); + } else { + // eslint-disable-next-line no-restricted-syntax + e = new Error(`No valid exports main found for '${basePath}'`); } - // 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; } diff --git a/src/env.h b/src/env.h index d6dd50b70110c5..223337704abca2 100644 --- a/src/env.h +++ b/src/env.h @@ -201,6 +201,7 @@ constexpr size_t kFsStatsBufferLength = V(crypto_rsa_pss_string, "rsa-pss") \ V(cwd_string, "cwd") \ V(data_string, "data") \ + V(default_string, "default") \ V(dest_string, "dest") \ V(destroyed_string, "destroyed") \ V(detached_string, "detached") \ @@ -215,6 +216,7 @@ constexpr size_t kFsStatsBufferLength = V(dns_srv_string, "SRV") \ V(dns_txt_string, "TXT") \ V(done_string, "done") \ + V(dot_string, ".") \ V(duration_string, "duration") \ V(emit_warning_string, "emitWarning") \ V(encoding_string, "encoding") \ @@ -278,6 +280,7 @@ constexpr size_t kFsStatsBufferLength = V(netmask_string, "netmask") \ V(next_string, "next") \ V(nistcurve_string, "nistCurve") \ + V(node_string, "node") \ V(nsname_string, "nsname") \ V(ocsp_request_string, "OCSPRequest") \ V(oncertcb_string, "oncertcb") \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 4c4a1ce863849e..0a4f53ff62bb22 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -835,10 +835,16 @@ void ThrowExportsInvalid(Environment* env, 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()); + if (subpath.length()) { + 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()); + } else { + const std::string msg = "Cannot resolve package main '" + target + "' in" + + pjson_url.ToFilePath() + ", imported from " + base.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + } } void ThrowExportsInvalid(Environment* env, @@ -857,13 +863,13 @@ void ThrowExportsInvalid(Environment* env, } } -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) { +Maybe ResolveExportsTargetString(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); @@ -901,68 +907,124 @@ Maybe ResolveExportsTarget(Environment* env, return Just(subpath_resolved); } +Maybe ResolveExportsTarget(Environment* env, + const URL& pjson_url, + Local target, + const std::string& subpath, + const std::string& pkg_subpath, + const URL& base, + bool throw_invalid = true) { + Isolate* isolate = env->isolate(); + Local context = env->context(); + if (target->IsString()) { + Utf8Value target_utf8(isolate, target.As()); + std::string target_str(*target_utf8, target_utf8.length()); + Maybe resolved = ResolveExportsTargetString(env, target_str, subpath, + pkg_subpath, pjson_url, base, throw_invalid); + if (resolved.IsNothing()) { + 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) { + if (throw_invalid) { + 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->IsArray()) { + Maybe resolved = ResolveExportsTarget(env, pjson_url, + target_item, subpath, pkg_subpath, base, false); + if (resolved.IsNothing()) continue; + return FinalizeResolution(env, resolved.FromJust(), base); + } + } + if (throw_invalid) { + auto invalid = target_arr->Get(context, length - 1).ToLocalChecked(); + Maybe resolved = ResolveExportsTarget(env, pjson_url, invalid, + subpath, pkg_subpath, base, true); + CHECK(resolved.IsNothing()); + } + return Nothing(); + } else if (target->IsObject()) { + Local target_obj = target.As(); + bool matched = false; + Local conditionalTarget; + if (target_obj->HasOwnProperty(context, env->node_string()).FromJust()) { + matched = true; + conditionalTarget = + target_obj->Get(context, env->node_string()).ToLocalChecked(); + Maybe resolved = ResolveExportsTarget(env, pjson_url, + conditionalTarget, subpath, pkg_subpath, base, false); + if (!resolved.IsNothing()) { + return resolved; + } + } + if (target_obj->HasOwnProperty(context, env->default_string()).FromJust()) { + matched = true; + conditionalTarget = + target_obj->Get(context, env->default_string()).ToLocalChecked(); + Maybe resolved = ResolveExportsTarget(env, pjson_url, + conditionalTarget, subpath, pkg_subpath, base, false); + if (!resolved.IsNothing()) { + return resolved; + } + } + if (matched && throw_invalid) { + Maybe resolved = ResolveExportsTarget(env, pjson_url, + conditionalTarget, subpath, pkg_subpath, base, true); + CHECK(resolved.IsNothing()); + return Nothing(); + } + } + if (throw_invalid) { + ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); + } + return Nothing(); +} + +bool FirstKeyStartsWithDot(Environment* env, Local exports) { + CHECK(exports->IsObject()); + Local context = env->context(); + Local exports_obj = exports.As(); + Local keys = + exports_obj->GetOwnPropertyNames(context).ToLocalChecked(); + if (keys->Length() > 0) { + Local key = keys->Get(context, 0).ToLocalChecked().As(); + Utf8Value key_utf8(env->isolate(), key); + std::string key_str(*key_utf8, key_utf8.length()); + return key_str.front() == '.'; + } + return false; +} + Maybe PackageMainResolve(Environment* env, const URL& pjson_url, const PackageConfig& pcfg, const URL& base) { if (pcfg.exists == Exists::Yes) { Isolate* isolate = env->isolate(); - Local context = env->context(); + if (!pcfg.exports.IsEmpty()) { Local exports = pcfg.exports.Get(isolate); - if (exports->IsString() || exports->IsObject() || exports->IsArray()) { - Local target; - if (!exports->IsObject()) { - target = exports; + if (exports->IsString() || exports->IsObject()) { + if (!exports->IsObject() || !FirstKeyStartsWithDot(env, exports)) { + return ResolveExportsTarget(env, pjson_url, exports, "", "", base, + true); } else { Local exports_obj = exports.As(); - Local dot_string = String::NewFromUtf8(env->isolate(), ".", - v8::NewStringType::kNormal).ToLocalChecked(); - target = - exports_obj->Get(env->context(), dot_string).ToLocalChecked(); - } - if (target->IsString()) { - Utf8Value target_utf8(isolate, target.As()); - std::string target(*target_utf8, target_utf8.length()); - Maybe resolved = ResolveExportsTarget(env, target, "", ".", - pjson_url, base); - if (resolved.IsNothing()) { - ThrowExportsInvalid(env, ".", 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, ".", 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, "", - ".", pjson_url, base, false); - if (resolved.IsNothing()) continue; - return FinalizeResolution(env, resolved.FromJust(), base); - } + if (exports_obj->HasOwnProperty(env->context(), env->dot_string()) + .FromJust()) { + Local target = + exports_obj->Get(env->context(), env->dot_string()) + .ToLocalChecked(); + return ResolveExportsTarget(env, pjson_url, target, "", "", base, + true); } - auto invalid = target_arr->Get(context, length - 1).ToLocalChecked(); - if (!invalid->IsString()) { - ThrowExportsInvalid(env, ".", 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, "", - ".", pjson_url, base); - CHECK(resolved.IsNothing()); - return Nothing(); - } else { - ThrowExportsInvalid(env, ".", target, pjson_url, base); - return Nothing(); } } } @@ -1002,7 +1064,7 @@ Maybe PackageExportsResolve(Environment* env, Isolate* isolate = env->isolate(); Local context = env->context(); Local exports = pcfg.exports.Get(isolate); - if (!exports->IsObject()) { + if (!exports->IsObject() || !FirstKeyStartsWithDot(env, exports)) { ThrowExportsNotFound(env, pkg_subpath, pjson_url, base); return Nothing(); } @@ -1012,49 +1074,12 @@ Maybe PackageExportsResolve(Environment* env, 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_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); + Maybe resolved = ResolveExportsTarget(env, pjson_url, target, "", + pkg_subpath, base); + if (resolved.IsNothing()) { return Nothing(); } + return FinalizeResolution(env, resolved.FromJust(), base); } Local best_match; @@ -1076,49 +1101,13 @@ Maybe PackageExportsResolve(Environment* env, 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); + + Maybe resolved = ResolveExportsTarget(env, pjson_url, target, subpath, + pkg_subpath, base); + if (resolved.IsNothing()) { return Nothing(); } + return FinalizeResolution(env, resolved.FromJust(), base); } ThrowExportsNotFound(env, pkg_subpath, pjson_url, base); diff --git a/src/node_options.cc b/src/node_options.cc index 695d7cee6541cc..8ba6068673f23b 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -331,6 +331,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "experimental ES Module support and caching modules", &EnvironmentOptions::experimental_modules, kAllowedInEnvironment); + AddOption("--experimental-require-target", + "experimental support for conditional require mapping", + &EnvironmentOptions::experimental_require_target, + kAllowedInEnvironment); AddOption("--experimental-resolve-self", "experimental support for require/import of the current package", &EnvironmentOptions::experimental_resolve_self, diff --git a/src/node_options.h b/src/node_options.h index a4af15e3e00c31..8539a91cece67f 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -103,6 +103,7 @@ class EnvironmentOptions : public Options { bool enable_source_maps = false; bool experimental_json_modules = false; bool experimental_modules = false; + bool experimental_require_target = false; bool experimental_resolve_self = false; std::string es_module_specifier_resolution; bool experimental_wasm_modules = false; diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index d8c33994188138..523b0ec36b4a42 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -1,4 +1,4 @@ -// Flags: --experimental-modules --experimental-resolve-self +// Flags: --experimental-modules --experimental-resolve-self --experimental-require-target import { mustCall } from '../common/index.mjs'; import { ok, deepStrictEqual, strictEqual } from 'assert'; @@ -23,7 +23,14 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; ['pkgexports/fallbackfile', { default: 'asdf' }], // Dot main ['pkgexports', { default: 'asdf' }], + // Conditional split for require + ['pkgexports/condition', isRequire ? { default: 'encoded path' } : + { default: 'asdf' }], + // Conditional sugar for exports + ['pkgexports-direct', isRequire ? { default: 'encoded path' } : + { default: 'asdf' }] ]); + for (const [validSpecifier, expected] of validSpecifiers) { if (validSpecifier === null) continue; diff --git a/test/fixtures/node_modules/pkgexports-direct/asdf.js b/test/fixtures/node_modules/pkgexports-direct/asdf.js new file mode 100644 index 00000000000000..683f2d8ba623a7 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-direct/asdf.js @@ -0,0 +1 @@ +module.exports = 'asdf'; diff --git a/test/fixtures/node_modules/pkgexports-direct/package.json b/test/fixtures/node_modules/pkgexports-direct/package.json new file mode 100644 index 00000000000000..be2d70a615d436 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-direct/package.json @@ -0,0 +1,6 @@ +{ + "exports": { + "default": "./asdf.js", + "require": "./sp ce.js" + } +} diff --git a/test/fixtures/node_modules/pkgexports-direct/sp ce.js b/test/fixtures/node_modules/pkgexports-direct/sp ce.js new file mode 100644 index 00000000000000..570237506e4586 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-direct/sp ce.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = 'encoded path'; diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index 38e2fc1a5c02f6..37c28cdc1a950f 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -1,7 +1,7 @@ { "name": "@pkgexports/name", + "main": "./asdf.js", "exports": { - ".": "./asdf.js", "./hole": "./lib/hole.js", "./space": "./sp%20ce.js", "./valid-cjs": "./asdf.js", @@ -18,6 +18,7 @@ "./fallbackfile": [[], null, {}, "builtin:x", "./asdf.js"], "./nofallback1": [], "./nofallback2": [null, {}, "builtin:x"], - "./nodemodules": "./node_modules/internalpkg/x.js" + "./nodemodules": "./node_modules/internalpkg/x.js", + "./condition": [{ "require": "./sp ce.js" }, "./asdf.js"] } } From 69350a7da7f185f8a7f6df70d84e797bfea61c83 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 1 Nov 2019 15:50:40 -0400 Subject: [PATCH 03/13] fixup: nits --- doc/api/esm.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index bed3bb60e0d07f..77605d979ab399 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -393,7 +393,7 @@ have a different exported value for CommonJS and ES modules, which can be a hazard in that it can result in having two separate instances of the same package in use in an application, which can cause a number of bugs. -Other conditions such as `"browser"`, `"electron"`, `"deno"`, `"react-native"` +Other conditions such as `"browser"`, `"electron"`, `"deno"`, `"react-native"`, etc. could be defined in other runtimes or tools. ## import Specifiers From a7db4603e44b8df189e4653a7dd23d5d4448b42a Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 1 Nov 2019 15:59:17 -0400 Subject: [PATCH 04/13] include conditional exports link --- doc/api/esm.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/api/esm.md b/doc/api/esm.md index 77605d979ab399..e1ac2a55193076 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -260,6 +260,9 @@ that would only be supported in ES module-supporting versions of Node.js (and other runtimes). New packages could be published containing only ES module sources, and would be compatible only with ES module-supporting runtimes. +To define separate package entry points for use by `require` and by `import`, +see [Conditional Exports][]. + ### Package Exports By default, all subpaths from a package can be imported (`import 'pkg/x.js'`). @@ -1082,6 +1085,7 @@ success! ``` [CommonJS]: modules.html +[Conditional Exports]: #esm_conditional_exports [ECMAScript-modules implementation]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md [ES Module Integration Proposal for Web Assembly]: https://github.com/webassembly/esm-integration [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md From b7db9631b2fe6eeda65a5014a00c0690f5083f35 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 1 Nov 2019 17:22:26 -0400 Subject: [PATCH 05/13] remove support for "node" condition initially --- doc/api/esm.md | 6 ++---- lib/internal/modules/cjs/loader.js | 8 -------- src/env.h | 1 - src/module_wrap.cc | 10 ---------- 4 files changed, 2 insertions(+), 23 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index e1ac2a55193076..9cb96bf9d3a8b9 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -385,9 +385,7 @@ The conditions supported in Node.js are matched in the following order: 1. `"require"` - matched when the package is loaded via `require()`. This is currently only supported behind the `--experimental-require-target` flag. -2. `"node"` - matched for any Node.js environment. Can be a CommonJS or ES - module file. -3. `"default"` - the generic fallback that will always match if no other +2. `"default"` - the generic fallback that will always match if no other more specific condition is matched first. Can be a CommonJS or ES module file. @@ -848,7 +846,7 @@ of these top-level routines unless stated otherwise. _isMain_ is **true** when resolving the Node.js application entry point. _defaultEnv_ is the conditional environment name priority array, -`["node", "default"]`. +`["default"]`.
Resolver algorithm specification diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index bfc5d47ad84e5e..ccaf5a3d1860e1 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -568,14 +568,6 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) { if (e.code !== 'MODULE_NOT_FOUND') throw e; } } - if (ObjectPrototype.hasOwnProperty(target, 'node')) { - try { - return resolveExportsTarget(pkgPath, target.node, subpath, - basePath, mappingKey); - } catch (e) { - if (e.code !== 'MODULE_NOT_FOUND') throw e; - } - } if (ObjectPrototype.hasOwnProperty(target, 'default')) { try { return resolveExportsTarget(pkgPath, target.default, subpath, diff --git a/src/env.h b/src/env.h index 223337704abca2..9680b9dd2bceff 100644 --- a/src/env.h +++ b/src/env.h @@ -280,7 +280,6 @@ constexpr size_t kFsStatsBufferLength = V(netmask_string, "netmask") \ V(next_string, "next") \ V(nistcurve_string, "nistCurve") \ - V(node_string, "node") \ V(nsname_string, "nsname") \ V(ocsp_request_string, "OCSPRequest") \ V(oncertcb_string, "oncertcb") \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 0a4f53ff62bb22..97d0b2590c6942 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -954,16 +954,6 @@ Maybe ResolveExportsTarget(Environment* env, Local target_obj = target.As(); bool matched = false; Local conditionalTarget; - if (target_obj->HasOwnProperty(context, env->node_string()).FromJust()) { - matched = true; - conditionalTarget = - target_obj->Get(context, env->node_string()).ToLocalChecked(); - Maybe resolved = ResolveExportsTarget(env, pjson_url, - conditionalTarget, subpath, pkg_subpath, base, false); - if (!resolved.IsNothing()) { - return resolved; - } - } if (target_obj->HasOwnProperty(context, env->default_string()).FromJust()) { matched = true; conditionalTarget = From 9f635a02a748db44d9818e1d89c09d9f4ef1aeb1 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 2 Nov 2019 16:18:56 -0400 Subject: [PATCH 06/13] fixup sugar removal --- lib/internal/modules/cjs/loader.js | 14 +------ src/module_wrap.cc | 41 ++++++------------- test/es-module/test-esm-exports.mjs | 3 -- .../node_modules/pkgexports-direct/asdf.js | 1 - .../pkgexports-direct/package.json | 6 --- .../node_modules/pkgexports-direct/sp ce.js | 3 -- 6 files changed, 14 insertions(+), 54 deletions(-) delete mode 100644 test/fixtures/node_modules/pkgexports-direct/asdf.js delete mode 100644 test/fixtures/node_modules/pkgexports-direct/package.json delete mode 100644 test/fixtures/node_modules/pkgexports-direct/sp ce.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index ccaf5a3d1860e1..28e1eb20f06177 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -457,7 +457,7 @@ function applyExports(basePath, expansion) { if (pkgExports === undefined || pkgExports === null || !experimentalModules) return path.resolve(basePath, mappingKey); - if (typeof pkgExports === 'object' && firstKeyStartsWithDot(pkgExports)) { + if (typeof pkgExports === 'object') { if (ObjectPrototype.hasOwnProperty(pkgExports, mappingKey)) { const mapping = pkgExports[mappingKey]; return resolveExportsTarget(pathToFileURL(basePath + '/'), mapping, '', @@ -484,8 +484,7 @@ function applyExports(basePath, expansion) { subpath, basePath, mappingKey); } } - if ((mappingKey === '.' && typeof pkgExports === 'string') || - (typeof pkgExports === 'object' && !firstKeyStartsWithDot(pkgExports))) { + if ((mappingKey === '.' && typeof pkgExports === 'string')) { return resolveExportsTarget(pathToFileURL(basePath + '/'), pkgExports, '', basePath, mappingKey); } @@ -520,15 +519,6 @@ function resolveExports(nmPath, request, absoluteRequest) { return path.resolve(nmPath, request); } -function firstKeyStartsWithDot(exports) { - for (const p in exports) { - if (!ObjectPrototype.hasOwnProperty(exports, p)) - continue; - return p[0] === '.'; - } - return false; -} - function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) { if (typeof target === 'string') { if (target.startsWith('./') && diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 97d0b2590c6942..d1b63b6b349ec0 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -977,21 +977,6 @@ Maybe ResolveExportsTarget(Environment* env, return Nothing(); } -bool FirstKeyStartsWithDot(Environment* env, Local exports) { - CHECK(exports->IsObject()); - Local context = env->context(); - Local exports_obj = exports.As(); - Local keys = - exports_obj->GetOwnPropertyNames(context).ToLocalChecked(); - if (keys->Length() > 0) { - Local key = keys->Get(context, 0).ToLocalChecked().As(); - Utf8Value key_utf8(env->isolate(), key); - std::string key_str(*key_utf8, key_utf8.length()); - return key_str.front() == '.'; - } - return false; -} - Maybe PackageMainResolve(Environment* env, const URL& pjson_url, const PackageConfig& pcfg, @@ -1001,20 +986,18 @@ Maybe PackageMainResolve(Environment* env, if (!pcfg.exports.IsEmpty()) { Local exports = pcfg.exports.Get(isolate); - if (exports->IsString() || exports->IsObject()) { - if (!exports->IsObject() || !FirstKeyStartsWithDot(env, exports)) { - return ResolveExportsTarget(env, pjson_url, exports, "", "", base, + if (exports->IsString()) { + return ResolveExportsTarget(env, pjson_url, exports, "", "", base, + true); + } else if (exports->IsObject()) { + Local exports_obj = exports.As(); + if (exports_obj->HasOwnProperty(env->context(), env->dot_string()) + .FromJust()) { + Local target = + exports_obj->Get(env->context(), env->dot_string()) + .ToLocalChecked(); + return ResolveExportsTarget(env, pjson_url, target, "", "", base, true); - } else { - Local exports_obj = exports.As(); - if (exports_obj->HasOwnProperty(env->context(), env->dot_string()) - .FromJust()) { - Local target = - exports_obj->Get(env->context(), env->dot_string()) - .ToLocalChecked(); - return ResolveExportsTarget(env, pjson_url, target, "", "", base, - true); - } } } } @@ -1054,7 +1037,7 @@ Maybe PackageExportsResolve(Environment* env, Isolate* isolate = env->isolate(); Local context = env->context(); Local exports = pcfg.exports.Get(isolate); - if (!exports->IsObject() || !FirstKeyStartsWithDot(env, exports)) { + if (!exports->IsObject()) { 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 523b0ec36b4a42..004b80f592a1d9 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -25,9 +25,6 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; ['pkgexports', { default: 'asdf' }], // Conditional split for require ['pkgexports/condition', isRequire ? { default: 'encoded path' } : - { default: 'asdf' }], - // Conditional sugar for exports - ['pkgexports-direct', isRequire ? { default: 'encoded path' } : { default: 'asdf' }] ]); diff --git a/test/fixtures/node_modules/pkgexports-direct/asdf.js b/test/fixtures/node_modules/pkgexports-direct/asdf.js deleted file mode 100644 index 683f2d8ba623a7..00000000000000 --- a/test/fixtures/node_modules/pkgexports-direct/asdf.js +++ /dev/null @@ -1 +0,0 @@ -module.exports = 'asdf'; diff --git a/test/fixtures/node_modules/pkgexports-direct/package.json b/test/fixtures/node_modules/pkgexports-direct/package.json deleted file mode 100644 index be2d70a615d436..00000000000000 --- a/test/fixtures/node_modules/pkgexports-direct/package.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "exports": { - "default": "./asdf.js", - "require": "./sp ce.js" - } -} diff --git a/test/fixtures/node_modules/pkgexports-direct/sp ce.js b/test/fixtures/node_modules/pkgexports-direct/sp ce.js deleted file mode 100644 index 570237506e4586..00000000000000 --- a/test/fixtures/node_modules/pkgexports-direct/sp ce.js +++ /dev/null @@ -1,3 +0,0 @@ -'use strict'; - -module.exports = 'encoded path'; From 0344847b92f9f86523b6e50b08fe734dcf8d6d43 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 2 Nov 2019 16:01:16 -0400 Subject: [PATCH 07/13] rename --experimental-require-target to --experimental-dual-resolution --- doc/api/cli.md | 18 +++++++++--------- doc/api/esm.md | 2 +- doc/node.1 | 4 ++-- lib/internal/modules/cjs/loader.js | 6 +++--- src/node_options.cc | 6 +++--- src/node_options.h | 2 +- test/es-module/test-esm-exports.mjs | 2 +- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index e370579249f56e..83dcb268e1c1f8 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -170,6 +170,14 @@ the ability to import a directory that has an index file. Please see [customizing esm specifier resolution][] for example usage. +### `--experimental-dual-resolution + + +Enable experimental support for separate resolution paths for `require()` and +`import`. See [Conditional Exports][] for more information. + ### `--experimental-json-modules` - -Enable experimental support for a `"require"` conditional package export target. -See [Conditional Exports][] for more information. - ### `--experimental-resolve-self` +```js +{ + "exports": { + ".": "./main.js" + } +} +``` + +can be written: + + +```js +{ + "exports": "./main.js" +} +``` + +When using conditional exports, the rule is that all keys in the object mapping +must not start with a `"."` otherwise they would be indistinguishable from +exports subpaths. + + +```js +{ + "exports": { + ".": { + "require": "./main.cjs", + "default": "./main.js" + } + } +} +``` + +can be written: + + +```js +{ + "exports": { + "require": "./main.cjs", + "default": "./main.js" + } +} +``` + +If writing any exports value that mixes up these two forms, an error will be +thrown: + + +```js +{ + // Throws on resolution! + "exports": { + "./feature": "./lib/feature.js", + "require": "./main.cjs", + "default": "./main.js" + } +} +``` + ## import Specifiers ### Terminology @@ -947,8 +1016,10 @@ _defaultEnv_ is the conditional environment name priority array, > 1. If _pjson_ is **null**, then > 1. Throw a _Module Not Found_ error. > 1. If _pjson.exports_ is not **null** or **undefined**, then -> 1. If _pjson.exports_ is a String or Array, or an Object whose first key -> does not start with _"."_, then +> 1. If _exports_ is an Object with both a key starting with _"."_ and a key +> not starting with _"."_, throw a "Invalid Package Configuration" error. +> 1. If _pjson.exports_ is a String or Array, or an Object containing no +> keys starting with _"."_, then > 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, > _pjson.exports_, _""_). > 1. If _pjson.exports_ is an Object containing a _"."_ property, then @@ -968,8 +1039,9 @@ _defaultEnv_ is the conditional environment name priority array, > 1. Return _legacyMainURL_. **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, _packagePath_, _exports_) - -> 1. If _exports_ is an Object, then +> 1. If _exports_ is an Object with both a key starting with _"."_ and a key not +> starting with _"."_, throw an "Invalid Package Configuration" error. +> 1. If _exports_ is an Object and all keys of _exports_ start with _"."_, then > 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\]_. diff --git a/doc/api/modules.md b/doc/api/modules.md index cf6005deb16722..d6856629210be7 100644 --- a/doc/api/modules.md +++ b/doc/api/modules.md @@ -232,8 +232,10 @@ RESOLVE_BARE_SPECIFIER(DIR, X) 2. If X matches this pattern and DIR/name/package.json is a file: a. Parse DIR/name/package.json, and look for "exports" field. b. If "exports" is null or undefined, GOTO 3. - c. If "exports" is a string, or object whose first key does not start with - ".", treat it as having that value as its "." object property. + c. If "exports" is an object with some keys starting with "." and some keys + not starting with ".", throw "invalid config". + c. If "exports" is a string, or object with no keys starting with ".", treat + it as having that value as its "." object property. d. If subpath is "." and "exports" does not have a "." entry, GOTO 3. e. Find the longest key in "exports" that the subpath starts with. f. If no such key can be found, throw "not found". diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 2684931a77e299..6d4a582631810c 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -981,7 +981,7 @@ E('ERR_INVALID_OPT_VALUE', (name, value) => E('ERR_INVALID_OPT_VALUE_ENCODING', 'The value "%s" is invalid for option "encoding"', TypeError); E('ERR_INVALID_PACKAGE_CONFIG', - 'Invalid package config in \'%s\' imported from %s', Error); + 'Invalid package config for \'%s\', %s', Error); E('ERR_INVALID_PERFORMANCE_MARK', 'The "%s" performance mark has not been set', Error); E('ERR_INVALID_PROTOCOL', diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 8730b9f30d8013..76fcd4902e7751 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -69,6 +69,7 @@ const { compileFunction } = internalBinding('contextify'); const { ERR_INVALID_ARG_VALUE, ERR_INVALID_OPT_VALUE, + ERR_INVALID_PACKAGE_CONFIG, ERR_REQUIRE_ESM } = require('internal/errors').codes; const { validateString } = require('internal/validators'); @@ -450,13 +451,39 @@ function trySelf(paths, exts, isMain, trailingSlash, request) { } } +function isConditionalDotExportSugar(exports, basePath) { + if (typeof exports === 'string') + return true; + if (Array.isArray(exports)) + return true; + if (typeof exports !== 'object') + return false; + let isConditional = false; + let firstCheck = true; + for (const key of Object.keys(exports)) { + const curIsConditional = key[0] !== '.'; + if (firstCheck) { + firstCheck = false; + isConditional = curIsConditional; + } else if (isConditional !== curIsConditional) { + throw new ERR_INVALID_PACKAGE_CONFIG(basePath, '"exports" must either ' + + 'be an object of package subpath keys starting with \'.\', or an ' + + 'object of main entry condition name keys not starting with \'.\''); + } + } + return isConditional; +} + function applyExports(basePath, expansion) { const mappingKey = `.${expansion}`; - const pkgExports = readPackageExports(basePath); + let pkgExports = readPackageExports(basePath); if (pkgExports === undefined || pkgExports === null || !experimentalModules) return path.resolve(basePath, mappingKey); + if (isConditionalDotExportSugar(pkgExports, basePath)) + pkgExports = { '.': pkgExports }; + if (typeof pkgExports === 'object') { if (ObjectPrototype.hasOwnProperty(pkgExports, mappingKey)) { const mapping = pkgExports[mappingKey]; @@ -484,10 +511,6 @@ function applyExports(basePath, expansion) { subpath, basePath, mappingKey); } } - if ((mappingKey === '.' && typeof pkgExports === 'string')) { - return resolveExportsTarget(pathToFileURL(basePath + '/'), pkgExports, - '', basePath, mappingKey); - } // Fallback to CJS main lookup when no main export is defined if (mappingKey === '.') return basePath; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index d1b63b6b349ec0..aaa067342c0947 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -977,6 +977,36 @@ Maybe ResolveExportsTarget(Environment* env, return Nothing(); } +Maybe IsConditionalExportsMainSugar(Environment* env, + Local exports, + const URL& pjson_url, + const URL& base) { + if (exports->IsString() || exports->IsArray()) return Just(true); + if (!exports->IsObject()) return Just(false); + Local context = env->context(); + Local exports_obj = exports.As(); + Local keys = + exports_obj->GetOwnPropertyNames(context).ToLocalChecked(); + bool isConditionalSugar = false; + for (uint32_t i = 0; i < keys->Length(); ++i) { + Local key = keys->Get(context, i).ToLocalChecked().As(); + Utf8Value key_utf8(env->isolate(), key); + bool curIsConditionalSugar = key_utf8.length() == 0 || key_utf8[0] != '.'; + if (i == 0) { + isConditionalSugar = curIsConditionalSugar; + } else if (isConditionalSugar != curIsConditionalSugar) { + const std::string msg = "Cannot resolve package exports in " + + pjson_url.ToFilePath() + ", imported from " + base.ToFilePath() + ". " + + "\"exports\" must either be an object of package subpath keys " + + "starting with '.', or an object of main entry condition name keys " + + "not starting with '.'"; + node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str()); + return Nothing(); + } + } + return Just(isConditionalSugar); +} + Maybe PackageMainResolve(Environment* env, const URL& pjson_url, const PackageConfig& pcfg, @@ -986,7 +1016,11 @@ Maybe PackageMainResolve(Environment* env, if (!pcfg.exports.IsEmpty()) { Local exports = pcfg.exports.Get(isolate); - if (exports->IsString()) { + Maybe isConditionalExportsMainSugar = + IsConditionalExportsMainSugar(env, exports, pjson_url, base); + if (isConditionalExportsMainSugar.IsNothing()) + return Nothing(); + if (isConditionalExportsMainSugar.FromJust()) { return ResolveExportsTarget(env, pjson_url, exports, "", "", base, true); } else if (exports->IsObject()) { @@ -1037,7 +1071,11 @@ Maybe PackageExportsResolve(Environment* env, Isolate* isolate = env->isolate(); Local context = env->context(); Local exports = pcfg.exports.Get(isolate); - if (!exports->IsObject()) { + Maybe isConditionalExportsMainSugar = + IsConditionalExportsMainSugar(env, exports, pjson_url, base); + if (isConditionalExportsMainSugar.IsNothing()) + return Nothing(); + if (!exports->IsObject() || isConditionalExportsMainSugar.FromJust()) { 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 c2c03f2bfe0d0a..90eeecda60b877 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -25,7 +25,12 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; ['pkgexports', { default: 'asdf' }], // Conditional split for require ['pkgexports/condition', isRequire ? { default: 'encoded path' } : - { default: 'asdf' }] + { default: 'asdf' }], + // String exports sugar + ['pkgexports-sugar', { default: 'main' }], + // Conditional object exports sugar + ['pkgexports-sugar2', isRequire ? { default: 'not-exported' } : + { default: 'main' }] ]); for (const [validSpecifier, expected] of validSpecifiers) { @@ -43,6 +48,9 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.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 `{}`. ['pkgexports-number/hidden.js', './hidden.js'], + // Sugar cases still encapsulate + ['pkgexports-sugar/not-exported.js', './not-exported.js'], + ['pkgexports-sugar2/not-exported.js', './not-exported.js'] ]); const invalidExports = new Map([ @@ -83,7 +91,7 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; assertStartsWith(err.message, (isRequire ? 'Package exports' : 'Cannot resolve')); assertIncludes(err.message, isRequire ? - `do not define a valid '${subpath}' subpath` : + `do not define a valid '${subpath}' target` : `matched for '${subpath}'`); })); } @@ -97,11 +105,21 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; 'Cannot find module'); })); - // THe use of %2F escapes in paths fails loading + // 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'); })); + + // Sugar conditional exports main mixed failure case + loadFixture('pkgexports-sugar-fail').catch(mustCall((err) => { + strictEqual(err.code, 'ERR_INVALID_PACKAGE_CONFIG'); + assertStartsWith(err.message, (isRequire ? 'Invalid package' : + 'Cannot resolve')); + assertIncludes(err.message, '"exports" must either be an object of ' + + 'package subpath keys starting with \'.\', or an object of main ' + + 'entry condition name keys not starting with \'.\''); + })); }); const { requireFromInside, importFromInside } = fromInside; @@ -128,6 +146,6 @@ function assertStartsWith(actual, expected) { } function assertIncludes(actual, expected) { - ok(actual.toString().indexOf(expected), + ok(actual.toString().indexOf(expected) !== -1, `${JSON.stringify(actual)} includes ${JSON.stringify(expected)}`); } diff --git a/test/fixtures/node_modules/pkgexports-sugar-fail/main.js b/test/fixtures/node_modules/pkgexports-sugar-fail/main.js new file mode 100644 index 00000000000000..dfdd47b877319c --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-sugar-fail/main.js @@ -0,0 +1 @@ +module.exports = 'main'; diff --git a/test/fixtures/node_modules/pkgexports-sugar-fail/not-exported.js b/test/fixtures/node_modules/pkgexports-sugar-fail/not-exported.js new file mode 100644 index 00000000000000..02e146dbe90985 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-sugar-fail/not-exported.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = 'not-exported'; diff --git a/test/fixtures/node_modules/pkgexports-sugar-fail/package.json b/test/fixtures/node_modules/pkgexports-sugar-fail/package.json new file mode 100644 index 00000000000000..0fb05a427a76e2 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-sugar-fail/package.json @@ -0,0 +1,6 @@ +{ + "exports": { + "default": "./main.js", + "./main": "./main.js" + } +} diff --git a/test/fixtures/node_modules/pkgexports-sugar/main.js b/test/fixtures/node_modules/pkgexports-sugar/main.js new file mode 100644 index 00000000000000..dfdd47b877319c --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-sugar/main.js @@ -0,0 +1 @@ +module.exports = 'main'; diff --git a/test/fixtures/node_modules/pkgexports-sugar/not-exported.js b/test/fixtures/node_modules/pkgexports-sugar/not-exported.js new file mode 100644 index 00000000000000..02e146dbe90985 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-sugar/not-exported.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = 'not-exported'; diff --git a/test/fixtures/node_modules/pkgexports-sugar/package.json b/test/fixtures/node_modules/pkgexports-sugar/package.json new file mode 100644 index 00000000000000..5ebad0b4bda380 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-sugar/package.json @@ -0,0 +1,3 @@ +{ + "exports": "./main.js" +} diff --git a/test/fixtures/node_modules/pkgexports-sugar2/main.js b/test/fixtures/node_modules/pkgexports-sugar2/main.js new file mode 100644 index 00000000000000..dfdd47b877319c --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-sugar2/main.js @@ -0,0 +1 @@ +module.exports = 'main'; diff --git a/test/fixtures/node_modules/pkgexports-sugar2/not-exported.js b/test/fixtures/node_modules/pkgexports-sugar2/not-exported.js new file mode 100644 index 00000000000000..02e146dbe90985 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-sugar2/not-exported.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = 'not-exported'; diff --git a/test/fixtures/node_modules/pkgexports-sugar2/package.json b/test/fixtures/node_modules/pkgexports-sugar2/package.json new file mode 100644 index 00000000000000..139b06665d85e0 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports-sugar2/package.json @@ -0,0 +1,6 @@ +{ + "exports": { + "require": "./not-exported.js", + "default": "./main.js" + } +} From cf9f64c2e912f7830954ee3f0d228a9977089978 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 6 Nov 2019 18:32:43 -0500 Subject: [PATCH 09/13] Revert "remove support for "node" condition initially" This reverts commit b7db9631b2fe6eeda65a5014a00c0690f5083f35. --- doc/api/esm.md | 6 ++++-- lib/internal/modules/cjs/loader.js | 8 ++++++++ src/env.h | 1 + src/module_wrap.cc | 10 ++++++++++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 834cbf8f84b83c..3642144bb8900d 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -385,7 +385,9 @@ The conditions supported in Node.js are matched in the following order: 1. `"require"` - matched when the package is loaded via `require()`. This is currently only supported behind the `--experimental-dual-resolution` flag. -2. `"default"` - the generic fallback that will always match if no other +2. `"node"` - matched for any Node.js environment. Can be a CommonJS or ES + module file. +3. `"default"` - the generic fallback that will always match if no other more specific condition is matched first. Can be a CommonJS or ES module file. @@ -915,7 +917,7 @@ of these top-level routines unless stated otherwise. _isMain_ is **true** when resolving the Node.js application entry point. _defaultEnv_ is the conditional environment name priority array, -`["default"]`. +`["node", "default"]`.
Resolver algorithm specification diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 76fcd4902e7751..c64904d4b09834 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -581,6 +581,14 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) { if (e.code !== 'MODULE_NOT_FOUND') throw e; } } + if (ObjectPrototype.hasOwnProperty(target, 'node')) { + try { + return resolveExportsTarget(pkgPath, target.node, subpath, + basePath, mappingKey); + } catch (e) { + if (e.code !== 'MODULE_NOT_FOUND') throw e; + } + } if (ObjectPrototype.hasOwnProperty(target, 'default')) { try { return resolveExportsTarget(pkgPath, target.default, subpath, diff --git a/src/env.h b/src/env.h index 9680b9dd2bceff..223337704abca2 100644 --- a/src/env.h +++ b/src/env.h @@ -280,6 +280,7 @@ constexpr size_t kFsStatsBufferLength = V(netmask_string, "netmask") \ V(next_string, "next") \ V(nistcurve_string, "nistCurve") \ + V(node_string, "node") \ V(nsname_string, "nsname") \ V(ocsp_request_string, "OCSPRequest") \ V(oncertcb_string, "oncertcb") \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index aaa067342c0947..3d04de46ca5881 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -954,6 +954,16 @@ Maybe ResolveExportsTarget(Environment* env, Local target_obj = target.As(); bool matched = false; Local conditionalTarget; + if (target_obj->HasOwnProperty(context, env->node_string()).FromJust()) { + matched = true; + conditionalTarget = + target_obj->Get(context, env->node_string()).ToLocalChecked(); + Maybe resolved = ResolveExportsTarget(env, pjson_url, + conditionalTarget, subpath, pkg_subpath, base, false); + if (!resolved.IsNothing()) { + return resolved; + } + } if (target_obj->HasOwnProperty(context, env->default_string()).FromJust()) { matched = true; conditionalTarget = From 1a6a2c5993dbec47bdf147f440a052c10b095b0b Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 6 Nov 2019 18:41:14 -0500 Subject: [PATCH 10/13] --experimental-conditional-exports flag for both "node" and "require" conditions --- doc/api/cli.md | 9 +++++---- doc/api/esm.md | 7 ++++--- doc/node.1 | 6 +++--- lib/internal/modules/cjs/loader.js | 9 +++++---- src/module_wrap.cc | 3 ++- src/node_options.cc | 6 +++--- src/node_options.h | 2 +- test/es-module/test-esm-exports.mjs | 2 +- 8 files changed, 24 insertions(+), 20 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 83dcb268e1c1f8..f7a59a5f238061 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -170,13 +170,14 @@ the ability to import a directory that has an index file. Please see [customizing esm specifier resolution][] for example usage. -### `--experimental-dual-resolution +### `--experimental-conditional-exports -Enable experimental support for separate resolution paths for `require()` and -`import`. See [Conditional Exports][] for more information. +Enable experimental support for the `"require"` and `"ndoe"` conditional +package export resolutions. +See [Conditional Exports][] for more information. ### `--experimental-json-modules` -Enable experimental support for the `"require"` and `"ndoe"` conditional +Enable experimental support for the `"require"` and `"node"` conditional package export resolutions. See [Conditional Exports][] for more information. From 17eb71fe089f811254e6dee73e472ef4c4da5fde Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Thu, 7 Nov 2019 15:22:11 -0500 Subject: [PATCH 12/13] remove unclear line --- doc/api/esm.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 082d2ec02914de..c1ae58ed6c498a 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -373,10 +373,6 @@ Node.js and the browser can be written: } ``` -When an exports target condition object is provided like the above, the first -matching key in the object that is supported by the current environment will -be resolved. - When resolving the `"."` export, if no matching target is found, the `"main"` will be used as the final fallback. From 9ef939a9f46bc798798bb0b0b2523debc941a9ed Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Thu, 7 Nov 2019 15:35:44 -0500 Subject: [PATCH 13/13] mixed case error rewording --- lib/internal/modules/cjs/loader.js | 7 ++++--- src/module_wrap.cc | 6 +++--- test/es-module/test-esm-exports.mjs | 7 ++++--- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 428d6153f55c9f..243b9867cdc999 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -466,9 +466,10 @@ function isConditionalDotExportSugar(exports, basePath) { firstCheck = false; isConditional = curIsConditional; } else if (isConditional !== curIsConditional) { - throw new ERR_INVALID_PACKAGE_CONFIG(basePath, '"exports" must either ' + - 'be an object of package subpath keys starting with \'.\', or an ' + - 'object of main entry condition name keys not starting with \'.\''); + throw new ERR_INVALID_PACKAGE_CONFIG(basePath, '"exports" cannot ' + + 'contain some keys starting with \'.\' and some not. The exports ' + + 'object must either be an object of package subpath keys or an ' + + 'object of main entry condition name keys only.'); } } return isConditional; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index d7277e50f415c4..5745cce9e099ab 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -1008,9 +1008,9 @@ Maybe IsConditionalExportsMainSugar(Environment* env, } else if (isConditionalSugar != curIsConditionalSugar) { const std::string msg = "Cannot resolve package exports in " + pjson_url.ToFilePath() + ", imported from " + base.ToFilePath() + ". " + - "\"exports\" must either be an object of package subpath keys " + - "starting with '.', or an object of main entry condition name keys " + - "not starting with '.'"; + "\"exports\" cannot contain some keys starting with '.' and some not." + + " The exports object must either be an object of package subpath keys" + + " or an object of main entry condition name keys only."; node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str()); return Nothing(); } diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index ebbe7b47cca5bc..2683b5df68e9fa 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -116,9 +116,10 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; strictEqual(err.code, 'ERR_INVALID_PACKAGE_CONFIG'); assertStartsWith(err.message, (isRequire ? 'Invalid package' : 'Cannot resolve')); - assertIncludes(err.message, '"exports" must either be an object of ' + - 'package subpath keys starting with \'.\', or an object of main ' + - 'entry condition name keys not starting with \'.\''); + assertIncludes(err.message, '"exports" cannot contain some keys starting ' + + 'with \'.\' and some not. The exports object must either be an object of ' + + 'package subpath keys or an object of main entry condition name keys ' + + 'only.'); })); });