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 fae517a

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 cad0402 commit fae517a
Copy full SHA for fae517a

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
@@ -349,33 +349,36 @@ Node.js and the browser can be written:
349349
When resolving the `"."` export, if no matching target is found, the `"main"`
350350
will be used as the final fallback.
351351

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

354-
1. `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
354+
* `"default"` - the generic fallback that will always match. Can be a CommonJS
355+
or ES module file.
356+
* `"import"` - matched when the package is loaded via `import` or
357+
`import()`. Can be any module format, this field does not set the type
358+
interpretation. _This is currently only supported behind the
359+
`--experimental-conditional-exports` flag._
360+
* `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
355361
module file. _This is currently only supported behind the
356362
`--experimental-conditional-exports` flag._
357-
2. `"require"` - matched when the package is loaded via `require()`.
363+
* `"require"` - matched when the package is loaded via `require()`.
358364
_This is currently only supported behind the
359365
`--experimental-conditional-exports` flag._
360-
3. `"import"` - matched when the package is loaded via `import` or
361-
`import()`. Can be any module format, this field does not set the type
362-
interpretation. _This is currently only supported behind the
363-
`--experimental-conditional-exports` flag._
364-
4. `"default"` - the generic fallback that will always match if no other
365-
more specific condition is matched first. Can be a CommonJS or ES module
366-
file.
367366

368-
> Setting any of the above flagged conditions for a published package is not
369-
> recommended until they are unflagged to avoid breaking changes to packages in
370-
> future.
367+
Condition matching is applied in object order from first to last within the
368+
`"exports"` object.
369+
370+
> Setting the above conditions for a published package is not recommended until
371+
> conditional exports have been unflagged to avoid breaking changes to packages.
371372
372373
Using the `"require"` condition it is possible to define a package that will
373374
have a different exported value for CommonJS and ES modules, which can be a
374375
hazard in that it can result in having two separate instances of the same
375376
package in use in an application, which can cause a number of bugs.
376377

377378
Other conditions such as `"browser"`, `"electron"`, `"deno"`, `"react-native"`,
378-
etc. could be defined in other runtimes or tools.
379+
etc. could be defined in other runtimes or tools. Condition names must not start
380+
with `"."` or be numbers. Further restrictions, definitions or guidance on
381+
condition names may be provided in future.
379382

380383
#### Exports Sugar
381384

