Changes the start script to tap on the done and failed hooks of the c…

[mAiNiNfEcTiOn]

What does this PR do:

• Changes the start script to tap on the done and failed hooks of the compiler to know the right time to run.

Where should the reviewer start:

• The packages/react-scripts/scripts/start.js was modified, more specifically the devServer.listen() callback.

What is this trying to solve:

• When doing yarn start the server boots up, opens the browser, but renders a blank page (with the <script> tags properly injected.
• After getting that, tried to reproduce the same behaviour with yarn build and serve -s build, which I was unable to. That led me to think that the problem was between the dev server or bundle compilation
• Next step, I edited the node_modules/react-scripts/scripts/start.js, in the same area I've changed for this PR, but placing only a setTimeout(() => openBrowser(urls.localUrlForBrowser), 5000); ... That solved it - but looked hackish... Led to the obvious question of "What if bundling would take more than X seconds set in the timeout? Not cool"...
  • So I changed the implementation to call on done or failed hooks on the compiler.
  • Last but not the least, I created a function that returns a closure controlling if the browser is already open. I'm totally fine with changing it to make it more readable, or even removing it totally if openBrowser already checks that.

Let me know your thoughts on this and how can I improve it, or if it's something that I shouldn't be doing. I'm all for learning more about it 👍

[Timer]

Hmm, this sounds like there's a bug in the webpack-dev-server. We probably want to fix that there instead of implementing a work around here, as this has always worked on v1.

[mAiNiNfEcTiOn]

@Timer actually, the 'compiled successfully' comes from react-dev-utils: https://github.com/facebook/create-react-app/blob/master/packages/react-dev-utils/WebpackDevServerUtils.js#L145 and it's tap'ing the done event.

Also, the responsibility to open the browser is from the start.js in react-scripts: https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/scripts/start.js#L115

[mAiNiNfEcTiOn]

Hmmm, another way could be to make (in the webpack-dev-server) to run the callback only after the 'done' of the compiler... Maybe, as you say, it was like that before... gonna check 👍

UPDATE: Maybe related to this PR: webpack/webpack-dev-server#1331 ?

[Timer]

Yeah, basically in v1 we could open the URL as soon as the app was listening on the port.
Based on this bug report, it sounds like sometimes webpack-dev-server is smart enough to display the content when done completes, other times it isn't.

The reason I suspect this is an issue on their end is because we haven't touched this code much. I'd be more than happy if you can prove this true or false.

I'd argue we definitely want the old behavior, i.e. working without delaying the browser open.

[mAiNiNfEcTiOn]

AFAICT, this was caused since v3.0.0-beta.1 (adding support for webpack 4 and removing webpack-serve's features).

I downgraded to 3.0.0-beta.1 (lowest version compatible with webpack 4) and it still had the issue.

However it seems it's possible to configure the old behaviour by changing the dev server's config: webpack/webpack-dev-server#1509 (comment)

Nope, misread. Someone was pointing out it was possible with webpack-serve before ... sorry -.-'

[mAiNiNfEcTiOn]

Well, it might be that it's only me, so I'm closing this as I can't figure out what causes it...

In case you're able to reproduce it, feel free to re-open it 😄 (I'll keep the branch, just in case)