From cf738aea99a817b495ac732fde5030cc52c80f83 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 7 Apr 2024 23:50:45 +0300 Subject: [PATCH] watch: fix some node argument not passed to watched process PR-URL: https://github.com/nodejs/node/pull/52358 Reviewed-By: Moshe Atlow Reviewed-By: Jacob Smith Reviewed-By: Benjamin Gruenbaum --- lib/internal/main/watch_mode.js | 31 ++++-- test/sequential/test-watch-mode.mjs | 164 +++++++++++++++++++++++++++- 2 files changed, 184 insertions(+), 11 deletions(-) diff --git a/lib/internal/main/watch_mode.js b/lib/internal/main/watch_mode.js index ab9f9fa95bc271..c594f64bfca87d 100644 --- a/lib/internal/main/watch_mode.js +++ b/lib/internal/main/watch_mode.js @@ -1,11 +1,12 @@ 'use strict'; const { - ArrayPrototypeFilter, ArrayPrototypeForEach, ArrayPrototypeJoin, ArrayPrototypeMap, + ArrayPrototypePush, ArrayPrototypePushApply, ArrayPrototypeSlice, + StringPrototypeIncludes, StringPrototypeStartsWith, } = primordials; @@ -37,11 +38,21 @@ const kWatchedPaths = ArrayPrototypeMap(getOptionValue('--watch-path'), (path) = const kPreserveOutput = getOptionValue('--watch-preserve-output'); const kCommand = ArrayPrototypeSlice(process.argv, 1); const kCommandStr = inspect(ArrayPrototypeJoin(kCommand, ' ')); -const args = ArrayPrototypeFilter(process.execArgv, (arg, i, arr) => - !StringPrototypeStartsWith(arg, '--watch-path') && - (!arr[i - 1] || !StringPrototypeStartsWith(arr[i - 1], '--watch-path')) && - arg !== '--watch' && !StringPrototypeStartsWith(arg, '--watch=') && arg !== '--watch-preserve-output'); -ArrayPrototypePushApply(args, kCommand); + +const argsWithoutWatchOptions = []; + +for (let i = 0; i < process.execArgv.length; i++) { + const arg = process.execArgv[i]; + if (StringPrototypeStartsWith(arg, '--watch')) { + if (!StringPrototypeIncludes(arg, '=')) { + i++; + } + continue; + } + ArrayPrototypePush(argsWithoutWatchOptions, arg); +} + +ArrayPrototypePushApply(argsWithoutWatchOptions, kCommand); const watcher = new FilesWatcher({ debounce: 200, mode: kShouldFilterModules ? 'filter' : 'all' }); ArrayPrototypeForEach(kWatchedPaths, (p) => watcher.watchPath(p)); @@ -53,7 +64,13 @@ let exited; function start() { exited = false; const stdio = kShouldFilterModules ? ['inherit', 'inherit', 'inherit', 'ipc'] : 'inherit'; - child = spawn(process.execPath, args, { stdio, env: { ...process.env, WATCH_REPORT_DEPENDENCIES: '1' } }); + child = spawn(process.execPath, argsWithoutWatchOptions, { + stdio, + env: { + ...process.env, + WATCH_REPORT_DEPENDENCIES: '1', + }, + }); watcher.watchChildProcessModules(child); child.once('exit', (code) => { exited = true; diff --git a/test/sequential/test-watch-mode.mjs b/test/sequential/test-watch-mode.mjs index ed072456cd6f7e..f7d85dcb3535d9 100644 --- a/test/sequential/test-watch-mode.mjs +++ b/test/sequential/test-watch-mode.mjs @@ -30,7 +30,14 @@ function createTmpFile(content = 'console.log("running");', ext = '.js', basenam } async function runWriteSucceed({ - file, watchedFile, watchFlag = '--watch', args = [file], completed = 'Completed running', restarts = 2, options = {} + file, + watchedFile, + watchFlag = '--watch', + args = [file], + completed = 'Completed running', + restarts = 2, + options = {}, + shouldFail = false }) { const child = spawn(execPath, [watchFlag, '--no-warnings', ...args], { encoding: 'utf8', stdio: 'pipe', ...options }); let completes = 0; @@ -57,6 +64,10 @@ async function runWriteSucceed({ cancelRestarts = restart(watchedFile); } } + + if (!shouldFail && data.startsWith('Failed running')) { + break; + } } } finally { child.kill(); @@ -120,7 +131,12 @@ describe('watch mode', { concurrency: true, timeout: 60_000 }, () => { it('should watch changes to a failing file', async () => { const file = createTmpFile('throw new Error("fails");'); - const { stderr, stdout } = await runWriteSucceed({ file, watchedFile: file, completed: 'Failed running' }); + const { stderr, stdout } = await runWriteSucceed({ + file, + watchedFile: file, + completed: 'Failed running', + shouldFail: true + }); assert.match(stderr, /Error: fails\r?\n/); assert.deepStrictEqual(stdout, [ @@ -159,7 +175,13 @@ describe('watch mode', { concurrency: true, timeout: 60_000 }, () => { const file = path.join(dir, 'non-existing.js'); const watchedFile = createTmpFile('', '.js', dir); const args = ['--watch-path', dir, file]; - const { stderr, stdout } = await runWriteSucceed({ file, watchedFile, args, completed: 'Failed running' }); + const { stderr, stdout } = await runWriteSucceed({ + file, + watchedFile, + args, + completed: 'Failed running', + shouldFail: true + }); assert.match(stderr, /Error: Cannot find module/g); assert.deepStrictEqual(stdout, [ @@ -177,7 +199,13 @@ describe('watch mode', { concurrency: true, timeout: 60_000 }, () => { const file = path.join(dir, 'non-existing.js'); const watchedFile = createTmpFile('', '.js', dir); const args = [`--watch-path=${dir}`, file]; - const { stderr, stdout } = await runWriteSucceed({ file, watchedFile, args, completed: 'Failed running' }); + const { stderr, stdout } = await runWriteSucceed({ + file, + watchedFile, + args, + completed: 'Failed running', + shouldFail: true + }); assert.match(stderr, /Error: Cannot find module/g); assert.deepStrictEqual(stdout, [ @@ -372,4 +400,132 @@ console.log(values.random); `Completed running ${inspect(file)}`, ]); }); + + it('should run when `--watch-path=./foo --require ./bar.js`', { + skip: !supportsRecursive, + }, async () => { + const projectDir = tmpdir.resolve('project2'); + mkdirSync(projectDir); + + const dir = path.join(projectDir, 'watched-dir'); + mkdirSync(dir); + + writeFileSync(path.join(projectDir, 'some.js'), 'console.log(\'hello\')'); + + const file = createTmpFile('console.log(\'running\');', '.js', projectDir); + const watchedFile = createTmpFile('', '.js', dir); + const args = [`--watch-path=${dir}`, '--require', './some.js', file]; + const { stdout, stderr } = await runWriteSucceed({ + file, watchedFile, args, options: { + cwd: projectDir + } + }); + + assert.strictEqual(stderr, ''); + assert.deepStrictEqual(stdout, [ + 'hello', + 'running', + `Completed running ${inspect(file)}`, + `Restarting ${inspect(file)}`, + 'hello', + 'running', + `Completed running ${inspect(file)}`, + ]); + }); + + it('should run when `--watch-path=./foo --require=./bar.js`', { + skip: !supportsRecursive, + }, async () => { + const projectDir = tmpdir.resolve('project3'); + mkdirSync(projectDir); + + const dir = path.join(projectDir, 'watched-dir'); + mkdirSync(dir); + + writeFileSync(path.join(projectDir, 'some.js'), "console.log('hello')"); + + const file = createTmpFile("console.log('running');", '.js', projectDir); + const watchedFile = createTmpFile('', '.js', dir); + const args = [`--watch-path=${dir}`, '--require=./some.js', file]; + const { stdout, stderr } = await runWriteSucceed({ + file, watchedFile, args, options: { + cwd: projectDir + } + }); + + assert.strictEqual(stderr, ''); + assert.deepStrictEqual(stdout, [ + 'hello', + 'running', + `Completed running ${inspect(file)}`, + `Restarting ${inspect(file)}`, + 'hello', + 'running', + `Completed running ${inspect(file)}`, + ]); + }); + + it('should run when `--watch-path ./foo --require ./bar.js`', { + skip: !supportsRecursive, + }, async () => { + const projectDir = tmpdir.resolve('project5'); + mkdirSync(projectDir); + + const dir = path.join(projectDir, 'watched-dir'); + mkdirSync(dir); + + writeFileSync(path.join(projectDir, 'some.js'), 'console.log(\'hello\')'); + + const file = createTmpFile('console.log(\'running\');', '.js', projectDir); + const watchedFile = createTmpFile('', '.js', dir); + const args = ['--watch-path', `${dir}`, '--require', './some.js', file]; + const { stdout, stderr } = await runWriteSucceed({ + file, watchedFile, args, options: { + cwd: projectDir + } + }); + + assert.strictEqual(stderr, ''); + assert.deepStrictEqual(stdout, [ + 'hello', + 'running', + `Completed running ${inspect(file)}`, + `Restarting ${inspect(file)}`, + 'hello', + 'running', + `Completed running ${inspect(file)}`, + ]); + }); + + it('should run when `--watch-path=./foo --require=./bar.js`', { + skip: !supportsRecursive, + }, async () => { + const projectDir = tmpdir.resolve('project6'); + mkdirSync(projectDir); + + const dir = path.join(projectDir, 'watched-dir'); + mkdirSync(dir); + + writeFileSync(path.join(projectDir, 'some.js'), "console.log('hello')"); + + const file = createTmpFile("console.log('running');", '.js', projectDir); + const watchedFile = createTmpFile('', '.js', dir); + const args = ['--watch-path', `${dir}`, '--require=./some.js', file]; + const { stdout, stderr } = await runWriteSucceed({ + file, watchedFile, args, options: { + cwd: projectDir + } + }); + + assert.strictEqual(stderr, ''); + assert.deepStrictEqual(stdout, [ + 'hello', + 'running', + `Completed running ${inspect(file)}`, + `Restarting ${inspect(file)}`, + 'hello', + 'running', + `Completed running ${inspect(file)}`, + ]); + }); });