Commit 59860eb
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 d578343 commit 59860eb
4 files changed
Lines changed: 14 additions & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1143,12 +1143,12 @@ node --import amaro/strip --watch-path=src --watch-preserve-output --test-isolat | |||
| 1143 | 1143 | The priority in configuration is as follows: | |
| 1144 | 1144 | ||
| 1145 | 1145 | 1. NODE\_OPTIONS and command-line options | |
| 1146 | - 2. Configuration file | ||
| 1147 | - 3. Dotenv NODE\_OPTIONS | ||
| 1146 | + 2. Dotenv NODE\_OPTIONS | ||
| 1147 | + 3. Configuration file | ||
| 1148 | 1148 | ||
| 1149 | 1149 | Values in the configuration file will not override the values in the environment | |
| 1150 | - variables and command-line options, but will override the values in the `NODE_OPTIONS` | ||
| 1151 | - env file parsed by the `--env-file` flag. | ||
| 1150 | + variables, command-line options, or the `NODE_OPTIONS` env file parsed by the | ||
| 1151 | + `--env-file` flag. | ||
| 1152 | 1152 | ||
| 1153 | 1153 | Keys cannot be duplicated within the same or different namespaces. | |
| 1154 | 1154 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -870,6 +870,7 @@ static ExitCode InitializeNodeWithArgsInternal( | |||
| 870 | 870 | HandleEnvOptions(per_process::cli_options->per_isolate->per_env); | |
| 871 | 871 | ||
| 872 | 872 | std::string node_options; | |
| 873 | + std::string node_options_from_dotenv; | ||
| 873 | 874 | auto env_files = node::Dotenv::GetDataFromArgs(*argv); | |
| 874 | 875 | ||
| 875 | 876 | if (!env_files.empty()) { | |
@@ -896,7 +897,8 @@ static ExitCode InitializeNodeWithArgsInternal( | |||
| 896 | 897 | } | |
| 897 | 898 | } | |
| 898 | 899 | ||
| 899 | - per_process::dotenv_file.AssignNodeOptionsIfAvailable(&node_options); | ||
| 900 | + per_process::dotenv_file.AssignNodeOptionsIfAvailable( | ||
| 901 | + &node_options_from_dotenv); | ||
| 900 | 902 | } | |
| 901 | 903 | ||
| 902 | 904 | std::string node_options_from_config; | |
@@ -932,9 +934,10 @@ static ExitCode InitializeNodeWithArgsInternal( | |||
| 932 | 934 | errors->emplace_back("The number of NODE_OPTIONS doesn't match " | |
| 933 | 935 | "the number of flags in the config file"); | |
| 934 | 936 | } | |
| 935 | - node_options += node_options_from_config; | ||
| 936 | 937 | } | |
| 937 | 938 | ||
| 939 | + node_options = node_options_from_config + node_options_from_dotenv; | ||
| 940 | + | ||
| 938 | 941 | #if !defined(NODE_WITHOUT_NODE_OPTIONS) | |
| 939 | 942 | bool should_parse_node_options = | |
| 940 | 943 | !(flags & ProcessInitializationFlags::kDisableNodeOptionsEnv); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -356,7 +356,7 @@ void Dotenv::AssignNodeOptionsIfAvailable(std::string* node_options) const { | |||
| 356 | 356 | auto match = store_.find("NODE_OPTIONS"); | |
| 357 | 357 | ||
| 358 | 358 | if (match != store_.end()) { | |
| 359 | - *node_options = match->second; | ||
| 359 | + *node_options = " " + match->second; | ||
| 360 | 360 | } | |
| 361 | 361 | } | |
| 362 | 362 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -86,16 +86,16 @@ test('should throw an error when a flag is declared twice', async () => { | |||
| 86 | 86 | assert.strictEqual(result.code, 9); | |
| 87 | 87 | }); | |
| 88 | 88 | ||
| 89 | - test('should override env-file', onlyWithAmaroAndNodeOptions, async () => { | ||
| 89 | + test('should not override env-file', onlyWithAmaroAndNodeOptions, async () => { | ||
| 90 | 90 | const result = await spawnPromisified(process.execPath, [ | |
| 91 | 91 | '--no-warnings', | |
| 92 | 92 | `--experimental-config-file=${fixtures.path('rc/strip-types.json')}`, | |
| 93 | 93 | '--env-file', fixtures.path('dotenv/node-options-no-tranform.env'), | |
| 94 | 94 | fixtures.path('typescript/ts/test-typescript.ts'), | |
| 95 | 95 | ]); | |
| 96 | - assert.strictEqual(result.stderr, ''); | ||
| 97 | - assert.match(result.stdout, /Hello, TypeScript!/); | ||
| 98 | - assert.strictEqual(result.code, 0); | ||
| 96 | + assert.match(result.stderr, /SyntaxError/); | ||
| 97 | + assert.strictEqual(result.stdout, ''); | ||
| 98 | + assert.strictEqual(result.code, 1); | ||
| 99 | 99 | }); | |
| 100 | 100 | ||
| 101 | 101 | test('should not override NODE_OPTIONS', onlyWithAmaro, async () => { | |
0 commit comments