Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f62fb76

Browse filesBrowse files
guybedfordMylesBorins
guybedford
authored and
MylesBorins
committed
module: logical conditional exports ordering
PR-URL: #31008 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com>
1 parent 94af94a commit f62fb76
Copy full SHA for f62fb76

File tree

Expand file treeCollapse file tree

7 files changed

+152
-86
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+152
-86
lines changed
Open diff view settings
Collapse file

‎doc/api/esm.md‎

Copy file name to clipboardExpand all lines: doc/api/esm.md
+27-21Lines changed: 27 additions & 21 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -343,33 +343,36 @@ Node.js and the browser can be written:
343343
When resolving the `"."` export, if no matching target is found, the `"main"`
344344
will be used as the final fallback.
345345

346-
The conditions supported in Node.js are matched in the following order:
346+
The conditions supported in Node.js condition matching:
347347

348-
1. `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
348+
* `"default"` - the generic fallback that will always match. Can be a CommonJS
349+
or ES module file.
350+
* `"import"` - matched when the package is loaded via `import` or
351+
`import()`. Can be any module format, this field does not set the type
352+
interpretation. _This is currently only supported behind the
353+
`--experimental-conditional-exports` flag._
354+
* `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
349355
module file. _This is currently only supported behind the
350356
`--experimental-conditional-exports` flag._
351-
2. `"require"` - matched when the package is loaded via `require()`.
357+
* `"require"` - matched when the package is loaded via `require()`.
352358
_This is currently only supported behind the
353359
`--experimental-conditional-exports` flag._
354-
3. `"import"` - matched when the package is loaded via `import` or
355-
`import()`. Can be any module format, this field does not set the type
356-
interpretation. _This is currently only supported behind the
357-
`--experimental-conditional-exports` flag._
358-
4. `"default"` - the generic fallback that will always match if no other
359-
more specific condition is matched first. Can be a CommonJS or ES module
360-
file.
361360

362-
> Setting any of the above flagged conditions for a published package is not
363-
> recommended until they are unflagged to avoid breaking changes to packages in
364-
> future.
361+
Condition matching is applied in object order from first to last within the
362+
`"exports"` object.
363+
364+
> Setting the above conditions for a published package is not recommended until
365+
> conditional exports have been unflagged to avoid breaking changes to packages.
365366
366367
Using the `"require"` condition it is possible to define a package that will
367368
have a different exported value for CommonJS and ES modules, which can be a
368369
hazard in that it can result in having two separate instances of the same
369370
package in use in an application, which can cause a number of bugs.
370371

371372
Other conditions such as `"browser"`, `"electron"`, `"deno"`, `"react-native"`,
372-
etc. could be defined in other runtimes or tools.
373+
etc. could be defined in other runtimes or tools. Condition names must not start
374+
with `"."` or be numbers. Further restrictions, definitions or guidance on
375+
condition names may be provided in future.
373376

374377
#### Exports Sugar
375378

@@ -1547,13 +1550,15 @@ _defaultEnv_ is the conditional environment name priority array,
15471550
> 1. If _resolved_ is contained in _resolvedTarget_, then
15481551
> 1. Return _resolved_.
15491552
> 1. Otherwise, if _target_ is a non-null Object, then
1550-
> 1. If _target_ has an object key matching one of the names in _env_, then
1551-
> 1. Let _targetValue_ be the corresponding value of the first object key
1552-
> of _target_ in _env_.
1553-
> 1. Let _resolved_ be the result of **PACKAGE_EXPORTS_TARGET_RESOLVE**
1554-
> (_packageURL_, _targetValue_, _subpath_, _env_).
1555-
> 1. Assert: _resolved_ is a String.
1556-
> 1. Return _resolved_.
1553+
> 1. If _exports_ contains any index property keys, as defined in ECMA-262
1554+
> [6.1.7 Array Index][], throw an _Invalid Package Configuration_ error.
1555+
> 1. For each property _p_ of _target_, in object insertion order as,
1556+
> 1. If _env_ contains an entry for _p_, then
1557+
> 1. Let _targetValue_ be the value of the _p_ property in _target_.
1558+
> 1. Let _resolved_ be the result of **PACKAGE_EXPORTS_TARGET_RESOLVE**
1559+
> (_packageURL_, _targetValue_, _subpath_, _env_).
1560+
> 1. Assert: _resolved_ is a String.
1561+
> 1. Return _resolved_.
15571562
> 1. Otherwise, if _target_ is an Array, then
15581563
> 1. For each item _targetValue_ in _target_, do
15591564
> 1. If _targetValue_ is an Array, continue the loop.
@@ -1649,3 +1654,4 @@ success!
16491654
[special scheme]: https://url.spec.whatwg.org/#special-scheme
16501655
[the official standard format]: https://tc39.github.io/ecma262/#sec-modules
16511656
[transpiler loader example]: #esm_transpiler_loader
1657+
[6.1.7 Array Index]: https://tc39.es/ecma262/#integer-index
Collapse file

