Enable passthrough IPC in watch mode by znewsham · Pull Request #50890 · nodejs/node · GitHub

Fixes #50880

When spawning a node process in watch mode, from another node process, there is currently no way to get IPC between the parent and the child. This PR enables this bi-directional flow:

parent -> watcher -> child
child -> watcher -> parent

This setup is useful in situations where you have a process responsible for building and running a client/server application which needs to be made aware (via IPC) that it's client bundle has changed and needs to be re-served, without restarting the server.

I think this should be directed towards the main branch

I think this should be directed towards the main branch

I confess, I'm not totally clear on the branching structure. I'd like this to be available in node 18, are you suggesting I point at v18.x or somewhere else?

So the general process is we land changes onto the main branch of this repo and then changes are backported to the release lines you can read about it here https://github.com/nodejs/node/blob/main/doc/contributing/backporting-to-release-lines.md
so for your case your branch should towards the main branch and after this change is approved and landed it shall be backported by the releasers to the 18.x branch and be available in 18.x

please also resolve conflicts

This will almost certainly require a backport PR

noting the original commit for my reference later: ad2131716bdd45d4279a9780f50e11e1aa4302ef

Could you fix the first commit message according to the guidelines https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#commit-message-guidelines , and these few linting issues that it complains about.

Both of those are fixed - for some reason I can't get eslint to play nice with vscode - but the linting is now resolved.

Changes LGTM, but I think the test can be simplified more

LGTM

Nice

Just want to check on the next steps here - the contribution guide suggests I should kick off the CI build and add an "Author Ready" label - I don't believe I have permissions to do either

CI: https://ci.nodejs.org/job/node-test-pull-request/56076/

What's the best way to debug failures on specific architectures? E.g.,
node-test-commit-arm-fanned
node-test-commit-linuxone-rhel8-s390x

I could probably get myself access to an arm machine if necessary

