Commit 99bb02f
src: swap dotenv and config file parsing order
Signed-off-by: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: #63035
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>1 parent 7cd2066 commit 99bb02f
4 files changed
Lines changed: 14 additions & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1020,12 +1020,12 @@ node --import amaro/strip --watch-path=src --watch-preserve-output --test-isolat | |||
| 1020 | 1020 | The priority in configuration is as follows: | |
| 1021 | 1021 | ||
| 1022 | 1022 | 1. NODE\_OPTIONS and command-line options | |
| 1023 | - 2. Configuration file | ||
| 1024 | - 3. Dotenv NODE\_OPTIONS | ||
| 1023 | + 2. Dotenv NODE\_OPTIONS | ||
| 1024 | + 3. Configuration file | ||
| 1025 | 1025 | ||
| 1026 | 1026 | Values in the configuration file will not override the values in the environment | |
| 1027 | - variables and command-line options, but will override the values in the `NODE_OPTIONS` | ||
| 1028 | - env file parsed by the `--env-file` flag. | ||
| 1027 | + variables, command-line options, or the `NODE_OPTIONS` env file parsed by the | ||
| 1028 | + `--env-file` flag. | ||
| 1029 | 1029 | ||
| 1030 | 1030 | Keys cannot be duplicated within the same or different namespaces. | |
| 1031 | 1031 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -879,6 +879,7 @@ static ExitCode InitializeNodeWithArgsInternal( | |||
| 879 | 879 | HandleEnvOptions(per_process::cli_options->per_isolate->per_env); | |
| 880 | 880 | ||
| 881 | 881 | std::string node_options; | |
| 882 | + std::string node_options_from_dotenv; | ||
| 882 | 883 | auto env_files = node::Dotenv::GetDataFromArgs(*argv); | |
| 883 | 884 | ||
| 884 | 885 | if (!env_files.empty()) { | |
@@ -905,7 +906,8 @@ static ExitCode InitializeNodeWithArgsInternal( | |||
| 905 | 906 | } | |
| 906 | 907 | } | |
| 907 | 908 | ||
| 908 | - per_process::dotenv_file.AssignNodeOptionsIfAvailable(&node_options); | ||
| 909 | + per_process::dotenv_file.AssignNodeOptionsIfAvailable( | ||
| 910 | + &node_options_from_dotenv); | ||
| 909 | 911 | } | |
| 910 | 912 | ||
| 911 | 913 | std::string node_options_from_config; | |
@@ -935,9 +937,10 @@ static ExitCode InitializeNodeWithArgsInternal( | |||
| 935 | 937 | errors->emplace_back("The number of NODE_OPTIONS doesn't match " | |
| 936 | 938 | "the number of flags in the config file"); | |
| 937 | 939 | } | |
| 938 | - node_options += node_options_from_config; | ||
| 939 | 940 | } | |
| 940 | 941 | ||
| 942 | + node_options = node_options_from_config + node_options_from_dotenv; | ||
| 943 | + | ||
| 941 | 944 | #if !defined(NODE_WITHOUT_NODE_OPTIONS) | |
| 942 | 945 | bool should_parse_node_options = | |
| 943 | 946 | !(flags & ProcessInitializationFlags::kDisableNodeOptionsEnv); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -345,7 +345,7 @@ void Dotenv::AssignNodeOptionsIfAvailable(std::string* node_options) const { | |||
| 345 | 345 | auto match = store_.find("NODE_OPTIONS"); | |
| 346 | 346 | ||
| 347 | 347 | if (match != store_.end()) { | |
| 348 | - *node_options = match->second; | ||
| 348 | + *node_options = " " + match->second; | ||
| 349 | 349 | } | |
| 350 | 350 | } | |
| 351 | 351 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -92,17 +92,17 @@ test('should throw an error when a flag is declared twice', async () => { | |||
| 92 | 92 | assert.strictEqual(result.code, 9); | |
| 93 | 93 | }); | |
| 94 | 94 | ||
| 95 | - test('should override env-file', onlyWithAmaroAndNodeOptions, async () => { | ||
| 95 | + test('should not override env-file', onlyWithAmaroAndNodeOptions, async () => { | ||
| 96 | 96 | const result = await spawnPromisified(process.execPath, [ | |
| 97 | 97 | '--no-warnings', | |
| 98 | 98 | '--experimental-config-file', | |
| 99 | 99 | fixtures.path('rc/transform-types.json'), | |
| 100 | 100 | '--env-file', fixtures.path('dotenv/node-options-no-tranform.env'), | |
| 101 | 101 | fixtures.path('typescript/ts/transformation/test-enum.ts'), | |
| 102 | 102 | ]); | |
| 103 | - assert.strictEqual(result.stderr, ''); | ||
| 104 | - assert.match(result.stdout, /Hello, TypeScript!/); | ||
| 105 | - assert.strictEqual(result.code, 0); | ||
| 103 | + assert.match(result.stderr, /SyntaxError/); | ||
| 104 | + assert.strictEqual(result.stdout, ''); | ||
| 105 | + assert.strictEqual(result.code, 1); | ||
| 106 | 106 | }); | |
| 107 | 107 | ||
| 108 | 108 | test('should not override NODE_OPTIONS', onlyWithAmaro, async () => { | |
0 commit comments