‎lib/internal/modules/cjs/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js
+41-27Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@ const {
2626
Error,
2727
JSONParse,
2828
Map,
29+
Number,
2930
ObjectCreate,
3031
ObjectDefineProperty,
3132
ObjectFreeze,
33+
ObjectIs,
3234
ObjectKeys,
3335
ObjectPrototypeHasOwnProperty,
3436
ReflectSet,
3537
SafeMap,
38+
String,
3639
StringPrototypeIndexOf,
3740
StringPrototypeMatch,
3841
StringPrototypeSlice,
@@ -554,6 +557,18 @@ function resolveExports(nmPath, request, absoluteRequest) {
554557
return path.resolve(nmPath, request);
555558
}
556559

560+
function isArrayIndex(p) {
561+
assert(typeof p === 'string');
562+
const n = Number(p);
563+
if (String(n) !== p)
564+
return false;
565+
if (ObjectIs(n, +0))
566+
return true;
567+
if (!Number.isInteger(n))
568+
return false;
569+
return n >= 0 && n < (2 ** 32) - 1;
570+
}
571+
557572
function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
558573
if (typeof target === 'string') {
559574
if (target.startsWith('./') &&
@@ -584,34 +599,33 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
584599
}
585600
}
586601
} else if (typeof target === 'object' && target !== null) {
587-
if (experimentalConditionalExports &&
588-
ObjectPrototypeHasOwnProperty(target, 'node')) {
589-
try {
590-
const result = resolveExportsTarget(pkgPath, target.node, subpath,
591-
basePath, mappingKey);
592-
emitExperimentalWarning('Conditional exports');
593-
return result;
594-
} catch (e) {
595-
if (e.code !== 'MODULE_NOT_FOUND') throw e;
596-
}
597-
}
598-
if (experimentalConditionalExports &&
599-
ObjectPrototypeHasOwnProperty(target, 'require')) {
600-
try {
601-
const result = resolveExportsTarget(pkgPath, target.require, subpath,
602-
basePath, mappingKey);
603-
emitExperimentalWarning('Conditional exports');
604-
return result;
605-
} catch (e) {
606-
if (e.code !== 'MODULE_NOT_FOUND') throw e;
607-
}
602+
const keys = ObjectKeys(target);
603+
if (keys.some(isArrayIndex)) {
604+
throw new ERR_INVALID_PACKAGE_CONFIG(basePath, '"exports" cannot ' +
605+
'contain numeric property keys.');
608606
}
609-
if (ObjectPrototypeHasOwnProperty(target, 'default')) {
610-
try {
611-
return resolveExportsTarget(pkgPath, target.default, subpath,
612-
basePath, mappingKey);
613-
} catch (e) {
614-
if (e.code !== 'MODULE_NOT_FOUND') throw e;
607+
for (const p of keys) {
608+
switch (p) {
609+
case 'node':
610+
case 'require':
611+
if (!experimentalConditionalExports)
612+
continue;
613+
try {
614+
emitExperimentalWarning('Conditional exports');
615+
const result = resolveExportsTarget(pkgPath, target[p], subpath,
616+
basePath, mappingKey);
617+
return result;
618+
} catch (e) {
619+
if (e.code !== 'MODULE_NOT_FOUND') throw e;
620+
}
621+
break;
622+
case 'default':
623+
try {
624+
return resolveExportsTarget(pkgPath, target.default, subpath,
625+
basePath, mappingKey);
626+
} catch (e) {
627+
if (e.code !== 'MODULE_NOT_FOUND') throw e;
628+
}
615629
}
616630
}
617631
}
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
-2Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,6 @@ constexpr size_t kFsStatsBufferLength =
202202
V(crypto_rsa_pss_string, "rsa-pss") \
203203
V(cwd_string, "cwd") \
204204
V(data_string, "data") \
205-
V(default_string, "default") \
206205
V(dest_string, "dest") \
207206
V(destroyed_string, "destroyed") \
208207
V(detached_string, "detached") \
@@ -257,7 +256,6 @@ constexpr size_t kFsStatsBufferLength =
257256
V(http_1_1_string, "http/1.1") \
258257
V(identity_string, "identity") \
259258
V(ignore_string, "ignore") \
260-
V(import_string, "import") \
261259
V(infoaccess_string, "infoAccess") \
262260
V(inherit_string, "inherit") \
263261
V(input_string, "input") \
Collapse file

‎src/module_wrap.cc‎

Copy file name to clipboardExpand all lines: src/module_wrap.cc
+59-35Lines changed: 59 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include <sys/stat.h> // S_IFDIR
1313

1414
#include <algorithm>
15-
#include <climits> // PATH_MAX
1615

1716
namespace node {
1817
namespace loader {
@@ -908,6 +907,25 @@ Maybe<URL> ResolveExportsTargetString(Environment* env,
908907
return Just(subpath_resolved);
909908
}
910909

910+
bool IsArrayIndex(Environment* env, Local<Value> p) {
911+
Local<Context> context = env->context();
912+
Local<String> p_str = p->ToString(context).ToLocalChecked();
913+
double n_dbl = static_cast<double>(p_str->NumberValue(context).FromJust());
914+
Local<Number> n = Number::New(env->isolate(), n_dbl);
915+
Local<String> cmp_str = n->ToString(context).ToLocalChecked();
916+
if (!p_str->Equals(context, cmp_str).FromJust()) {
917+
return false;
918+
}
919+
if (n_dbl == 0 && std::signbit(n_dbl) == false) {
920+
return true;
921+
}
922+
Local<Integer> cmp_integer;
923+
if (!n->ToInteger(context).ToLocal(&cmp_integer)) {
924+
return false;
925+
}
926+
return n_dbl > 0 && n_dbl < (2 ^ 32) - 1;
927+
}
928+
911929
Maybe<URL> ResolveExportsTarget(Environment* env,
912930
const URL& pjson_url,
913931
Local<Value> target,
@@ -953,44 +971,50 @@ Maybe<URL> ResolveExportsTarget(Environment* env,
953971
return Nothing<URL>();
954972
} else if (target->IsObject()) {
955973
Local<Object> target_obj = target.As<Object>();
956-
bool matched = false;
974+
Local<Array> target_obj_keys =
975+
target_obj->GetOwnPropertyNames(context).ToLocalChecked();
957976
Local<Value> conditionalTarget;
958-
if (env->options()->experimental_conditional_exports &&
959-
target_obj->HasOwnProperty(context, env->node_string()).FromJust()) {
960-
matched = true;
961-
conditionalTarget =
962-
target_obj->Get(context, env->node_string()).ToLocalChecked();
963-
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
964-
conditionalTarget, subpath, pkg_subpath, base, false);
965-
if (!resolved.IsNothing()) {
966-
ProcessEmitExperimentalWarning(env, "Conditional exports");
967-
return resolved;
977+
bool matched = false;
978+
for (uint32_t i = 0; i < target_obj_keys->Length(); ++i) {
979+
Local<Value> key =
980+
target_obj_keys->Get(context, i).ToLocalChecked();
981+
if (IsArrayIndex(env, key)) {
982+
const std::string msg = "Invalid package config for " +
983+
pjson_url.ToFilePath() + ", \"exports\" cannot contain numeric " +
984+
"property keys.";
985+
node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str());
986+
return Nothing<URL>();
968987
}
969988
}
970-
if (env->options()->experimental_conditional_exports &&
971-
target_obj->HasOwnProperty(context, env->import_string()).FromJust()) {
972-
matched = true;
973-
conditionalTarget =
974-
target_obj->Get(context, env->import_string()).ToLocalChecked();
975-
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
989+
for (uint32_t i = 0; i < target_obj_keys->Length(); ++i) {
990+
Local<Value> key = target_obj_keys->Get(context, i).ToLocalChecked();
991+
Utf8Value key_utf8(env->isolate(),
992+
key->ToString(context).ToLocalChecked());
993+
std::string key_str(*key_utf8, key_utf8.length());
994+
if (key_str == "node" || key_str == "import") {
995+
if (!env->options()->experimental_conditional_exports) continue;
996+
matched = true;
997+
conditionalTarget = target_obj->Get(context, key).ToLocalChecked();
998+
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
976999
conditionalTarget, subpath, pkg_subpath, base, false);
977-
if (!resolved.IsNothing()) {
978-
return resolved;
979-
}
980-
}
981-
if (target_obj->HasOwnProperty(context, env->default_string()).FromJust()) {
982-
matched = true;
983-
conditionalTarget =
984-
target_obj->Get(context, env->default_string()).ToLocalChecked();
985-
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
1000+
if (!resolved.IsNothing()) {
1001+
ProcessEmitExperimentalWarning(env, "Conditional exports");
1002+
return resolved;
1003+
}
1004+
} else if (key_str == "default") {
1005+
matched = true;
1006+
conditionalTarget = target_obj->Get(context, key).ToLocalChecked();
1007+
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
9861008
conditionalTarget, subpath, pkg_subpath, base, false);
987-
if (!resolved.IsNothing()) {
988-
return resolved;
1009+
if (!resolved.IsNothing()) {
1010+
ProcessEmitExperimentalWarning(env, "Conditional exports");
1011+
return resolved;
1012+
}
9891013
}
9901014
}
9911015
if (matched && throw_invalid) {
9921016
Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
993-
conditionalTarget, subpath, pkg_subpath, base, true);
1017+
conditionalTarget, subpath, pkg_subpath, base, true);
9941018
CHECK(resolved.IsNothing());
9951019
return Nothing<URL>();
9961020
}
@@ -1013,8 +1037,8 @@ Maybe<bool> IsConditionalExportsMainSugar(Environment* env,
10131037
exports_obj->GetOwnPropertyNames(context).ToLocalChecked();
10141038
bool isConditionalSugar = false;
10151039
for (uint32_t i = 0; i < keys->Length(); ++i) {
1016-
Local<String> key = keys->Get(context, i).ToLocalChecked().As<String>();
1017-
Utf8Value key_utf8(env->isolate(), key);
1040+
Local<Value> key = keys->Get(context, i).ToLocalChecked();
1041+
Utf8Value key_utf8(env->isolate(), key->ToString(context).ToLocalChecked());
10181042
bool curIsConditionalSugar = key_utf8.length() == 0 || key_utf8[0] != '.';
10191043
if (i == 0) {
10201044
isConditionalSugar = curIsConditionalSugar;
@@ -1122,13 +1146,13 @@ Maybe<URL> PackageExportsResolve(Environment* env,
11221146
Local<Array> keys =
11231147
exports_obj->GetOwnPropertyNames(context).ToLocalChecked();
11241148
for (uint32_t i = 0; i < keys->Length(); ++i) {
1125-
Local<String> key = keys->Get(context, i).ToLocalChecked().As<String>();
1126-
Utf8Value key_utf8(isolate, key);
1149+
Local<Value> key = keys->Get(context, i).ToLocalChecked();
1150+
Utf8Value key_utf8(isolate, key->ToString(context).ToLocalChecked());
11271151
std::string key_str(*key_utf8, key_utf8.length());
11281152
if (key_str.back() != '/') continue;
11291153
if (pkg_subpath.substr(0, key_str.length()) == key_str &&
11301154
key_str.length() > best_match_str.length()) {
1131-
best_match = key;
1155+
best_match = key->ToString(context).ToLocalChecked();
11321156
best_match_str = key_str;
11331157
}
11341158
}
Collapse file

‎test/es-module/test-esm-exports.mjs‎

Copy file name to clipboardExpand all lines: test/es-module/test-esm-exports.mjs
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,11 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
124124
'ERR_MODULE_NOT_FOUND');
125125
}));
126126

127+
// Package export with numeric index properties must throw a validation error
128+
loadFixture('pkgexports-numeric').catch(mustCall((err) => {
129+
strictEqual(err.code, 'ERR_INVALID_PACKAGE_CONFIG');
130+
}));
131+
127132
// Sugar conditional exports main mixed failure case
128133
loadFixture('pkgexports-sugar-fail').catch(mustCall((err) => {
129134
strictEqual(err.code, 'ERR_INVALID_PACKAGE_CONFIG');
Collapse file

‎test/fixtures/node_modules/pkgexports-numeric/package.json‎

Copy file name to clipboardExpand all lines: test/fixtures/node_modules/pkgexports-numeric/package.json
+6Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/node_modules/pkgexports/package.json‎

Copy file name to clipboardExpand all lines: test/fixtures/node_modules/pkgexports/package.json
+14-1Lines changed: 14 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.