facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.52k stars 26.78k forks source link

Start script not listening to stdin #11255

Open razwanizmi opened 3 years ago

razwanizmi commented 3 years ago

Describe the bug

I use Create React App as part of an asset pipeline in a web framework (in my case Phoenix). As part of the development pipeline, I've set up a watcher to run react-scripts start whenever my web server starts.

This works fine, but whenever I terminate my web server, Webpack dev server remains running in the background. I want Webpack dev server to listen on stdin for SIGINT and SIGTERM so it can terminate itself.

I noticed in start.js that there is an event listener set up for SIGINT and SIGTERM, but this didn't work for me.

Referring to this documentation, I can fix the issue on my machine by adding this line;

process.stdin.resume()

before any of the process event handlers are added. I think process will exit prematurely and not listen to stdin without that.

If this works for everyone, I'd be happy to put in a PR.

Did you try recovering your dependencies?

Yes.

Which terms did you search for in User Guide?

Start, listen, stdin, sigint, and sigterm.

Environment

  System:
    OS: macOS 10.15.7
    CPU: (4) x64 Intel(R) Core(TM) i5-5250U CPU @ 1.60GHz
  Binaries:
    Node: 15.8.0 - ~/.asdf/installs/nodejs/15.8.0/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 7.5.4 - ~/.asdf/installs/nodejs/15.8.0/.npm/bin/npm
  Browsers:
    Chrome: 92.0.4515.107
    Edge: Not Found
    Firefox: 83.0
    Safari: 14.1.1
  npmPackages:
    react: ^17.0.2 => 17.0.2
    react-dom: ^17.0.2 => 17.0.2
    react-scripts: 4.0.3 => 4.0.3
  npmGlobalPackages:
    create-react-app: Not Found

Steps to reproduce

Described above.

Expected behavior

react-scripts start should listen to stdin for SIGINT and SIGTERM and terminate itself.

Actual behavior

Webpack dev server keeps running in the background after its spawning process gets terminated.

screenshot

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

jonatanklosko commented 1 year ago

Hey, the original fix from #7203 did work, however process.stdin.resume() has been reverted in #8700, and consequently broke it again.

The simplest way to reproduce the issue is to create a new CRA app and run:

node_modules/react-scripts/bin/react-scripts.js start <&-

Note: <&- immediately closes stdin of the script.

jonatanklosko commented 1 year ago

A similar story happened in rollup. It watched stdin, then process.stdin.resume() was removed in https://github.com/rollup/rollup/pull/2410, but then added back in https://github.com/rollup/rollup/pull/3493.

See the commit description in https://github.com/rollup/rollup/commit/57bb54ea26bef7eab062925364158a4c478062f3 for more details.

I think we should bring process.stdin.resume() back.