Handle process stdin #3323
Handle process stdin #3323
Conversation
|
Great, thank you! This doesn't seem to play well with our CI; could you please look into why and fixing that? |
|
To be honest it looks to me like this is a pre-existing condition which is shared between many of the latest pull request. Even on I will look into it to see if I can untangle it, but I doubt it has anything to do with the changes introduced here. I will let you know! |
|
Looking more attentively I could replicate the issue on my machine and I will now address it! But the problem I have is the one on Travis and not the one on appveyor... |
Kjir
force-pushed
the
handle-process-stdin
branch
from
91ec965
to
3dec3cf
Compare
|
After a lot of head scratching, I found out what the problem is thanks to this question: https://unix.stackexchange.com/questions/3886/difference-between-nohup-disown-and What happens is that when bash puts a process as a background job, it will halt that program as soon as it tries to read from STDIN. What this means is that with this patch will make it harder to run |
|
Just to be on the safe side, I also added a note in the Troubleshooting section. |
|
There could be an alternative: this functionality is only enabled through a flag that we would provide to Since this decision is not technical I await further insights and will eventually change my PR to go in this other direction, if deemed preferable. |
|
In all this I forgot about Windows! (Sorry about the numerous comments) |
Kjir
force-pushed
the
handle-process-stdin
branch
from
3dec3cf
to
a728c1e
Compare
The kitchensink e2e tests were failing because in those tests the `npm start` command is sent to the background. The shell will then halt any background job that tries to read from the standard input, and since we are watching standard input to determine if we should exit or not the dev server would immediately exit. The workaround is to provide run the dev-server as `npm start </dev/zero &` instead, which provides a different standard input that will not close immediately. This, alas, doesn't work on Windows and might change the expected behaviour for some people that are used to start the dev-server as a background job. For this reason a --watch-stdin flag has been added to only enable this behaviour on request for the few applicable use cases. A note has also been added in the Troubleshooting section to help any final user that might start the dev-server as a background job.
Kjir
force-pushed
the
handle-process-stdin
branch
from
a728c1e
to
a078545
Compare
|
Anything that needs to be done to get this merged / drive this forward? Let me know if I can help out. |
|
|
||
| When using create-react-app in a Phoenix application, the webpack-dev-server is not closed automatically if it's configured as a watcher. This is because Phoenix expects the watchers to close when the standart input is closed. | ||
|
|
||
| To watch for standart input you can pass the `--watch-stdin` option: |
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. |
|
This pull request has been automatically closed because it has not had any recent activity. The conversation will be locked in 7 days unless the pull request is reopened. Thank you for your contribution. |
This PR continues the work started in #1754 so that it might be merged.
I rebased the changes on top of master (which should fix the test errors, hopefully) and I created the helper in
react-dev-utilsas suggested in the previous code review.I tested that it still works with a very simple python program:
After 10 seconds the program does actually quit, both when watching test and when running the dev server.
I am not sure there is a suitable way of adding a test for this, if I receive some suggestions about this I will gladly implement them!
Fixes #1753