use MiniCssExtractPlugin with hmr in dev mode

[vatz88]

Remove style-loader and use MiniCssExtractPlugin for both development and production environment.

I believe one of the main reasons to use style-loader in development mode is to support hot module reloading since styles are in js files. But MiniCssExtractPlugin which is used for production build does support HMR feature since v0.6.0 (2019-04-10).

Discussion on spectrum https://spectrum.chat/create-react-app/general/why-react-scripts-webpack-config-uses-style-loader-in-dev-mode-and-not-minicssextractplugin-with-hmr~f966f7b0-0b42-4918-b0be-79e590ed8cdb

[facebook-github-bot]

Hi vatz88! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[andriijas]

Great find. In my mind it's always better if dev and prod are as similar as possible. Do you have any idea if performance is affected?

[vatz88]

@andriijas I haven't observed any difference in performance.

About the tests which are failing. I see the test code includes doc.getElementsByTagName('style')[i].textContent.replace(/\s/g, ''). What I think is, not sure though, since MiniCssExtractPlugin will put styles in a file unlike style-loader, textContent on the style tag will not be available. Will need some help if I have to update the test cases.

[elektronik2k5]

Is isEnvDevelopment && necessary?

[vatz88]

Not really, just following the pattern how it's written elsewhere in the code. For example https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/config/webpack.config.js#L194

[elektronik2k5]

Thanks for the reference - I hadn't noticed that. Consistency is indeed important, but I can't help but wonder about the purpose of these checks 🤔

[stale]

This pull request 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.

[elektronik2k5]

This pull request 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.

Bad robot!

[stale]

This pull request 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.

[elektronik2k5]

@vatz88 can you please update your PR?
Can anyone from the core team have a look?

[vatz88]

About the tests which are failing. I see the test code includes doc.getElementsByTagName('style')[i].textContent.replace(/\s/g, ''). What I think is, not sure though, since MiniCssExtractPlugin will put styles in a file unlike style-loader, textContent on the style tag will not be available. Will need some help if I have to update the test cases.

Haven't really gotten time to understand and fix the tests. Will close this PR for now. Though I'd appreciate any help regarding this same.

[andriijas]

@vatz88 sorry for not getting back in time. Could you rebase your changes on top of the latest and I will take a look at the tests? thanks

[vatz88]

@andriijas Thanks, updated the branch.

[andriijas]

Thanks, I will check this later in the week! I have been in a project with standalone webpack configs where we are running minicssextractplugin in dev with hmr working fine for a while so I think this should be the way to go. I will check the tests!

[chybisov]

@andriijas just checked this and it seems not working solution for me, because it refreshes page instead of just replacing css like style-loader did.
#8378

[stale]

This pull request 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.

[elektronik2k5]

Not stale. @vatz88 can you please resolve the conflict and look into what @chybisov wrote?

[stale]

This pull request 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.

[andriijas]

As mentioned in #8378 proper hmr wont be possible in either style-loader or minicssextract plugin until webpack 5 is stable.

[vatz88]

I guess it's good to close this PR for now and be worked on when we have stable release of webpack v5.