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

Minimizing bundle size in CRA (Create React App) #19549

Closed
thexpand opened this issue Feb 3, 2020 · 20 comments · Fixed by #19768
Closed

Minimizing bundle size in CRA (Create React App) #19549

thexpand opened this issue Feb 3, 2020 · 20 comments · Fixed by #19768
Labels
docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@thexpand
Copy link

thexpand commented Feb 3, 2020

I'm using CRA and the icons from material-ui, so I'm using only partial (named) imports like so:

import { Add, Check } from '@material-ui/icons';

I've read the page from the documentation about minimizing bundle size, but the information there about CRA seems to be a bit old. However, I'm not sure if that's 100% true. I tried researching, but found nothing. So, my question is - using CRA and named imports from material-ui, do I need to do anything to enable bundle size minimization?

Given that more and more people are using CRA, even for production, it would be best to add a section in the documentation for bundle size minimization specifically for applications, created with Create React App.

@eps1lon
Copy link
Member

eps1lon commented Feb 3, 2020

but the information there about CRA seems to be a bit old

Which in particular?

do I need to do anything to enable bundle size minimization?

Not that I know of. Which part of the docs lead you to believe you have to do anything? We specifically mention that right from the start:

If you're using ES6 modules and a bundler that supports tree-shaking (webpack >= 2.x, parcel with a flag) you can safely use named imports and expect only a minimal set of Material-UI components in your bundle:

But this requires knowledge about what bundler your framework is using. We can probably mention the specifics later (bundler) and start with saying that tree-shaking works out of the box in modern frameworks.

would be best to add a section in the documentation for bundle size minimization specifically for applications, created with Create React App.

create-react-app is in that regard not much different. It isn't maintainable to document each popular app framework if we can do it once for all.

@eps1lon eps1lon added discussion docs Improvements or additions to the documentation labels Feb 3, 2020
@thexpand
Copy link
Author

thexpand commented Feb 3, 2020

@eps1lon Thanks for digging deep. In order of answering:

but the information there about CRA seems to be a bit old

Which in particular?

and

do I need to do anything to enable bundle size minimization?

Not that I know of. Which part of the docs lead you to believe you have to do anything? We specifically mention that right from the start:

The information is about installing react-app-rewired and customize-cra, which allowed customization of .babelrc for CRA version 2. However, IIRC from CRA version 3, you can have your own .babelrc in the root of the project. Here's the misleading information I was talking about: https://github.com/mui-org/material-ui/blame/master/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md#L149-L151


If you're using ES6 modules and a bundler that supports tree-shaking (webpack >= 2.x, parcel with a flag) you can safely use named imports and expect only a minimal set of Material-UI components in your bundle:

But this requires knowledge about what bundler your framework is using. We can probably mention the specifics later (bundler) and start with saying that tree-shaking works out of the box in modern frameworks.

Yes, indeed. I like your idea of having that information at the top.


would be best to add a section in the documentation for bundle size minimization specifically for applications, created with Create React App.

create-react-app is in that regard not much different. It isn't maintainable to document each popular app framework if we can do it once for all.

As I come to think of it - you are correct, as CRA is using webpack and tree-shaking internally, so nothing to do in order to make it work, as it works out of the box.

@oliviertassinari
Copy link
Member

It sounds like we should be able to improve this page of the documentation :).

@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Feb 4, 2020
@bugzpodder
Copy link
Contributor

bugzpodder commented Feb 8, 2020

However, IIRC from CRA version 3, you can have your own .babelrc in the root of the project.

No.

As I come to think of it - you are correct, as CRA is using webpack and tree-shaking internally, so nothing to do in order to make it work, as it works out of the box.

EDIT: webpack tree shaking work as expected in a stock CRA environment + @material-ui/core and @material-ui/icons as expected with the latest CRA version (3.3.1).

@oliviertassinari
Copy link
Member

@bugzpodder So, should we close the issue?

@bugzpodder
Copy link
Contributor

The docs section on CRA looks correct. You'll need some like react-app-rewired on top of CRA in order to minimize bundle size at least right now. I don't think it needs to be updated.

@eps1lon
Copy link
Member

eps1lon commented Feb 9, 2020

It doesn't actually work last time I checked.

@bugzpodder Please provide a repro when making these claims.

@bugzpodder
Copy link
Contributor

bugzpodder commented Feb 9, 2020

@eps1lon my claim is in line with what your doc describes. You want me to provide a repro to prove your doc is correct?

@eps1lon
Copy link
Member

eps1lon commented Feb 9, 2020

@eps1lon my claim is in line with what your doc describes. You want me to provide a repro to prove your doc is correct?

The docs say it works out of the box. You said

It doesn't actually work last time I checked.

referring to

as it works out of the box.

We should probably change the wording. The current question implies someone needs to take action. Assuming latest software is the default, this isn't required. It's a dev-only optimization we describe.

@thexpand
Copy link
Author

thexpand commented Feb 9, 2020

I'm now more confused than I was before asking the question. Is the bundle size automatically minimized for the production build of CRA? Is it for development?

@oliviertassinari
Copy link
Member

@thexpand The topic here is for development only. You don't have to worry about production.

@thexpand
Copy link
Author

thexpand commented Feb 9, 2020

@oliviertassinari So, what we're discussing is about minimizing for development, and thus reducing the time for hot reloading, by reducing the build time, is that correct?

@bugzpodder
Copy link
Contributor

bugzpodder commented Feb 9, 2020

@eps1lon my mistake.
I reread the doc again (this is probably the tenth time I read it, and each time I always thought the options were for production), and you are right the docs options are for development only. I think people reading the docs doesn't necessarily read every word, they just goto the section they want to see, and miss the one sentence on top.

