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.

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
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
38 changes: 34 additions & 4 deletions lib/internal/main/watch_mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,40 @@ 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');

const watchOptions = ['--watch', '--watch-path', '--watch-preserve-output'];

function argIsWatchOptionWithValue(arg) {
return watchOptions.some((watchOption) => StringPrototypeStartsWith(arg, `${watchOption}=`));
}

function argIsWatchOption(arg) {
return watchOptions.includes(arg) || argIsWatchOptionWithValue(arg);
}

const args = ArrayPrototypeFilter(process.execArgv, (arg, i, arr) => {
rluvaton marked this conversation as resolved.
Show resolved Hide resolved
// Don't pass --watch related args to children
if (argIsWatchOption(arg)) {
return false;
}

// First arg so it cant be a value of some watch option
if (i === 0) {
return true;
}

const prevArg = arr[i - 1];
const prevArgIsWatchOption = argIsWatchOption(prevArg);

// This is not a value of arg option as previous arg is also not watch related
if (!prevArgIsWatchOption) {
return true;
}

// If prev arg is watch related, but it has a value it means than this arg is not a value of such option
return argIsWatchOptionWithValue(prevArg);
});

ArrayPrototypePushApply(args, kCommand);

const watcher = new FilesWatcher({ debounce: 200, mode: kShouldFilterModules ? 'filter' : 'all' });
Expand Down
149 changes: 145 additions & 4 deletions test/sequential/test-watch-mode.mjs
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,117 @@ console.log(values.random);
`Completed running ${inspect(file)}`,
]);
});

it('when --watch-path has an equals and --require does not and --require is after the --watch-path and require is' +
'requiring from node modules package exports, required path should run', {
skip: !supportsRecursive,
}, async () => {
const projectDir = tmpdir.resolve('project1');
mkdirSync(projectDir);

const dir = path.join(projectDir, 'watched-dir');
mkdirSync(dir);

// Create a package example
const demoPackageDir = path.join(projectDir, 'node_modules/demo');
mkdirSync(demoPackageDir, { recursive: true });

writeFileSync(path.join(demoPackageDir, 'package.json'), JSON.stringify({
'name': 'demo',
'version': '1.0.0',
'main': 'some.js',
'exports': {
'./some': './some.js'
}
}
));

writeFileSync(path.join(demoPackageDir, 'some.js'), 'console.log("hello")');

const file = createTmpFile(undefined, undefined, projectDir);
const watchedFile = createTmpFile('', '.js', dir);
const args = [`--watch-path=${dir}`, '--require', 'demo/some', 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)}`,
]);
});
rluvaton marked this conversation as resolved.
Show resolved Hide resolved

it('when --watch-path has an equals and --require does not and --require is after the --watch-path,' +
'the required file and the main module file should ran', {
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)}`,
]);
});
rluvaton marked this conversation as resolved.
Show resolved Hide resolved

it('when --watch-path and --require has an equals and --require is after the --watch-path,' +
'the required file and the main module file should ran', {
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)}`,
]);
});
rluvaton marked this conversation as resolved.
Show resolved Hide resolved
});