Skip to content

Commit

Permalink
Fixes #50880 - enable passthrough IPC in watch mode
Browse files Browse the repository at this point in the history
  • Loading branch information
znewsham committed Nov 24, 2023
1 parent 8c72210 commit 1c04659
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 7 deletions.
17 changes: 10 additions & 7 deletions lib/internal/main/watch_mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function start() {
process.stdout.write(`${red}Failed running ${kCommandStr}${white}\n`);
}
});
return child;
}

async function killAndWait(signal = kKillSignal, force = false) {
Expand Down Expand Up @@ -96,29 +97,31 @@ function reportGracefulTermination() {
};
}

async function stop() {
async function stop(child) {
// without this line, the child process is still able to receive IPC, but is unable to send additional messages

Check failure on line 101 in lib/internal/main/watch_mode.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Comments should not begin with a lowercase character
watcher.destroyIPC(child);
watcher.clearFileFilters();
const clearGraceReport = reportGracefulTermination();
await killAndWait();
clearGraceReport();
}

async function restart() {
async function restart(child) {
if (!kPreserveOutput) process.stdout.write(clear);
process.stdout.write(`${green}Restarting ${kCommandStr}${white}\n`);
await stop();
start();
await stop(child);
return start();
}

(async () => {
emitExperimentalWarning('Watch mode');

let child;
try {
start();
child = start();

// eslint-disable-next-line no-unused-vars
for await (const _ of on(watcher, 'changed')) {
await restart();
child = await restart(child);
}
} catch (error) {
triggerUncaughtException(error, true /* fromPromise */);
Expand Down
23 changes: 23 additions & 0 deletions lib/internal/watch_mode/files_watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class FilesWatcher extends EventEmitter {
#debounce;
#mode;
#signal;
#wantsPassthroughIPC = false;

constructor({ debounce = 200, mode = 'filter', signal } = kEmptyObject) {
super();
Expand All @@ -39,6 +40,7 @@ class FilesWatcher extends EventEmitter {
this.#debounce = debounce;
this.#mode = mode;
this.#signal = signal;
this.#wantsPassthroughIPC = !!process.send;

if (signal) {
EventEmitter.addAbortListener(signal, () => this.clear());
Expand Down Expand Up @@ -124,7 +126,28 @@ class FilesWatcher extends EventEmitter {
this.#ownerDependencies.set(owner, dependencies);
}
}


#setupIPC(child) {
child._ipcMessages = {
parentToChild: message => child.send(message),

Check failure on line 133 in lib/internal/watch_mode/files_watcher.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected parentheses around arrow function argument
childToParent: message => process.send(message)

Check failure on line 134 in lib/internal/watch_mode/files_watcher.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected parentheses around arrow function argument

Check failure on line 134 in lib/internal/watch_mode/files_watcher.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Missing trailing comma
};
process.on("message", child._ipcMessages.parentToChild);

Check failure on line 136 in lib/internal/watch_mode/files_watcher.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Strings must use singlequote
child.on("message", child._ipcMessages.childToParent);

Check failure on line 137 in lib/internal/watch_mode/files_watcher.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Strings must use singlequote
}

destroyIPC(child) {
if (this.#wantsPassthroughIPC) {
process.off("message", child._ipcMessages.parentToChild);

Check failure on line 142 in lib/internal/watch_mode/files_watcher.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Strings must use singlequote
child.off("message", child._ipcMessages.childToParent);

Check failure on line 143 in lib/internal/watch_mode/files_watcher.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Strings must use singlequote
}
}

watchChildProcessModules(child, key = null) {
if (this.#wantsPassthroughIPC) {
this.#setupIPC(child);
}
if (this.#mode !== 'filter') {
return;
}
Expand Down
73 changes: 73 additions & 0 deletions test/sequential/test-watch-mode.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import fs from 'fs/promises';

Check failure on line 1 in test/sequential/test-watch-mode.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Mandatory module "common" must be loaded before any other modules

Check failure on line 1 in test/sequential/test-watch-mode.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'fs' is defined but never used
import * as common from '../common/index.mjs';
import tmpdir from '../common/tmpdir.js';
import assert from 'node:assert';
Expand All @@ -8,6 +9,7 @@ import { spawn } from 'node:child_process';
import { writeFileSync, readFileSync, mkdirSync } from 'node:fs';
import { inspect } from 'node:util';
import { pathToFileURL } from 'node:url';
import { once } from 'node:events';
import { createInterface } from 'node:readline';

if (common.isIBMi)
Expand Down Expand Up @@ -293,6 +295,7 @@ console.log(values.random);
]);
});


// TODO: Remove skip after https://github.com/nodejs/node/pull/45271 lands
it('should not watch when running an missing file', {
skip: !supportsRecursive
Expand Down Expand Up @@ -356,4 +359,74 @@ console.log(values.random);
`Completed running ${inspect(file)}`,
]);
});

it('should pass IPC messages from a spawning parent to the child and back', async () => {
const file = createTmpFile('console.log("running");\nprocess.on("message", (message) => {\n if (message === "exit") {\n process.exit(0);\n } else {\n console.log("Received:", message);\n process.send(message);\n }\n})');

const child = spawn(
execPath,
[
'--watch',
'--no-warnings',
file
],
{
encoding: 'utf8',
stdio: ['pipe', 'pipe', 'pipe', 'ipc'],
}
);

let stderr = '';
let stdout = '';

child.stdout.on("data", data => stdout += data);
child.stderr.on("data", data => stderr += data);
async function waitForEcho(msg) {
const receivedPromise = new Promise((resolve) => {
const fn = (message) => {
if (message === msg) {
child.off("message", fn);
resolve();
}
};
child.on("message", fn);
});
child.send(msg);
await receivedPromise;
}
async function waitForText(text) {
const seenPromise = new Promise((resolve) => {
const fn = (data) => {
if (data.toString().includes(text)) {
resolve();
child.stdout.off("data", fn);
}
}
child.stdout.on("data", fn);
});
await seenPromise;
}

await waitForEcho("first message");
const stopRestarts = restart(file);
await waitForText("running");
stopRestarts();
await waitForEcho("second message");
const exitedPromise = once(child, 'exit');
child.send("exit");
await waitForText("Completed");
child.disconnect();
child.kill();
await exitedPromise;
assert.strictEqual(stderr, '');
const lines = stdout.split(/\r?\n/).filter(Boolean);
assert.deepStrictEqual(lines, [
'running',
'Received: first message',
`Restarting '${file}'`,
'running',
'Received: second message',
`Completed running ${inspect(file)}`,
]);
});
});

0 comments on commit 1c04659

Please sign in to comment.