@@ -1557,13 +1560,15 @@ _defaultEnv_ is the conditional environment name priority array,
15571560
> 1. If _resolved_ is contained in _resolvedTarget_, then
15581561
> 1. Return _resolved_.
15591562
> 1. Otherwise, if _target_ is a non-null Object, then
1560-
> 1. If _target_ has an object key matching one of the names in _env_, then
1561-
> 1. Let _targetValue_ be the corresponding value of the first object key
1562-
> of _target_ in _env_.
1563-
> 1. Let _resolved_ be the result of **PACKAGE_EXPORTS_TARGET_RESOLVE**
1564-
> (_packageURL_, _targetValue_, _subpath_, _env_).
1565-
> 1. Assert: _resolved_ is a String.
1566-
> 1. Return _resolved_.
1563+
> 1. If _exports_ contains any index property keys, as defined in ECMA-262
1564+
> [6.1.7 Array Index][], throw an _Invalid Package Configuration_ error.
1565+
> 1. For each property _p_ of _target_, in object insertion order as,
1566+
> 1. If _env_ contains an entry for _p_, then
1567+
> 1. Let _targetValue_ be the value of the _p_ property in _target_.
1568+
> 1. Let _resolved_ be the result of **PACKAGE_EXPORTS_TARGET_RESOLVE**
1569+
> (_packageURL_, _targetValue_, _subpath_, _env_).
1570+
> 1. Assert: _resolved_ is a String.
1571+
> 1. Return _resolved_.
15671572
> 1. Otherwise, if _target_ is an Array, then
15681573
> 1. For each item _targetValue_ in _target_, do
15691574
> 1. If _targetValue_ is an Array, continue the loop.
@@ -1659,3 +1664,4 @@ success!
16591664
[special scheme]: https://url.spec.whatwg.org/#special-scheme
16601665
[the official standard format]: https://tc39.github.io/ecma262/#sec-modules
16611666
[transpiler loader example]: #esm_transpiler_loader
1667+
[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,
@@ -555,6 +558,18 @@ function resolveExports(nmPath, request, absoluteRequest) {
555558
return path.resolve(nmPath, request);
556559
}
557560

561+
function isArrayIndex(p) {
562+
assert(typeof p === 'string');
563+
const n = Number(p);
564+
if (String(n) !== p)
565+
return false;
566+
if (ObjectIs(n, +0))
567+
return true;
568+
if (!Number.isInteger(n))
569+
return false;
570+
return n >= 0 && n < (2 ** 32) - 1;
571+
}
572+
558573
function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
559574
if (typeof target === 'string') {
560575
if (target.startsWith('./') &&
@@ -585,34 +600,33 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
585600
}
586601
}
587602
} else if (typeof target === 'object' && target !== null) {
588-
if (experimentalConditionalExports &&
589-
ObjectPrototypeHasOwnProperty(target, 'node')) {
590-
try {
591-
const result = resolveExportsTarget(pkgPath, target.node, subpath,
592-
basePath, mappingKey);
593-
emitExperimentalWarning('Conditional exports');
594-
return result;
595-
} catch (e) {
596-
if (e.code !== 'MODULE_NOT_FOUND') throw e;
597-
}
598-
}
599-
if (experimentalConditionalExports &&
600-
ObjectPrototypeHasOwnProperty(target, 'require')) {
601-
try {
602-
const result = resolveExportsTarget(pkgPath, target.require, subpath,
603-
basePath, mappingKey);
604-
emitExperimentalWarning('Conditional exports');
605-
return result;
606-
} catch (e) {
607-
if (e.code !== 'MODULE_NOT_FOUND') throw e;
608-
}
603+
const keys = ObjectKeys(target);
604+
if (keys.some(isArrayIndex)) {
605+
throw new ERR_INVALID_PACKAGE_CONFIG(basePath, '"exports" cannot ' +
606+
'contain numeric property keys.');
609607
}
610-
if (ObjectPrototypeHasOwnProperty(target, 'default')) {
611-
try {
612-
return resolveExportsTarget(pkgPath, target.default, subpath,
613-
basePath, mappingKey);
614-
} catch (e) {
615-
if (e.code !== 'MODULE_NOT_FOUND') throw e;
608+
for (const p of keys) {
609+
switch (p) {
610+
case 'node':
611+
case 'require':
612+
if (!experimentalConditionalExports)
613+
continue;
614+
try {
615+
emitExperimentalWarning('Conditional exports');
616+
const result = resolveExportsTarget(pkgPath, target[p], subpath,
617+
basePath, mappingKey);
618+
return result;
619+
} catch (e) {
620+
if (e.code !== 'MODULE_NOT_FOUND') throw e;
621+
}
622+
break;
623+
case 'default':
624+
try {
625+
return resolveExportsTarget(pkgPath, target.default, subpath,
626+
basePath, mappingKey);
627+
} catch (e) {
628+
if (e.code !== 'MODULE_NOT_FOUND') throw e;
629+
}
616630
}
617631
}
618632
}
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
@@ -203,7 +203,6 @@ constexpr size_t kFsStatsBufferLength =
203203
V(crypto_rsa_pss_string, "rsa-pss") \
204204
V(cwd_string, "cwd") \
205205
V(data_string, "data") \
206-
V(default_string, "default") \
207206
V(dest_string, "dest") \
208207
V(destroyed_string, "destroyed") \
209208
V(detached_string, "detached") \
@@ -258,7 +257,6 @@ constexpr size_t kFsStatsBufferLength =
258257
V(http_1_1_string, "http/1.1") \
259258
V(identity_string, "identity") \
260259
V(ignore_string, "ignore") \
261-
V(import_string, "import") \
262260
V(infoaccess_string, "infoAccess") \
263261
V(inherit_string, "inherit") \
264262
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
@@ -125,6 +125,11 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
125125
'ERR_MODULE_NOT_FOUND');
126126
}));
127127

128+
// Package export with numeric index properties must throw a validation error
129+
loadFixture('pkgexports-numeric').catch(mustCall((err) => {
130+
strictEqual(err.code, 'ERR_INVALID_PACKAGE_CONFIG');
131+
}));
132+
128133
// Sugar conditional exports main mixed failure case
129134
loadFixture('pkgexports-sugar-fail').catch(mustCall((err) => {
130135
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.