EDIT: webpack tree shaking work as expected in a stock CRA environment + @material-ui/core and @material-ui/icons as expected with the latest CRA version (3.3.1).

@bugzpodder
Copy link
Contributor

bugzpodder commented Feb 9, 2020

I just tested this on stock CRA and @material-ui and actually tree-shaking work as expected. It does work in production as the docs claim when you use named exports.

I tested this on my own production app but still have issues with tree shaking, so it must be something with my own configuration. That's something I can figure out myself and likely won't affect others.

I went back and edited my comments to remove misleading claims. Apologize for any confusion.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 10, 2020

What do you guys think of this header structure change? Given the current structure, I can very well understand why somebody that would skip-jump when reading could be confused (myself included)!

diff --git a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md
index d5ef2afe53..4d4ef0b994 100644
--- a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md
+++ b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md
@@ -9,7 +9,7 @@ on every commit for every package and critical parts of those packages ([view th
 Combined with [dangerJS](https://danger.systems/js/) we can inspect
 [detailed bundle size changes](https://github.com/mui-org/material-ui/pull/14638#issuecomment-466658459) on every Pull Request.

-## How to reduce the bundle size?
+## How to use tree-shaking?

 For convenience, Material-UI exposes its full API on the top-level `material-ui` import.
 If you're using ES6 modules and a bundler that supports tree-shaking ([`webpack` >= 2.x](https://webpack.js.org/guides/tree-shaking/), [`parcel` with a flag](https://en.parceljs.org/cli.html#enable-experimental-scope-hoisting/tree-shaking-support)) you can safely
@@ -19,7 +19,9 @@ use named imports and expect only a minimal set of Material-UI components in you
 import { Button, TextField } from '@material-ui/core';
 ```

-⚠️ Be aware that tree-shaking is an optimization that is usually only applied to production
+## Development environment
+
+⚠️ Be aware that tree-shaking is an optimization that is usually only applied to **production**
 bundles. Development bundles will contain the full library which can lead to **slower
 startup times**. This is especially noticeable if you import from `@material-ui/icons`.
 Startup times can be approximately 6x slower than without named imports from the top-level API.

@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. and removed discussion ready to take Help wanted. Guidance available. There is a high chance the change will be accepted labels Feb 10, 2020
@larsenwork
Copy link
Contributor

@oliviertassinari FWIW I read the docs the other day and only after quite some time did I realise they were only for development environments and that my CRA setup already shook things as much as possible.

A slightly more radical change than the one above could have "saved" me (maybe?)

diff --git a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md
index d5ef2afe5..34188a1ba 100644
--- a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md
+++ b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md
@@ -9,20 +9,25 @@ on every commit for every package and critical parts of those packages ([view th
 Combined with [dangerJS](https://danger.systems/js/) we can inspect
 [detailed bundle size changes](https://github.com/mui-org/material-ui/pull/14638#issuecomment-466658459) on every Pull Request.
 
-## How to reduce the bundle size?
+## When and how to use tree-shaking?
 
-For convenience, Material-UI exposes its full API on the top-level `material-ui` import.
-If you're using ES6 modules and a bundler that supports tree-shaking ([`webpack` >= 2.x](https://webpack.js.org/guides/tree-shaking/), [`parcel` with a flag](https://en.parceljs.org/cli.html#enable-experimental-scope-hoisting/tree-shaking-support)) you can safely
-use named imports and expect only a minimal set of Material-UI components in your bundle:
+Your Material-UI bundle is most likely already tree-shaken in production. Material UI exposes its full API on the top-level
+`material-ui` import. If you're using ES6 modules and a bundler that supports tree-shaking
+([`webpack` >= 2.x](https://webpack.js.org/guides/tree-shaking/),
+[`parcel` with a flag](https://en.parceljs.org/cli.html#enable-experimental-scope-hoisting/tree-shaking-support)) you can safely
+use named imports and still get an optimised bundle size automatically:
 
 ```js
  import { Button, TextField } from '@material-ui/core';
 ```
 
-⚠️ Be aware that tree-shaking is an optimization that is usually only applied to production
-bundles. Development bundles will contain the full library which can lead to **slower
-startup times**. This is especially noticeable if you import from `@material-ui/icons`.
-Startup times can be approximately 6x slower than without named imports from the top-level API.
+⚠️ The following instructions are only needed if you want to optimize your development startup times or if you are using an older bundler
+that doesn't support tree-shaking.
+
+## Development environment
+
+Development bundles can contain the full library which can lead to **slower startup times**. This is especially noticeable if you
+import from `@material-ui/icons`. Startup times can be approximately 6x slower than without named imports from the top-level API.
 
 If this is an issue for you, you have various options:

@oliviertassinari
Copy link
Member

@larsenwork Sounds great, do you want to open a pull request? :)

@larsenwork
Copy link
Contributor

Sure, I'll draft something (also inspired by comments above).

larsenwork added a commit to larsenwork/material-ui that referenced this issue Feb 18, 2020
@olelivalife
Copy link

The docs section on CRA looks correct. You'll need some like react-app-rewired on top of CRA in order to minimize bundle size at least right now. I don't think it needs to be updated.

Reading this thread leaves me puzzled... Is it still necessary (react-scripts v3.4.3) to use react-app-rewired to support tree shaking? Or it's it available in stock CRA?

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 1, 2020

@olelivalife Ignore this thread, https://material-ui.com/guides/minimizing-bundle-size/ is the source of truth. You should find the answer to your question there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants