FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

module: logical conditional exports ordering · nodejs/node@fae517a · GitHub

/ node Public

Commit fae517a

Browse files
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

7 files changed

Lines changed: 152 additions & 86 deletions

File tree

‎doc/api/esm.md‎

Lines changed: 27 additions & 21 deletions
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

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

Lines 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
}

‎src/env.h‎

Lines 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") \

‎src/module_wrap.cc‎

Lines 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
}

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

Lines 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');

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

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines 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)
Back | FazBrowse Home | New Git URL