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

Handle stdin closing #1753

Closed
wmonk opened this issue Mar 7, 2017 · 9 comments · Fixed by #2246
Closed

Handle stdin closing #1753

wmonk opened this issue Mar 7, 2017 · 9 comments · Fixed by #2246

Comments

@wmonk
Copy link

wmonk commented Mar 7, 2017

Description

When running react-script from something like Phoenix, when you stop the phoenix process, the react-script process hangs around in the background. Brunch and webpack have both implemented this.

Expected behavior

The process is stopped when stdin is closed.

Actual behavior

The process does not stop.

@gaearon
Copy link
Contributor

gaearon commented Mar 7, 2017

Do you want to send a PR?

@gaearon gaearon added this to the 0.9.5 milestone Mar 7, 2017
@gaearon gaearon modified the milestones: 0.9.5, 0.9.6 Mar 9, 2017
@SpaceK33z
Copy link
Contributor

Not sure if it helps anyone, but we recently fixed the same issue in webpack-dev-server. See here. I imagine the fix in CRA would look a lot like this.

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

Hopefully this is fixed by #2246.
Let me know if you see this again with react-scripts >= 1.0.1.

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

react-scripts@1.0.1 should be out with a fix.
Can you please verify it helps?

@notnarb
Copy link

notnarb commented Sep 6, 2017

I'm not OP but as far as I can tell #2246 does not solve this issue. This issue is about adding an option to create-react-app to terminate when stdin closes, not gracefully handling SIGINT and SIGTERM. For better or worse, listening for stdin closing is how Phoenix signals to its watcher processes that they should terminate


The relevant code for webpack is here:

https://github.com/webpack/webpack/blob/2bbbf508439728a84c33880bfb6ac547f0aa2a15/bin/webpack.js#L374-L378

if(watchOptions.stdin) {
	process.stdin.on("end", function() {
		process.exit(); // eslint-disable-line
	});
	process.stdin.resume();
}

webpack/webpack-dev-server has an example for specifically this functionality:

https://github.com/webpack/webpack-dev-server/tree/master/examples/cli-stdin

node ../../bin/webpack-dev-server.js --stdin
When stdin ends, we want to close the webpack-dev-server.


I see two solutions:

  1. Modify create-react-app to pass the --stdin flag to webpack (or any flag? does that feature already exist?)
  2. Copy the code from webpack and incorporate it into create-react-app

Happy to make a PR if desired

@1player
Copy link

1player commented Sep 16, 2017

Manually fixed in an ejected app by editing scripts/start.js and changing the end of the main block so that it reads:

   // [snip]
    ["SIGINT", "SIGTERM"].forEach(function(sig) {
      process.on(sig, function() {
        devServer.close();
        process.exit();
      });
    });

   // ADDED BY ME
    process.stdin.on("end", function() {
      devServer.close();
      process.exit();
    });
    process.stdin.resume();

@gaearon Can we please reopen this? It's not fixed at this time.

@Timer
Copy link
Contributor

Timer commented Sep 16, 2017

Could you please send a pull request?

@jakehasler
Copy link

@1player @Timer I just submitted a PR with the proposed (and working) change here.

#3430

@connorjacobsen
Copy link

connorjacobsen commented Aug 17, 2018

This is still broken. The above PR is still open.

@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
8 participants