Reorganize webpack configuration structure #7450

TheLarkInn · 2019-07-29T19:33:21Z

This PR Kicks off the process of reorganizing the current webpack configuration for CRA. We at webpack want to give first time webpack user's the best possible experience. This demographic includes developers who would eject CRA to experiment, learn, extend possibly for the first time.

We think the current config format has a lot of opportunities for organization. That being said:

Changes in this PR

Create development and production config containing their environment specific configuration details.
Separate module from the base webpack config and isolate to a separate file for manageability.
Add webpack-merge as a dependency to safely compose prod and dev configs with base config
Remove some defaulted configuration values already set for specific env's.
Add webpack.ProgressPlugin() for development environment
Remove chunks:'all' optimization which is only relevant for Multipage App Architectures
Any time a CRA Issue was mentioned in comments, I removed prepended commentary (for the sake of readability.)

facebook-github-bot · 2019-07-29T19:33:27Z

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 up 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 the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

TheLarkInn · 2019-07-29T19:40:49Z

packages/react-dev-utils/README.md

@@ -412,5 +412,5 @@ Returns a cache identifier (string) consisting of the specified environment and
 ```js
 var getCacheIdentifier = require('react-dev-utils/getCacheIdentifier');

-getCacheIdentifier('prod', ['react-dev-utils', 'chalk']); // # => 'prod:[email protected]:[email protected]'
+getCacheIdentifier('production', ['react-dev-utils', 'chalk']); // # => 'prod:[email protected]:[email protected]'


I updated this because current usage of this function inside of webpack config(s) was "development" or "production".

Don't forget to update the comment too.

petetnt · 2019-07-29T21:26:05Z

packages/react-scripts/config/webpack.development.js

+          }
+        )
+      ),
+      new webpack.ProgressPlugin(),


There's been lots of discussion in the past about different progress plugin issues (inaccuracy, slowness) (eg. back in #1011, last time in #6310 (comment)). How's the current state of the webpack.ProgressPlugin compared to those old solutions?

I don't know what versions of webpack these comments where in context to, however we have made a few enhancements and fixes over webpack 2 and 3. cc @sokra for his comments.

The lastest version has some fixes and ProgressPlugin should have very small impact.

see also webpack/webpack#9466

iansu · 2019-07-29T23:12:05Z

packages/react-dev-utils/README.md

@@ -412,5 +412,5 @@ Returns a cache identifier (string) consisting of the specified environment and
 ```js
 var getCacheIdentifier = require('react-dev-utils/getCacheIdentifier');

-getCacheIdentifier('prod', ['react-dev-utils', 'chalk']); // # => 'prod:[email protected]:[email protected]'
+getCacheIdentifier('production', ['react-dev-utils', 'chalk']); // # => 'prod:[email protected]:[email protected]'


Don't forget to update the comment too.

iansu · 2019-07-29T23:12:44Z

packages/react-scripts/config/webpack.development.js

