fix: ignore all unrelated messages from child process

[LinusU]

Summary

This is a small part of #13521 that I felt would be easier to review and merge on its own. It makes sure that any other part of the code that also uses IPC (e.g. a Node.js loader) doesn't interfere and crash the Jest process.

Test plan

I've tested this by running yarn build and manually coping the changed files into another small test project 🙈

[LinusU]

rebased on master

ping @SimenB @privatenumber, is there anything that I can do in order to move this forward?

[SimenB]

rebased on master

ping @SimenB @privatenumber, is there anything that I can do in order to move this forward?

Sorry, just swamped at work atm 🙂

---

We should mirror this functionality in the worker_threads implementation

[LinusU]

Sorry, just swamped at work atm 🙂

No worries at all!

We should mirror this functionality in the worker_threads implementation

Will do 👍

[LinusU]

Rebased on latest main, and mirrored the changes for the worker_threads implementation 🐎

[jeysal]

I don't have a whole lot of context on the whole ESM project that @SimenB has almost entirely been handling, but being lenient on this seems dangerous to me. Unless every tool and library out there starts namespacing their IPC do be clearly distinguishable (almost like making it a standard protocol), there's risk of catching and misinterpreting others' messages. I think that the code that spawns the child (which is jest-worker) owning the IPC communication is reasonable. However, if there is some standard for namespacing Node IPC communication (I don't know), it could make sense for us to follow that starting from a next jest-worker major version, freeing up the rest of the channel. I'll go comment on the other issue as well

[privatenumber]

To prevent collision, jest should add some sort of integrity check.

For example, prefixing the message with the name and version properties from package.json would probably be unique enough. For something super secure, generating a UUID in the parent, spawning the worker with the UUID as an argument, and prefixing the messages with it.

[jeysal]

Yes that would be a way of namespacing the IPC. I wouldn't be opposed to changing jest-worker to do this, but I'm still wondering if it's necessary because I haven't seen something like this in action anywhere. And it seems to me like a reasonable assumption that the code spawning a process gets to interact with it via IPC and other code that had nothing to do with spawning the child should not touch the IPC channel.

Like, can you point to examples of other ecosystem tools or libraries namespacing IPC in this way? Or to examples of libraries doing child-parent communication even though they were not responsible for spawning a child?

I'm just not sure that it's actually Jest using an unclean approach here, versus Jest keeping it simple as everyone does and esm-loader assuming access to an IPC that it does not own and should not touch.

Also cc @SimenB in case you know whether it is convention for libs to assume they can use the IPC channel

[privatenumber]

I don't think a convention is necessary to move forward.

esm-loader assuming access to an IPC that it does not own and should not touch.

It's not owned by jest either. The parent node process owns the IPC. In this case, jest and esm-loader are executed in the same node process so it's shared.

[SimenB]

works for me 👍

[SimenB]

https://github.com/facebook/jest/releases/tag/v29.4.2

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.