Skip to content

Commit

Permalink
module: logical conditional exports ordering
Browse files Browse the repository at this point in the history
PR-URL: #31008
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
  • Loading branch information
guybedford authored and MylesBorins committed Jan 16, 2020
1 parent 94af94a commit f62fb76
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 86 deletions.
48 changes: 27 additions & 21 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -343,33 +343,36 @@ Node.js and the browser can be written:
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:
The conditions supported in Node.js condition matching:

1. `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
* `"default"` - the generic fallback that will always match. Can be a CommonJS
or ES module file.
* `"import"` - matched when the package is loaded via `import` or
`import()`. Can be any module format, this field does not set the type
interpretation. _This is currently only supported behind the
`--experimental-conditional-exports` flag._
* `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
module file. _This is currently only supported behind the
`--experimental-conditional-exports` flag._
2. `"require"` - matched when the package is loaded via `require()`.
* `"require"` - matched when the package is loaded via `require()`.
_This is currently only supported behind the
`--experimental-conditional-exports` flag._
3. `"import"` - matched when the package is loaded via `import` or
`import()`. Can be any module format, this field does not set the type
interpretation. _This is currently only supported behind the
`--experimental-conditional-exports` flag._
4. `"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.

> Setting any of the above flagged conditions for a published package is not
> recommended until they are unflagged to avoid breaking changes to packages in
> future.
Condition matching is applied in object order from first to last within the
`"exports"` object.

> Setting the above conditions for a published package is not recommended until
> conditional exports have been unflagged to avoid breaking changes to packages.
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.
etc. could be defined in other runtimes or tools. Condition names must not start
with `"."` or be numbers. Further restrictions, definitions or guidance on
condition names may be provided in future.

#### Exports Sugar

Expand Down Expand Up @@ -1547,13 +1550,15 @@ _defaultEnv_ is the conditional environment name priority array,
> 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. If _exports_ contains any index property keys, as defined in ECMA-262
> [6.1.7 Array Index][], throw an _Invalid Package Configuration_ error.
> 1. For each property _p_ of _target_, in object insertion order as,
> 1. If _env_ contains an entry for _p_, then
> 1. Let _targetValue_ be the value of the _p_ property in _target_.
> 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 an Array, continue the loop.
Expand Down Expand Up @@ -1649,3 +1654,4 @@ success!
[special scheme]: https://url.spec.whatwg.org/#special-scheme
[the official standard format]: https://tc39.github.io/ecma262/#sec-modules
[transpiler loader example]: #esm_transpiler_loader
[6.1.7 Array Index]: https://tc39.es/ecma262/#integer-index
68 changes: 41 additions & 27 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@ const {
Error,
JSONParse,
Map,
Number,
ObjectCreate,
ObjectDefineProperty,
ObjectFreeze,
ObjectIs,
ObjectKeys,
ObjectPrototypeHasOwnProperty,
ReflectSet,
SafeMap,
String,
StringPrototypeIndexOf,
StringPrototypeMatch,
StringPrototypeSlice,
Expand Down Expand Up @@ -554,6 +557,18 @@ function resolveExports(nmPath, request, absoluteRequest) {
return path.resolve(nmPath, request);
}

function isArrayIndex(p) {
assert(typeof p === 'string');
const n = Number(p);
if (String(n) !== p)
return false;
if (ObjectIs(n, +0))
return true;
if (!Number.isInteger(n))
return false;
return n >= 0 && n < (2 ** 32) - 1;
}

function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
if (typeof target === 'string') {
if (target.startsWith('./') &&
Expand Down Expand Up @@ -584,34 +599,33 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
}
}
} else if (typeof target === 'object' && target !== null) {
if (experimentalConditionalExports &&
ObjectPrototypeHasOwnProperty(target, 'node')) {
try {
const result = resolveExportsTarget(pkgPath, target.node, subpath,
basePath, mappingKey);
emitExperimentalWarning('Conditional exports');
return result;
} catch (e) {
if (e.code !== 'MODULE_NOT_FOUND') throw e;
}
}
if (experimentalConditionalExports &&
ObjectPrototypeHasOwnProperty(target, 'require')) {
try {
const result = resolveExportsTarget(pkgPath, target.require, subpath,
basePath, mappingKey);
emitExperimentalWarning('Conditional exports');
return result;
} catch (e) {
if (e.code !== 'MODULE_NOT_FOUND') throw e;
}
const keys = ObjectKeys(target);
if (keys.some(isArrayIndex)) {
throw new ERR_INVALID_PACKAGE_CONFIG(basePath, '"exports" cannot ' +
'contain numeric property keys.');
}
if (ObjectPrototypeHasOwnProperty(target, 'default')) {
try {
return resolveExportsTarget(pkgPath, target.default, subpath,
basePath, mappingKey);
} catch (e) {
if (e.code !== 'MODULE_NOT_FOUND') throw e;
for (const p of keys) {
switch (p) {
case 'node':
case 'require':
if (!experimentalConditionalExports)
continue;
try {
emitExperimentalWarning('Conditional exports');
const result = resolveExportsTarget(pkgPath, target[p], subpath,
basePath, mappingKey);
return result;
} catch (e) {
if (e.code !== 'MODULE_NOT_FOUND') throw e;
}
break;
case 'default':
try {
return resolveExportsTarget(pkgPath, target.default, subpath,
basePath, mappingKey);
} catch (e) {
if (e.code !== 'MODULE_NOT_FOUND') throw e;
}
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ 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") \
Expand Down Expand Up @@ -257,7 +256,6 @@ constexpr size_t kFsStatsBufferLength =
V(http_1_1_string, "http/1.1") \
V(identity_string, "identity") \
V(ignore_string, "ignore") \
V(import_string, "import") \
V(infoaccess_string, "infoAccess") \
V(inherit_string, "inherit") \
V(input_string, "input") \
Expand Down
94 changes: 59 additions & 35 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include <sys/stat.h> // S_IFDIR

#include <algorithm>
#include <climits> // PATH_MAX

namespace node {
namespace loader {
Expand Down Expand Up @@ -908,6 +907,25 @@ Maybe<URL> ResolveExportsTargetString(Environment* env,
return Just(subpath_resolved);
}

bool IsArrayIndex(Environment* env, Local<Value> p) {
Local<Context> context = env->context();
Local<String> p_str = p->ToString(context).ToLocalChecked();
double n_dbl = static_cast<double>(p_str->NumberValue(context).FromJust());
Local<Number> n = Number::New(env->isolate(), n_dbl);
Local<String> cmp_str = n->ToString(context).ToLocalChecked();
if (!p_str->Equals(context, cmp_str).FromJust()) {
return false;
}
if (n_dbl == 0 && std::signbit(n_dbl) == false) {
return true;
}
Local<Integer> cmp_integer;
if (!n->ToInteger(context).ToLocal(&cmp_integer)) {
return false;
}
return n_dbl > 0 && n_dbl < (2 ^ 32) - 1;
}

Maybe<URL> ResolveExportsTarget(Environment* env,
const URL& pjson_url,
Local<Value> target,
Expand Down Expand Up @@ -953,44 +971,50 @@ Maybe<URL> ResolveExportsTarget(Environment* env,
return Nothing<URL>();
} else if (target->IsObject()) {
Local<Object> target_obj = target.As<Object>();
bool matched = false;
Local<Array> target_obj_keys =
target_obj->GetOwnPropertyNames(context).ToLocalChecked();
Local<Value> conditionalTarget;
if (env->options()->experimental_conditional_exports &&
target_obj->HasOwnProperty(context, env->node_string()).FromJust()) {
matched = true;
conditionalTarget =
target_obj->Get(context, env->node_string()).ToLocalChecked();
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
conditionalTarget, subpath, pkg_subpath, base, false);
if (!resolved.IsNothing()) {
ProcessEmitExperimentalWarning(env, "Conditional exports");
return resolved;
bool matched = false;
for (uint32_t i = 0; i < target_obj_keys->Length(); ++i) {
Local<Value> key =
target_obj_keys->Get(context, i).ToLocalChecked();
if (IsArrayIndex(env, key)) {
const std::string msg = "Invalid package config for " +
pjson_url.ToFilePath() + ", \"exports\" cannot contain numeric " +
"property keys.";
node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str());
return Nothing<URL>();
}
}
if (env->options()->experimental_conditional_exports &&
target_obj->HasOwnProperty(context, env->import_string()).FromJust()) {
matched = true;
conditionalTarget =
target_obj->Get(context, env->import_string()).ToLocalChecked();
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
for (uint32_t i = 0; i < target_obj_keys->Length(); ++i) {
Local<Value> key = target_obj_keys->Get(context, i).ToLocalChecked();
Utf8Value key_utf8(env->isolate(),
key->ToString(context).ToLocalChecked());
std::string key_str(*key_utf8, key_utf8.length());
if (key_str == "node" || key_str == "import") {
if (!env->options()->experimental_conditional_exports) continue;
matched = true;
conditionalTarget = target_obj->Get(context, key).ToLocalChecked();
Maybe<URL> 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<URL> resolved = ResolveExportsTarget(env, pjson_url,
if (!resolved.IsNothing()) {
ProcessEmitExperimentalWarning(env, "Conditional exports");
return resolved;
}
} else if (key_str == "default") {
matched = true;
conditionalTarget = target_obj->Get(context, key).ToLocalChecked();
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
conditionalTarget, subpath, pkg_subpath, base, false);
if (!resolved.IsNothing()) {
return resolved;
if (!resolved.IsNothing()) {
ProcessEmitExperimentalWarning(env, "Conditional exports");
return resolved;
}
}
}
if (matched && throw_invalid) {
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
conditionalTarget, subpath, pkg_subpath, base, true);
conditionalTarget, subpath, pkg_subpath, base, true);
CHECK(resolved.IsNothing());
return Nothing<URL>();
}
Expand All @@ -1013,8 +1037,8 @@ Maybe<bool> IsConditionalExportsMainSugar(Environment* env,
exports_obj->GetOwnPropertyNames(context).ToLocalChecked();
bool isConditionalSugar = false;
for (uint32_t i = 0; i < keys->Length(); ++i) {
Local<String> key = keys->Get(context, i).ToLocalChecked().As<String>();
Utf8Value key_utf8(env->isolate(), key);
Local<Value> key = keys->Get(context, i).ToLocalChecked();
Utf8Value key_utf8(env->isolate(), key->ToString(context).ToLocalChecked());
bool curIsConditionalSugar = key_utf8.length() == 0 || key_utf8[0] != '.';
if (i == 0) {
isConditionalSugar = curIsConditionalSugar;
Expand Down Expand Up @@ -1122,13 +1146,13 @@ Maybe<URL> PackageExportsResolve(Environment* env,
Local<Array> keys =
exports_obj->GetOwnPropertyNames(context).ToLocalChecked();
for (uint32_t i = 0; i < keys->Length(); ++i) {
Local<String> key = keys->Get(context, i).ToLocalChecked().As<String>();
Utf8Value key_utf8(isolate, key);
Local<Value> key = keys->Get(context, i).ToLocalChecked();
Utf8Value key_utf8(isolate, key->ToString(context).ToLocalChecked());
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 = key->ToString(context).ToLocalChecked();
best_match_str = key_str;
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/es-module/test-esm-exports.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
'ERR_MODULE_NOT_FOUND');
}));

// Package export with numeric index properties must throw a validation error
loadFixture('pkgexports-numeric').catch(mustCall((err) => {
strictEqual(err.code, 'ERR_INVALID_PACKAGE_CONFIG');
}));

// Sugar conditional exports main mixed failure case
loadFixture('pkgexports-sugar-fail').catch(mustCall((err) => {
strictEqual(err.code, 'ERR_INVALID_PACKAGE_CONFIG');
Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/node_modules/pkgexports-numeric/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 14 additions & 1 deletion test/fixtures/node_modules/pkgexports/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f62fb76

Please sign in to comment.