Skip to content

Commit 3dc9b42

Browse files
marco-ippolitomarco-ippolito
committed
src: allow empty --experimental-config-file
1 parent f6464c5 commit 3dc9b42

File tree

9 files changed

+66
-17
lines changed

9 files changed

+66
-17
lines changed

‎doc/api/cli.md‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,7 +1014,7 @@ added:
10141014
10151015
Enable experimental import support for `.node` addons.
10161016

1017-
### `--experimental-config-file=config`
1017+
### `--experimental-config-file=`, `--experimental-config-file`
10181018

10191019
<!-- YAML
10201020
added:
@@ -1025,6 +1025,10 @@ added:
10251025
> Stability: 1.0 - Early development
10261026
10271027
If present, Node.js will look for a configuration file at the specified path.
1028+
If the path is not specified, Node.js will look for a `node.config.json` file
1029+
in the current working directory.
1030+
The alias `--experimental-default-config-file` is equivalent to
1031+
`--experimental-config-file` without an argument.
10281032
Node.js will read the configuration file and apply the settings. The
10291033
configuration file should be a JSON file with the following structure. `vX.Y.Z`
10301034
in the `$schema` must be replaced with the version of Node.js you are using.
@@ -1131,9 +1135,10 @@ added:
11311135

11321136
> Stability: 1.0 - Early development
11331137
1134-
If the `--experimental-default-config-file` flag is present, Node.js will look for a
1138+
This flag is an alias for `--experimental-config-file` without an argument.
1139+
If present, Node.js will look for a
11351140
`node.config.json` file in the current working directory and load it as a
1136-
as configuration file.
1141+
configuration file.
11371142

11381143
### `--experimental-eventsource`
11391144

‎doc/api/test.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4131,7 +4131,7 @@ Can be used to abort test subtasks when the test has been aborted.
41314131
[`suite()`]: #suitename-options-fn
41324132
[`test()`]: #testname-options-fn
41334133
[code coverage]: #collecting-code-coverage
4134-
[configuration files]: cli.md#--experimental-config-fileconfig
4134+
[configuration files]: cli.md#experimental-config-file
41354135
[describe options]: #describename-options-fn
41364136
[it options]: #testname-options-fn
41374137
[running tests from the command line]: #running-tests-from-the-command-line

‎doc/node.1‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -602,8 +602,10 @@ It is possible to run code containing inline types unless the
602602
.It Fl -experimental-addon-modules
603603
Enable experimental import support for \fB.node\fR addons.
604604
.
605-
.It Fl -experimental-config-file Ns = Ns Ar config
605+
.It Fl -experimental-config-file= Ns Ar config , Fl -experimental-config-file , Fl -experimental-default-config-file
606606
If present, Node.js will look for a configuration file at the specified path.
607+
If the path is not specified, Node.js will look for a \fBnode.config.json\fR file
608+
in the current working directory.
607609
Node.js will read the configuration file and apply the settings. The
608610
configuration file should be a JSON file with the following structure. \fBvX.Y.Z\fR
609611
in the \fB$schema\fR must be replaced with the version of Node.js you are using.
@@ -689,9 +691,10 @@ Node.js will not sanitize or perform validation on the user-provided configurati
689691
so \fBNEVER\fR use untrusted configuration files.
690692
.
691693
.It Fl -experimental-default-config-file
692-
If the \fB--experimental-default-config-file\fR flag is present, Node.js will look for a
694+
This flag is an alias for \fB--experimental-config-file\fR without an argument.
695+
If present, Node.js will look for a
693696
\fBnode.config.json\fR file in the current working directory and load it as a
694-
as configuration file.
697+
configuration file.
695698
.
696699
.It Fl -experimental-eventsource
697700
Enable exposition of EventSource Web API on the global scope.

