Repository: nodejs/node
Pull request: watch: fix watch args not being properly filtered (second attempt)

I'm re-introducing this change that previously landed in https://github.com/nodejs/node/pull/57936 but got reverted in https://github.com/nodejs/node/pull/58190.

I believe that one of the main reasons for reverting the PR was the fact that when it landed it didn't include the changes that I however later introduced in https://github.com/nodejs/node/pull/58183 (that also got reverted as part of #58190) (making me wonder if the revert was actually completely necessary after all :thinking:).

Another reason for the revert was that I was adding the tests to the `test/sequential/test-watch-mode.mjs` file (which is marked as flaky) instead of creating a new test file.

I am not sure if there were other issues associated to the original PR, however I am opening this PR that reintroduces the changes and also addresses the two above issues (if there were other issues I'd imagine we'll see them when running ci against this PR).

____

Fixes: https://github.com/nodejs/node/issues/57124

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/internal/main/watch_mode.js
patch: @@ -43,11 +43,26 @@ const argsWithoutWatchOptions = [];
 
 for (let i = 0; i < process.execArgv.length; i++) {
   const arg = process.execArgv[i];
-  if (StringPrototypeStartsWith(arg, '--watch')) {
-    i++;
-    const nextArg = process.execArgv[i];
-    if (nextArg && nextArg[0] === '-') {
-      ArrayPrototypePush(argsWithoutWatchOptions, nextArg);
+  if (StringPrototypeStartsWith(arg, '--watch=')) {
+    continue;
+  }
+  if (arg === '--watch') {
+    const nextArg = process.execArgv[i + 1];
+    if (nextArg && nextArg[0] !== '-') {
+      // If `--watch` doesn't include `=` and the next
+      // argument is not a flag then it is interpreted as
+      // the watch argument, so we need to skip that as well
+      i++;
+    }
+    continue;
+  }
+  if (StringPrototypeStartsWith(arg, '--watch-path')) {
+    const lengthOfWatchPathStr = 12;
+    if (arg[lengthOfWatchPathStr] !== '=') {
+      // if --watch-path doesn't include `=` it means
+      // that the next arg is the target path, so we
+      // need to skip that as well
+      i++;
     }
     continue;
   }

filename: test/sequential/test-watch-mode-watch-flags.mjs
patch: @@ -0,0 +1,97 @@
+import * as common from '../common/index.mjs';
+import tmpdir from '../common/tmpdir.js';
+import assert from 'node:assert';
+import path from 'node:path';
+import { execPath } from 'node:process';
+import { describe, it } from 'node:test';
+import { spawn } from 'node:child_process';
+import { writeFileSync, mkdirSync } from 'node:fs';
+import { inspect } from 'node:util';
+import { createInterface } from 'node:readline';
+
+if (common.isIBMi)
+  common.skip('IBMi does not support `fs.watch()`');
+
+let tmpFiles = 0;
+function createTmpFile(content, ext, basename) {
+  const file = path.join(basename, `${tmpFiles++}${ext}`);
+  writeFileSync(file, content);
+  return file;
+}
+
+async function runNode({
+  args,
+  expectedCompletionLog = 'Completed running',
+  options = {},
+}) {
+  const child = spawn(execPath, args, { encoding: 'utf8', stdio: 'pipe', ...options });
+  let stderr = '';
+  const stdout = [];
+
+  child.stderr.on('data', (data) => {
+    stderr += data;
+  });
+
+  try {
+    // Break the chunks into lines
+    for await (const data of createInterface({ input: child.stdout })) {
+      if (!data.startsWith('Waiting for graceful termination') && !data.startsWith('Gracefully restarted')) {
+        stdout.push(data);
+      }
+      if (data.startsWith(expectedCompletionLog)) {
+        break;
+      }
+    }
+  } finally {
+    child.kill();
+  }
+  return { stdout, stderr, pid: child.pid };
+}
+
+tmpdir.refresh();
+
+describe('watch mode - watch flags', { concurrency: !process.env.TEST_PARALLEL, timeout: 60_000 }, () => {
+  it('when multiple `--watch` flags are provided should run as if only one was', async () => {
+    const projectDir = tmpdir.resolve('project-multi-flag');
+    mkdirSync(projectDir);
+
+    const file = createTmpFile(`
+      console.log(
+        process.argv.some(arg => arg === '--watch')
+        ? 'Error: unexpected --watch args present'
+        : 'no --watch args present'
+      );`, '.js', projectDir);
+    const args = ['--watch', '--watch', file];
+    const { stdout, stderr } = await runNode({
+      file, args, options: { cwd: projectDir }
+    });
+
+    assert.strictEqual(stderr, '');
+    assert.deepStrictEqual(stdout, [
+      'no --watch args present',
+      `Completed running ${inspect(file)}. Waiting for file changes before restarting...`,
+    ]);
+  });
+
+  it('`--watch-path` args without `=` used alongside `--watch` should not make it into the script', async () => {
+    const projectDir = tmpdir.resolve('project-watch-watch-path-args');
+    mkdirSync(projectDir);
+
+    const file = createTmpFile(`
+      console.log(
+        process.argv.slice(2).some(arg => arg.endsWith('.js'))
+        ? 'some cli args end with .js'
+        : 'no cli arg ends with .js'
+      );`, '.js', projectDir);
+    const args = ['--watch', `--watch-path`, file, file];
+    const { stdout, stderr } = await runNode({
+      file, args, options: { cwd: projectDir }
+    });
+
+    assert.strictEqual(stderr, '');
+    assert.deepStrictEqual(stdout, [
+      'no cli arg ends with .js',
+      `Completed running ${inspect(file)}. Waiting for file changes before restarting...`,
+    ]);
+  });
+});