@MoLow I apologise, the failing test was caused by a previously fixed test that the rebase must have removed (an extraneous ' in the expected test result). Could you please re-trigger the CI run

Failed to start CI

   ⚠  Something was pushed to the Pull Request branch since the last approving review.
   ✘  Refusing to run CI on potentially unsafe PR

https://github.com/nodejs/node/actions/runs/9014337858

CI: https://ci.nodejs.org/job/node-test-pull-request/59046/

CI: https://ci.nodejs.org/job/node-test-pull-request/59047/

CI: https://ci.nodejs.org/job/node-test-pull-request/59049/

CI: https://ci.nodejs.org/job/node-test-pull-request/59056/

CI: https://ci.nodejs.org/job/node-test-pull-request/59059/

CI: https://ci.nodejs.org/job/node-test-pull-request/59061/

Landed in 70995bd

@MoLow thanks for your help in getting this across the line - I've read through the contribution guide (and the backporting guide), but it still isn't clear to me which release this will appear in (or when) - will it be a minor version increase in v22, or does it have to wait until v23? Is backporting to v20 (or even v18, it's not clear what "maintenance" means) an option?

PR-URL: #50890 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

It should get to all active releases (20, 21, 22)

PR-URL: #50890 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: nodejs#50890 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

nodejs-github-bot added needs-ci PRs that need a full CI run. v18.x labels Nov 24, 2023

znewsham changed the base branch from v18.x-staging to main November 24, 2023 14:56

znewsham force-pushed the znewsham/watch-with-ipc branch from ad21317 to 1c04659 Compare November 24, 2023 14:58

debadree25 added watch-mode Issues and PRs related to watch mode and removed v18.x labels Nov 25, 2023

debadree25 reviewed Nov 25, 2023

View reviewed changes

znewsham force-pushed the znewsham/watch-with-ipc branch 2 times, most recently from 1b82c7f to 2fa9259 Compare November 25, 2023 18:20

MoLow reviewed Nov 25, 2023

View reviewed changes

Comment thread lib/internal/watch_mode/files_watcher.js Outdated

MoLow reviewed Nov 25, 2023

View reviewed changes

Comment thread lib/internal/watch_mode/files_watcher.js Outdated

MoLow reviewed Nov 25, 2023

View reviewed changes

Comment thread test/sequential/test-watch-mode.mjs

MoLow reviewed Nov 26, 2023

View reviewed changes

Comment thread lib/internal/watch_mode/files_watcher.js Outdated

MoLow reviewed Nov 26, 2023

View reviewed changes

MoLow approved these changes Nov 27, 2023

View reviewed changes

MoLow requested a review from benjamingr November 27, 2023 11:50

benjamingr reviewed Nov 27, 2023

View reviewed changes

Comment thread lib/internal/watch_mode/files_watcher.js

benjamingr approved these changes Nov 27, 2023

View reviewed changes

debadree25 added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no outstanding review comments, and a CI started. labels Dec 4, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 4, 2023

watch: remove extraneous quote from failing test

2369dc8

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label May 9, 2024

github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels May 9, 2024

MoLow approved these changes May 9, 2024

View reviewed changes

MoLow added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels May 9, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 9, 2024

MoLow added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels May 9, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 9, 2024

nodejs-github-bot merged commit 70995bd into nodejs:main May 9, 2024

targos pushed a commit that referenced this pull request May 11, 2024

watch: enable passthrough ipc in watch mode

e895f7c

PR-URL: #50890 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

targos mentioned this pull request May 13, 2024

v22.2.0 release proposal #52971

Merged

marco-ippolito mentioned this pull request Jun 17, 2024

v20.15.0 proposal #53486

Merged

marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024

watch: enable passthrough ipc in watch mode

1ff8f31

PR-URL: #50890 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024

watch: enable passthrough ipc in watch mode

044678d

PR-URL: nodejs#50890 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Enable passthrough IPC in watch mode by znewsham · Pull Request #50890 · nodejs/node · GitHub

Uh oh!

Uh oh!

Enable passthrough IPC in watch mode - #50890

Enable passthrough IPC in watch mode#50890nodejs-github-bot merged 9 commits intonodejs:mainnodejs/node:mainfrom znewsham:znewsham/watch-with-ipcznewsham/node:znewsham/watch-with-ipcCopy head branch name to clipboard

Conversation

znewsham commented Nov 24, 2023

Uh oh!

debadree25 commented Nov 24, 2023

Uh oh!

znewsham commented Nov 24, 2023

Uh oh!

debadree25 commented Nov 24, 2023

Uh oh!

MoLow commented Nov 24, 2023

Uh oh!

znewsham commented Nov 24, 2023

Uh oh!

debadree25 left a comment

Choose a reason for hiding this comment

Uh oh!

znewsham commented Nov 25, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MoLow left a comment

Choose a reason for hiding this comment

Uh oh!

MoLow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

benjamingr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

znewsham commented Dec 4, 2023

Uh oh!

nodejs-github-bot commented Dec 4, 2023

Uh oh!

znewsham commented Dec 4, 2023

Uh oh!

znewsham commented May 8, 2024

Uh oh!

github-actions Bot commented May 9, 2024

Uh oh!

nodejs-github-bot commented May 9, 2024

Uh oh!

nodejs-github-bot commented May 9, 2024

Uh oh!

nodejs-github-bot commented May 9, 2024

Uh oh!

nodejs-github-bot commented May 9, 2024

Uh oh!

nodejs-github-bot commented May 9, 2024

Uh oh!

nodejs-github-bot commented May 9, 2024

Uh oh!

nodejs-github-bot commented May 9, 2024

Uh oh!

znewsham commented May 9, 2024

Uh oh!

MoLow commented May 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Enable passthrough IPC in watch mode#50890
nodejs-github-bot merged 9 commits into
nodejs:mainfrom
znewsham:znewsham/watch-with-ipc

benjamingr left a comment •
edited
Loading