‎lib/internal/process/pre_execution.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,8 @@ function setupSQLite() {
382382

383383
function initializeConfigFileSupport() {
384384
if (getOptionValue('--experimental-default-config-file') ||
385-
getOptionValue('--experimental-config-file')) {
385+
getOptionValue('--experimental-config-file') ||
386+
getOptionValue('--experimental-config-file=')) {
386387
emitExperimentalWarning('--experimental-config-file');
387388
}
388389
}

‎src/node_config_file.cc‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,22 @@ std::optional<std::string_view> ConfigReader::GetDataFromArgs(
1717
for (auto it = args.begin(); it != args.end(); ++it) {
1818
if (*it == flag_path) {
1919
// Case: "--experimental-config-file foo"
20-
if (auto next = std::next(it); next != args.end()) {
20+
if (auto next = std::next(it);
21+
next != args.end() && !next->starts_with("-")) {
2122
return *next;
2223
}
24+
// Case: "--experimental-config-file" without argument, use default
25+
has_default_config_file = true;
2326
} else if (it->starts_with(flag_path)) {
2427
// Case: "--experimental-config-file=foo"
2528
if (it->size() > flag_path.size() && (*it)[flag_path.size()] == '=') {
26-
return std::string_view(*it).substr(flag_path.size() + 1);
29+
std::string_view value =
30+
std::string_view(*it).substr(flag_path.size() + 1);
31+
if (value.empty()) {
32+
has_default_config_file = true;
33+
} else {
34+
return value;
35+
}
2736
}
2837
} else if (*it == default_file || it->starts_with(default_file)) {
2938
has_default_config_file = true;

‎src/node_options.cc‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -868,12 +868,16 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
868868
"set environment variables from supplied file",
869869
&EnvironmentOptions::optional_env_file);
870870
Implies("--env-file-if-exists", "[has_env_file_string]");
871-
AddOption("--experimental-config-file",
871+
AddOption("--experimental-config-file=",
872872
"set config file from supplied file",
873-
&EnvironmentOptions::experimental_config_file_path);
874-
AddOption("--experimental-default-config-file",
875-
"set config file from default config file",
876-
&EnvironmentOptions::experimental_default_config_file);
873+
&EnvironmentOptions::experimental_config_file_path,
874+
kDisallowedInEnvvar);
875+
AddOption("--experimental-config-file",
876+
"set default config file",
877+
&EnvironmentOptions::experimental_default_config_file,
878+
kDisallowedInEnvvar);
879+
AddAlias("--experimental-config-file <arg>", "--experimental-config-file=");
880+
AddAlias("--experimental-default-config-file", "--experimental-config-file");
877881
AddOption("--test",
878882
"launch test runner on startup",
879883
&EnvironmentOptions::test_runner,

‎test/parallel/test-cli-node-options-docs.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,5 +122,6 @@ for (const [, envVar, config] of nodeOptionsCC.matchAll(addOptionRE)) {
122122

123123
// add alias handling
124124
manPagesOptions.delete('-trace-events-enabled');
125+
manPagesOptions.delete('-experimental-default-config-file');
125126

126127
assert.strictEqual(manPagesOptions.size, 0, `Man page options not documented: ${[...manPagesOptions]}`);

‎test/parallel/test-config-file.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,34 @@ test('should use node.config.json as default', onlyIfNodeOptionsSupport, async (
371371
assert.strictEqual(result.code, 0);
372372
});
373373

374+
test('should use node.config.json when --experimental-config-file has no argument',
375+
onlyIfNodeOptionsSupport, async () => {
376+
const result = await spawnPromisified(process.execPath, [
377+
'--no-warnings',
378+
'--experimental-config-file',
379+
'-p', 'http.maxHeaderSize',
380+
], {
381+
cwd: fixtures.path('rc/default'),
382+
});
383+
assert.strictEqual(result.stderr, '');
384+
assert.strictEqual(result.stdout, '10\n');
385+
assert.strictEqual(result.code, 0);
386+
});
387+
388+
test('should use node.config.json when --experimental-config-file= has empty argument',
389+
onlyIfNodeOptionsSupport, async () => {
390+
const result = await spawnPromisified(process.execPath, [
391+
'--no-warnings',
392+
'--experimental-config-file=',
393+
'-p', 'http.maxHeaderSize',
394+
], {
395+
cwd: fixtures.path('rc/default'),
396+
});
397+
assert.strictEqual(result.stderr, '');
398+
assert.strictEqual(result.stdout, '10\n');
399+
assert.strictEqual(result.code, 0);
400+
});
401+
374402
test('should override node.config.json when specificied', onlyIfNodeOptionsSupport, async () => {
375403
const result = await spawnPromisified(process.execPath, [
376404
'--no-warnings',

‎test/parallel/test-runner-flag-propagation.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ const runner = path.join(fixtureDir, 'runner.mjs');
1515
describe('test runner flag propagation', () => {
1616
describe('via command line', () => {
1717
const flagPropagationTests = [
18-
['--experimental-config-file', 'node.config.json', ''],
19-
['--experimental-default-config-file', '', false],
2018
['--env-file', '.env', '.env'],
2119
['--env-file-if-exists', '.env', '.env'],
2220
['--test-concurrency', '2', '2'],

0 commit comments

Comments
 (0)