Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

watch: fix some node argument not passed to watched process #52358

Merged
merged 4 commits into from
Apr 7, 2024
Merged

watch: fix some node argument not passed to watched process #52358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a4e308
watch: fix some node argument not passed to watched process
rluvaton Apr 3, 2024
8bba166
test: add tests for without equals
rluvaton Apr 4, 2024
57756e8
test: remove node module test as it covert in other places + simplify…
rluvaton Apr 4, 2024
dbf6246
watch: use primordials and fix lint
rluvaton Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions lib/internal/main/watch_mode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
'use strict';
const {
ArrayPrototypeFilter,
ArrayPrototypeForEach,
ArrayPrototypeJoin,
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypePushApply,
ArrayPrototypeSlice,
StringPrototypeIncludes,
StringPrototypeStartsWith,
} = primordials;

Expand Down Expand Up @@ -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));
Expand All @@ -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;
Expand Down
164 changes: 160 additions & 4 deletions test/sequential/test-watch-mode.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
rluvaton marked this conversation as resolved.
Show resolved Hide resolved
}) {
const child = spawn(execPath, [watchFlag, '--no-warnings', ...args], { encoding: 'utf8', stdio: 'pipe', ...options });
let completes = 0;
Expand All @@ -57,6 +64,10 @@ async function runWriteSucceed({
cancelRestarts = restart(watchedFile);
}
}

if (!shouldFail && data.startsWith('Failed running')) {
break;
}
}
} finally {
child.kill();
Expand Down Expand Up @@ -120,7 +131,12 @@ describe('watch mode', { concurrency: !process.env.TEST_PARALLEL, timeout: 60_00

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
rluvaton marked this conversation as resolved.
Show resolved Hide resolved
});

assert.match(stderr, /Error: fails\r?\n/);
assert.deepStrictEqual(stdout, [
Expand Down Expand Up @@ -159,7 +175,13 @@ describe('watch mode', { concurrency: !process.env.TEST_PARALLEL, timeout: 60_00
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
rluvaton marked this conversation as resolved.
Show resolved Hide resolved
});

assert.match(stderr, /Error: Cannot find module/g);
assert.deepStrictEqual(stdout, [
Expand All @@ -177,7 +199,13 @@ describe('watch mode', { concurrency: !process.env.TEST_PARALLEL, timeout: 60_00
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
rluvaton marked this conversation as resolved.
Show resolved Hide resolved
});

assert.match(stderr, /Error: Cannot find module/g);
assert.deepStrictEqual(stdout, [
Expand Down Expand Up @@ -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)}`,
]);
});
});