Skip to content

Commit ab05798

Browse files
trivikrrichardlau
trivikr
authored and
richardlau
committed
watch: strip underscore aliases of watch flags
The C++ option parser treats `_` and `-` as the same character in option names, so `--watch_path` is the same as `--watch-path`. Watch mode compared spellings literally when it removed its own flags from `process.execArgv` and `NODE_OPTIONS` before spawning the child. The underscore spellings got through, so each child went back into watch mode and spawned another child, and the application never ran. Normalize option names before comparing them. Forward the original arguments to the child unchanged. Signed-off-by: Trivikram Kamat <[email protected]> Assisted-by: claude:opus-5.5 PR-URL: #66365 Fixes: #66364 Reviewed-By: Moshe Atlow <[email protected]>
1 parent 7f1937d commit ab05798

2 files changed

Lines changed: 80 additions & 9 deletions

File tree

‎lib/internal/main/watch_mode.js‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ const {
77
ArrayPrototypePushApply,
88
ArrayPrototypeSlice,
99
StringPrototypeIncludes,
10+
StringPrototypeIndexOf,
1011
StringPrototypeReplaceAll,
12+
StringPrototypeSlice,
1113
StringPrototypeStartsWith,
1214
} = primordials;
1315

@@ -42,9 +44,23 @@ const kPreserveOutput = getOptionValue('--watch-preserve-output');
4244
const kCommand = ArrayPrototypeSlice(process.argv, 1);
4345
const kCommandStr = inspect(ArrayPrototypeJoin(kCommand, ' '));
4446

47+
// The C++ option parser treats `_` and `-` as equivalent in option names
48+
// (e.g. --watch_path is --watch-path), so normalize before matching.
49+
function normalizeOptionName(arg) {
50+
if (!StringPrototypeStartsWith(arg, '--')) {
51+
return arg;
52+
}
53+
const equalsIndex = StringPrototypeIndexOf(arg, '=');
54+
if (equalsIndex === -1) {
55+
return StringPrototypeReplaceAll(arg, '_', '-');
56+
}
57+
return StringPrototypeReplaceAll(StringPrototypeSlice(arg, 0, equalsIndex), '_', '-') +
58+
StringPrototypeSlice(arg, equalsIndex);
59+
}
60+
4561
const argsWithoutWatchOptions = [];
4662
for (let i = 0; i < process.execArgv.length; i++) {
47-
const arg = process.execArgv[i];
63+
const arg = normalizeOptionName(process.execArgv[i]);
4864
if (StringPrototypeStartsWith(arg, '--watch=')) {
4965
continue;
5066
}
@@ -77,7 +93,7 @@ for (let i = 0; i < process.execArgv.length; i++) {
7793
if (arg === '--experimental-default-config-file') {
7894
continue;
7995
}
80-
ArrayPrototypePush(argsWithoutWatchOptions, arg);
96+
ArrayPrototypePush(argsWithoutWatchOptions, process.execArgv[i]);
8197
}
8298

8399
ArrayPrototypePushApply(argsWithoutWatchOptions, kCommand);
@@ -91,15 +107,16 @@ if (kNodeOptions != null) {
91107
const parts = parseNodeOptionsEnvVar(kNodeOptions);
92108
for (let i = 0; i < parts.length; i++) {
93109
const part = parts[i];
94-
if (part === '--watch' ||
95-
part === '--watch-preserve-output' ||
96-
StringPrototypeStartsWith(part, '--watch=') ||
97-
StringPrototypeStartsWith(part, '--watch-preserve-output=') ||
98-
StringPrototypeStartsWith(part, '--watch-path=') ||
99-
StringPrototypeStartsWith(part, '--watch-kill-signal=')) {
110+
const name = normalizeOptionName(part);
111+
if (name === '--watch' ||
112+
name === '--watch-preserve-output' ||
113+
StringPrototypeStartsWith(name, '--watch=') ||
114+
StringPrototypeStartsWith(name, '--watch-preserve-output=') ||
115+
StringPrototypeStartsWith(name, '--watch-path=') ||
116+
StringPrototypeStartsWith(name, '--watch-kill-signal=')) {
100117
continue;
101118
}
102-
if (part === '--watch-path' || part === '--watch-kill-signal') {
119+
if (name === '--watch-path' || name === '--watch-kill-signal') {
103120
// Skip the flag and its separate value argument
104121
i++;
105122
continue;

‎test/sequential/test-watch-mode.mjs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,6 +1033,60 @@ process.on('message', (message) => {
10331033
}
10341034
});
10351035

1036+
it('should strip underscore aliases of watch flags from NODE_OPTIONS', async () => {
1037+
// Avoid recursively watching the repository's source and test trees.
1038+
const cwd = tmpdir.resolve('node-options-underscore');
1039+
mkdirSync(path.join(cwd, 'src'), { recursive: true });
1040+
mkdirSync(path.join(cwd, 'test'));
1041+
const file = createTmpFile('console.log(process.env.NODE_OPTIONS);');
1042+
const nodeOptions = [
1043+
'--watch_path=./src',
1044+
'--watch_path', './test',
1045+
'--watch_preserve_output',
1046+
'--watch_preserve_output=true',
1047+
'--watch_kill_signal=SIGKILL',
1048+
'--watch_kill_signal', 'SIGINT',
1049+
'--max_old_space_size=4096',
1050+
'--no-warnings',
1051+
].join(' ');
1052+
const { done, restart } = runInBackground({
1053+
args: ['--watch', file],
1054+
options: {
1055+
cwd,
1056+
env: { ...process.env, NODE_OPTIONS: nodeOptions },
1057+
},
1058+
});
1059+
1060+
try {
1061+
const { stdout, stderr } = await restart();
1062+
1063+
assert.strictEqual(stderr, '');
1064+
const nodeOptionsLine = stdout.find((line) => line.includes('--max_old_space_size'));
1065+
assert.ok(nodeOptionsLine);
1066+
assert.strictEqual(nodeOptionsLine, '--max_old_space_size=4096 --no-warnings');
1067+
} finally {
1068+
await done();
1069+
}
1070+
});
1071+
1072+
it('should strip --watch_path from execArgv in child process', async () => {
1073+
const dir = tmpdir.resolve('exec-argv-underscore');
1074+
mkdirSync(dir, { recursive: true });
1075+
const file = createTmpFile('console.log(JSON.stringify(process.execArgv));');
1076+
const { done, restart } = runInBackground({
1077+
args: [`--watch_path=${dir}`, file],
1078+
});
1079+
1080+
try {
1081+
const { stdout, stderr } = await restart();
1082+
1083+
assert.strictEqual(stderr, '');
1084+
assert.ok(stdout.includes('["--no-warnings"]'));
1085+
} finally {
1086+
await done();
1087+
}
1088+
});
1089+
10361090
it('should not strip --watch when it appears inside a quoted NODE_OPTIONS value', {
10371091
// Honoring --require from NODE_OPTIONS is required for this test.
10381092
skip: !!process.config.variables.node_without_node_options,

0 commit comments

Comments
 (0)