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

fix: make babel-node spawned process bubble msg #13037

Merged
merged 2 commits into from Mar 23, 2021
Merged

fix: make babel-node spawned process bubble msg #13037

merged 2 commits into from Mar 23, 2021

Conversation

lambertkevin
Copy link
Contributor

Q                       A
Fixed Issues? Fix #4554
Patch: Bug Fix? Yes
Tests Added + Pass? No
Any Dependency Changes? No
License MIT

Just linked the subprocess messages events to the process.send in order to bubble the event and make it listenable when spawning a subprocess with babel-node.

@lambertkevin
Copy link
Contributor Author

lambertkevin commented Mar 22, 2021

Couldn't really find out the process to create the test yet. But this would make it successful:

parent.js

const { spawn } = require('child_process');

const child = spawn('babel-node', ['child.js'], {
  stdio: ['inherit', 'inherit', 'inherit', 'ipc']
});

child.on('message', (msg) => {
   expect(msg).to.be.equal('hello');
   done();
});

child.js

process.send('hello');

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 22, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2562c01:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@nicolo-ribaudo nicolo-ribaudo added pkg: node PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Mar 22, 2021
@nicolo-ribaudo
Copy link
Member

Thanks! I can try working on a test based on your code.

@babel-bot
Copy link
Collaborator

babel-bot commented Mar 22, 2021

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/44576/

@lambertkevin
Copy link
Contributor Author

lambertkevin commented Mar 22, 2021

@nicolo-ribaudo I squashed the typo and a linting error. I might need some help to understand the Publish to local Verdaccio registry (pull_request)test that fails, 'cause I've no idea what it is.

EDIT: Ok nvm, looks like it passed this time 🤷‍♂️

@nicolo-ribaudo
Copy link
Member

Test added! It's indeed failing without your change.

For reviewers: please review my commit separately, since one only renames files but git doesn't properly track the rename when viewing the commit together.

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@lambertkevin
Copy link
Contributor Author

Test added! It's indeed failing without your change.

For reviewers: please review my commit separately, since one only renames files but git doesn't properly track the rename when viewing the commit together.

Thanks for your help with the tests @nicolo-ribaudo 👍

@nicolo-ribaudo
Copy link
Member

Ok it looks like I have to increase the timeout


afterEach(() => {
process.chdir(cwd);
const child = spawn(binLoc, [childLoc], {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shebang in bin/babel-node may not work on Windows, consider invoke babel-node via node.

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo and others added 2 commits March 23, 2021 14:00
* Add ipc test

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
@nicolo-ribaudo nicolo-ribaudo merged commit b360d8d into babel:main Mar 23, 2021
nicolo-ribaudo added a commit that referenced this pull request Mar 23, 2021
@lambertkevin
Copy link
Contributor Author

Thanks @nicolo-ribaudo @JLHwung

@lambertkevin lambertkevin deleted the bugfix/make-subprocess-event-propagate branch March 23, 2021 13:39
@nicolo-ribaudo
Copy link
Member

Thanks for the fix!

@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 23, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: node PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

process.send is not a function in child spawned with babel-node (ipc doesn't work)
4 participants