From 11928c7f57d4b1229ebf759940a7332be872b6de Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 5 Aug 2020 12:06:28 -0700 Subject: [PATCH 01/10] module: custom --conditions flag option --- doc/api/cli.md | 15 ++++++++++ doc/node.1 | 4 +++ lib/internal/modules/cjs/loader.js | 28 +++++++------------ lib/internal/modules/esm/resolve.js | 3 +- src/node_options.cc | 5 ++++ src/node_options.h | 1 + test/es-module/test-esm-custom-exports.mjs | 10 +++++++ .../pkgexports/custom-condition.js | 1 + .../node_modules/pkgexports/package.json | 5 +++- 9 files changed, 52 insertions(+), 20 deletions(-) create mode 100644 test/es-module/test-esm-custom-exports.mjs create mode 100644 test/fixtures/node_modules/pkgexports/custom-condition.js diff --git a/doc/api/cli.md b/doc/api/cli.md index 899c28f6f76675..493e40a79c5db5 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -76,6 +76,21 @@ $ node --completion-bash > node_bash_completion $ source node_bash_completion ``` +### `-cc`, `--conditions=conditionA,conditionB` + + +> Stability: 1 - Experimental + +Enable experimental support for custom conditional exports resolution +conditions. + +Any custom comma-separated string condition names are permitted. + +The default Node.js conditions of `"node"`, `"default"`, `"import"` and +`"require"` will always apply as defined. + ### `--cpu-prof` @@ -1247,6 +1247,7 @@ node --require "./a.js" --require "./b.js" Node.js options that are allowed are: +* `--conditions` * `--diagnostic-dir` * `--disable-proto` * `--enable-fips` diff --git a/doc/node.1 b/doc/node.1 index e35dd697638bd6..b6b062cd346b5b 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -78,7 +78,7 @@ Aborting instead of exiting causes a core file to be generated for analysis. .It Fl -completion-bash Print source-able bash completion script for Node.js. . -.It Fl cc , Fl -conditions Ar string +.It Fl -conditions Ar string Use custom conditional exports conditions .Ar string . diff --git a/src/node_options.cc b/src/node_options.cc index 541eb2b1906d57..d61ea6bec25085 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -285,7 +285,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "set the conditional exports conditions", &EnvironmentOptions::conditions, kAllowedInEnvironment); - AddAlias("-cc", "--conditions"); AddOption("--diagnostic-dir", "set dir for all output files" " (default: current working directory)", From e1a91c85d0f040b49ded26a0b46f5a788eca1e95 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 5 Aug 2020 12:52:08 -0700 Subject: [PATCH 04/10] todo is done --- lib/internal/modules/cjs/loader.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 8aa20119c91b62..1b7abb26dbd0f1 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1009,7 +1009,6 @@ Module._load = function(request, parent, isMain) { return module.exports; }; -// TODO: Use this set when resolving pkg#exports conditions. const cjsConditions = new SafeSet(['require', 'node', ...userConditions]); Module._resolveFilename = function(request, parent, isMain, options) { if (NativeModule.canBeRequiredByUsers(request)) { From ed6923507f9664044eac688a490843f247a84109 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 5 Aug 2020 12:58:53 -0700 Subject: [PATCH 05/10] -m alias, docs --- doc/api/cli.md | 4 ++-- doc/api/esm.md | 11 +++++++++++ doc/node.1 | 2 +- src/node_options.cc | 1 + 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 805198c9516cd7..d19e0628f1889b 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -76,7 +76,7 @@ $ node --completion-bash > node_bash_completion $ source node_bash_completion ``` -### `--conditions=conditionA,conditionB` +### `-m`, `--conditions=conditionA,conditionB` @@ -1247,7 +1247,7 @@ node --require "./a.js" --require "./b.js" Node.js options that are allowed are: -* `--conditions` +* `--conditions`, `-m` * `--diagnostic-dir` * `--disable-proto` * `--enable-fips` diff --git a/doc/api/esm.md b/doc/api/esm.md index 551eef604eac0e..0a75edf2ad56bc 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -475,6 +475,17 @@ order to support packages with conditional exports. For this reason, using `"node"` and `"default"` condition branches is usually preferable to using `"node"` and `"browser"` condition branches. +When running Node.js, custom comma-separated conditions can be set with the +`--conditions` or `-m` flag: + +```bash +node --conditions=development main.js +``` + +which would then resolve the `"development"` condition in exports, along with +the existing `"node"`, `"default"`, `"import"` and `"require"` conditions as +appropriate. + #### Nested conditions In addition to direct mappings, Node.js also supports nested condition objects. diff --git a/doc/node.1 b/doc/node.1 index b6b062cd346b5b..caf1705fa9f704 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -78,7 +78,7 @@ Aborting instead of exiting causes a core file to be generated for analysis. .It Fl -completion-bash Print source-able bash completion script for Node.js. . -.It Fl -conditions Ar string +.It Fl m , Fl -conditions Ar string Use custom conditional exports conditions .Ar string . diff --git a/src/node_options.cc b/src/node_options.cc index d61ea6bec25085..ea2943812680a0 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -285,6 +285,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "set the conditional exports conditions", &EnvironmentOptions::conditions, kAllowedInEnvironment); + AddAlias("-m", "--conditions"); AddOption("--diagnostic-dir", "set dir for all output files" " (default: current working directory)", From 0180f8648527f64a4a00f4bc1daffeb1660ba52d Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 5 Aug 2020 13:04:00 -0700 Subject: [PATCH 06/10] doc reorder --- doc/api/esm.md | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 0a75edf2ad56bc..2ada58dfadc59c 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -475,17 +475,6 @@ order to support packages with conditional exports. For this reason, using `"node"` and `"default"` condition branches is usually preferable to using `"node"` and `"browser"` condition branches. -When running Node.js, custom comma-separated conditions can be set with the -`--conditions` or `-m` flag: - -```bash -node --conditions=development main.js -``` - -which would then resolve the `"development"` condition in exports, along with -the existing `"node"`, `"default"`, `"import"` and `"require"` conditions as -appropriate. - #### Nested conditions In addition to direct mappings, Node.js also supports nested condition objects. @@ -512,6 +501,19 @@ a nested conditional does not have any mapping it will continue checking the remaining conditions of the parent condition. In this way nested conditions behave analogously to nested JavaScript `if` statements. +#### Executing custom conditions + +When running Node.js, custom comma-separated conditions can be set with the +`--conditions` or `-m` flag: + +```bash +node --conditions=development main.js +``` + +which would then resolve the `"development"` condition in exports, along with +the existing `"node"`, `"default"`, `"import"` and `"require"` conditions as +appropriate. + #### Self-referencing a package using its name Within a package, the values defined in the package’s From dba70c16578c04fe530c6a2db68bd160b62f4f31 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 5 Aug 2020 13:04:48 -0700 Subject: [PATCH 07/10] rename heading --- 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 2ada58dfadc59c..97c8c23bbe1def 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -501,7 +501,7 @@ a nested conditional does not have any mapping it will continue checking the remaining conditions of the parent condition. In this way nested conditions behave analogously to nested JavaScript `if` statements. -#### Executing custom conditions +#### Resolving custom conditions When running Node.js, custom comma-separated conditions can be set with the `--conditions` or `-m` flag: From 52f216cd0506de9cfca2b40dfd137a94e3e17c92 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 5 Aug 2020 13:43:40 -0700 Subject: [PATCH 08/10] use -u short flag --- doc/api/cli.md | 4 ++-- doc/node.1 | 2 +- src/node_options.cc | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index d19e0628f1889b..3b56f2390d4684 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -76,7 +76,7 @@ $ node --completion-bash > node_bash_completion $ source node_bash_completion ``` -### `-m`, `--conditions=conditionA,conditionB` +### `-u`, `--conditions=conditionA,conditionB` @@ -1247,7 +1247,7 @@ node --require "./a.js" --require "./b.js" Node.js options that are allowed are: -* `--conditions`, `-m` +* `--conditions`, `-u` * `--diagnostic-dir` * `--disable-proto` * `--enable-fips` diff --git a/doc/node.1 b/doc/node.1 index caf1705fa9f704..7eb877bb1c8b4d 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -78,7 +78,7 @@ Aborting instead of exiting causes a core file to be generated for analysis. .It Fl -completion-bash Print source-able bash completion script for Node.js. . -.It Fl m , Fl -conditions Ar string +.It Fl u , Fl -conditions Ar string Use custom conditional exports conditions .Ar string . diff --git a/src/node_options.cc b/src/node_options.cc index ea2943812680a0..f5dcc18b105a10 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -285,7 +285,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "set the conditional exports conditions", &EnvironmentOptions::conditions, kAllowedInEnvironment); - AddAlias("-m", "--conditions"); + AddAlias("-u", "--conditions"); AddOption("--diagnostic-dir", "set dir for all output files" " (default: current working directory)", From acbe3ee460e24e46d7d64dd729d7fa99783e8844 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 5 Aug 2020 14:10:23 -0700 Subject: [PATCH 09/10] Apply suggestions from code review Co-authored-by: Jordan Harband Co-authored-by: Jan Olaf Krems --- doc/api/cli.md | 2 +- doc/api/esm.md | 4 ++-- lib/internal/modules/cjs/loader.js | 2 +- lib/internal/modules/esm/resolve.js | 2 +- src/node_options.cc | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 3b56f2390d4684..077239efe03d14 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -88,7 +88,7 @@ conditions. Any custom comma-separated string condition names are permitted. -The default Node.js conditions of `"node"`, `"default"`, `"import"` and +The default Node.js conditions of `"node"`, `"default"`, `"import"`, and `"require"` will always apply as defined. ### `--cpu-prof` diff --git a/doc/api/esm.md b/doc/api/esm.md index 97c8c23bbe1def..bf6ceb20908611 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -504,14 +504,14 @@ conditions behave analogously to nested JavaScript `if` statements. #### Resolving custom conditions When running Node.js, custom comma-separated conditions can be set with the -`--conditions` or `-m` flag: +`--conditions` or `-u` flag: ```bash node --conditions=development main.js ``` which would then resolve the `"development"` condition in exports, along with -the existing `"node"`, `"default"`, `"import"` and `"require"` conditions as +the existing `"node"`, `"default"`, `"import"`, and `"require"` conditions as appropriate. #### Self-referencing a package using its name diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 1b7abb26dbd0f1..b058e6ccd4e8ff 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -86,7 +86,7 @@ const manifest = getOptionValue('--experimental-policy') ? null; const { compileFunction } = internalBinding('contextify'); const userConditions = getOptionValue('--conditions') && - getOptionValue('--conditions').split(','); + StringPrototypeSplit(getOptionValue('--conditions'), ','); // Whether any user-provided CJS modules had been loaded (executed). // Used for internal assertions. diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index fec0ee181c7e28..5fb5dd72f6f04c 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -52,7 +52,7 @@ const { Module: CJSModule } = require('internal/modules/cjs/loader'); const packageJsonReader = require('internal/modules/package_json_reader'); const userConditions = getOptionValue('--conditions') && - getOptionValue('--conditions').split(','); + StringPrototypeSplit(getOptionValue('--conditions'), ','); const DEFAULT_CONDITIONS = ObjectFreeze(['node', 'import', ...userConditions]); const DEFAULT_CONDITIONS_SET = new SafeSet(DEFAULT_CONDITIONS); diff --git a/src/node_options.cc b/src/node_options.cc index f5dcc18b105a10..d3fdc459de1ee2 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -282,7 +282,7 @@ DebugOptionsParser::DebugOptionsParser() { EnvironmentOptionsParser::EnvironmentOptionsParser() { AddOption("--conditions", - "set the conditional exports conditions", + "additional user conditions for conditional exports and imports", &EnvironmentOptions::conditions, kAllowedInEnvironment); AddAlias("-u", "--conditions"); From 1ed44a4f144cfc0209cf3a3b1866f20b9ee38e4e Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 5 Aug 2020 14:31:31 -0700 Subject: [PATCH 10/10] use multiple flags over comma separated --- doc/api/cli.md | 4 ++-- doc/api/esm.md | 12 +++++++----- lib/internal/modules/cjs/loader.js | 3 +-- lib/internal/modules/esm/resolve.js | 3 +-- src/node_options.h | 2 +- test/es-module/test-esm-custom-exports.mjs | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 077239efe03d14..413784538d289d 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -76,7 +76,7 @@ $ node --completion-bash > node_bash_completion $ source node_bash_completion ``` -### `-u`, `--conditions=conditionA,conditionB` +### `-u`, `--conditions=condition` @@ -86,7 +86,7 @@ added: REPLACEME Enable experimental support for custom conditional exports resolution conditions. -Any custom comma-separated string condition names are permitted. +Any number of custom string condition names are permitted. The default Node.js conditions of `"node"`, `"default"`, `"import"`, and `"require"` will always apply as defined. diff --git a/doc/api/esm.md b/doc/api/esm.md index bf6ceb20908611..adbb55f0d82f0d 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -501,18 +501,20 @@ a nested conditional does not have any mapping it will continue checking the remaining conditions of the parent condition. In this way nested conditions behave analogously to nested JavaScript `if` statements. -#### Resolving custom conditions +#### Resolving user conditions -When running Node.js, custom comma-separated conditions can be set with the +When running Node.js, custom user conditions can be added with the `--conditions` or `-u` flag: ```bash node --conditions=development main.js ``` -which would then resolve the `"development"` condition in exports, along with -the existing `"node"`, `"default"`, `"import"`, and `"require"` conditions as -appropriate. +which would then resolve the `"development"` condition in package imports and +exports, while resolving the existing `"node"`, `"default"`, `"import"`, and +`"require"` conditions as appropriate. + +Any number of custom conditions can be set with repeat flags. #### Self-referencing a package using its name diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index b058e6ccd4e8ff..44b373f3e76063 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -85,8 +85,7 @@ const manifest = getOptionValue('--experimental-policy') ? require('internal/process/policy').manifest : null; const { compileFunction } = internalBinding('contextify'); -const userConditions = getOptionValue('--conditions') && - StringPrototypeSplit(getOptionValue('--conditions'), ','); +const userConditions = getOptionValue('--conditions'); // Whether any user-provided CJS modules had been loaded (executed). // Used for internal assertions. diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 5fb5dd72f6f04c..7cf3552948194d 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -51,8 +51,7 @@ const { const { Module: CJSModule } = require('internal/modules/cjs/loader'); const packageJsonReader = require('internal/modules/package_json_reader'); -const userConditions = getOptionValue('--conditions') && - StringPrototypeSplit(getOptionValue('--conditions'), ','); +const userConditions = getOptionValue('--conditions'); const DEFAULT_CONDITIONS = ObjectFreeze(['node', 'import', ...userConditions]); const DEFAULT_CONDITIONS_SET = new SafeSet(DEFAULT_CONDITIONS); diff --git a/src/node_options.h b/src/node_options.h index c668abb0b61303..3258d4b3f0df0c 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -100,7 +100,7 @@ class DebugOptions : public Options { class EnvironmentOptions : public Options { public: bool abort_on_uncaught_exception = false; - std::string conditions; + std::vector conditions; bool enable_source_maps = false; bool experimental_json_modules = false; bool experimental_modules = false; diff --git a/test/es-module/test-esm-custom-exports.mjs b/test/es-module/test-esm-custom-exports.mjs index 227b4652588a5f..ad81abfdafd861 100644 --- a/test/es-module/test-esm-custom-exports.mjs +++ b/test/es-module/test-esm-custom-exports.mjs @@ -1,4 +1,4 @@ -// Flags: --conditions=custom-condition,another +// Flags: --conditions=custom-condition -u another import { mustCall } from '../common/index.mjs'; import { strictEqual } from 'assert'; import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';