+    },
+    plugins: [
+      new HtmlWebpackPlugin(
+        Object.assign(


Is this Object.assign necessary?

If they are separated now I don't think so. Looks like it was more necessary when they were combined with the ternary.

iansu · 2019-07-29T23:15:47Z

packages/react-scripts/config/webpack.config.js

+      mode,
+      // https://webpack.js.org/configuartion/output
+      output: {
+        // TODO: remove this when upgrading to webpack 5


This comment applies to the futureEmitAssets flag on the next line.

robertvansteen · 2019-07-30T06:02:43Z

packages/react-scripts/config/webpack.development.js

+      // This is necessary to emit hot updates (currently CSS only):
+      new webpack.HotModuleReplacementPlugin(),
+      // See https://github.com/facebook/create-react-app/issues/240
+      new CaseSensitivePathsPlugin(),


This looks like a duplicate (see line 51)

ianschmitz · 2019-07-31T02:52:31Z

packages/react-scripts/config/webpack.development.js

+      // See https://github.com/facebook/create-react-app/issues/240
+      new CaseSensitivePathsPlugin(),
+      // See https://github.com/facebook/create-react-app/issues/186
+      new WatchMissingNodeModulesPlugin(paths.appNodeModules),


Dupe here as well

ianschmitz · 2019-07-31T02:58:51Z

Do you mind signing the CLA @TheLarkInn? :)

mrmckeb · 2019-08-01T08:18:22Z

This is amazing, thanks @TheLarkInn.

sokra · 2019-08-05T19:37:13Z

packages/react-scripts/config/webpack.config.js

+      // https://webpack.js.org/configuartion/optimization
+      optimization: {
+        splitChunks: {
+          name: false,


chunks: "all" should be kept. It allows to vendor split the main chunk

sokra · 2019-08-05T19:41:20Z

packages/react-scripts/config/webpack.development.js

+// We are in the development config. This contains development only features.
+module.exports = function() {
+  return {
+    devtool: 'cheap-module-source-map',


eval-cheap-module-source-map would be faster

We had an issue when we switched to eval-source-map due to mismatch line numbers in react-error-overlay. I don't remember whether I tried eval-cheap-module-source-map.
See webpack/webpack#8910

sokra · 2019-08-05T19:46:42Z

packages/react-scripts/config/webpack.development.js

+          }
+        )
+      ),
+      new webpack.ProgressPlugin(),


The lastest version has some fixes and ProgressPlugin should have very small impact.

see also webpack/webpack#9466

sokra · 2019-08-05T19:47:05Z

packages/react-scripts/config/webpack.development.js

+      // See https://github.com/facebook/create-react-app/issues/186
+      new WatchMissingNodeModulesPlugin(paths.appNodeModules),
+      // This is necessary to emit hot updates (currently CSS only):
+      new webpack.HotModuleReplacementPlugin(),


sokra · 2019-08-05T19:53:05Z

packages/react-scripts/config/webpack.rules.js

+        // By default we support CSS Modules with the extension .module.css
+        {
+          test: cssRegex,
+          exclude: cssModuleRegex,


This exclude could be omitted when this rule is switched with the next rule.

sokra · 2019-08-05T19:53:38Z

packages/react-scripts/config/webpack.rules.js

+        // extensions .module.scss or .module.sass
+        {
+          test: sassRegex,
+          exclude: sassModuleRegex,


Same. Switch with the next rule.

mrmckeb · 2019-10-16T17:19:23Z

@TheLarkInn, we'd love to get this in - we're thinking 3.3.

Are you able to take a look at the suggestions/requested changes?

yordis · 2019-10-16T17:24:03Z

@mrmckeb #7638 ...

At this point, I gave up fixing the merge conflict since it happens in the entire file at once.

Unless you want @TheLarkInn to be the person moving forward with this, I can fix the merge conflict and continue with the task.

I fixed all the requested changes.

TheLarkInn requested review from bugzpodder, ianschmitz, iansu, mrmckeb and petetnt as code owners July 29, 2019 19:33

TheLarkInn commented Jul 29, 2019

View reviewed changes

Reorganize webpack configuration structure

94f91ab

TheLarkInn force-pushed the feature/organize-config branch from 58c1828 to 94f91ab Compare July 29, 2019 19:45

Merge branch 'master' into feature/organize-config

5ff14ae

TheLarkInn force-pushed the feature/organize-config branch from f626d4d to 5ff14ae Compare July 29, 2019 19:53

bugzpodder added this to the 3.2 milestone Jul 29, 2019

petetnt reviewed Jul 29, 2019

View reviewed changes

iansu reviewed Jul 29, 2019

View reviewed changes

robertvansteen reviewed Jul 30, 2019

View reviewed changes

ianschmitz reviewed Jul 31, 2019

View reviewed changes

sokra reviewed Aug 5, 2019

View reviewed changes

yordis mentioned this pull request Sep 4, 2019

feat: update Webpack config #7638

Closed

ianschmitz modified the milestones: 3.3, 3.4 Dec 5, 2019

iansu modified the milestones: 3.4, 3.5 Feb 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize webpack configuration structure #7450

Reorganize webpack configuration structure #7450

TheLarkInn commented Jul 29, 2019 •

edited

Loading

facebook-github-bot commented Jul 29, 2019

TheLarkInn Jul 29, 2019

iansu Jul 29, 2019

petetnt Jul 29, 2019

TheLarkInn Jul 29, 2019

sokra Aug 5, 2019

iansu Jul 29, 2019

iansu Jul 29, 2019

TheLarkInn Jul 30, 2019

iansu Jul 29, 2019

robertvansteen Jul 30, 2019

ianschmitz Jul 31, 2019

ianschmitz commented Jul 31, 2019

mrmckeb commented Aug 1, 2019

sokra Aug 5, 2019

sokra Aug 5, 2019

bugzpodder Aug 5, 2019

sokra Aug 5, 2019

sokra Aug 5, 2019

sokra Aug 5, 2019

sokra Aug 5, 2019

mrmckeb commented Oct 16, 2019

yordis commented Oct 16, 2019

Reorganize webpack configuration structure #7450

Are you sure you want to change the base?

Reorganize webpack configuration structure #7450

Conversation

TheLarkInn commented Jul 29, 2019 • edited Loading

Changes in this PR

facebook-github-bot commented Jul 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianschmitz commented Jul 31, 2019

mrmckeb commented Aug 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrmckeb commented Oct 16, 2019

yordis commented Oct 16, 2019

TheLarkInn commented Jul 29, 2019 •

edited

Loading