Tree Shaking?

[deehuey]

Is this a bug report?

Maybe?

Can you also reproduce the problem with npm 4.x?

(Write your answer here.)

Which terms did you search for in User Guide?

(Write your answer here if relevant.)

Environment

1. node -v: 6.9.5
2. npm -v: 5.0.3
3. yarn --version (if you use Yarn):
4. npm ls react-scripts (if you haven’t ejected): 1.0.10

Sorry about removing the template, I figure this will be a pretty quick yes or no answer.

    "react-scripts": "^1.0.7"

So I've read around a lot that webpack 2 supports tree shaking out of the box. Cool! Also, I know that material-ui is written such that it SHOULD tree shake. I'm also assuming react-scripts v1.0.7 is built on webpack 2? Anyways:

import { AppBar } from 'material-ui';

When I run the analyzer you guys suggested adding my output is as follows:

Should this not be getting rid of unused components? I've googled around a lot trying to figure this one out.

Thanks!
-Dan

[gaearon]

It is using Webpack 2. Perhaps you can find someone who maintains material-ui to confirm whether it’s supposed to tree shake, and ask them to reply in this thread? I don’t know about anything that would prevent it from working on our side.

[oliviertassinari]

We have the following in our package.json, I would expect the tree shaking to work. Alternatively, we are suggesting some babel helpers as a workaround on the v1-alpha branch.

{
  "name": "material-ui",
  "author": "Material-UI Team",
  "version": "0.18.6",
  "description": "React Components that Implement Google's Material Design.",
  "main": "./index.js",
  "module": "./index.es.js",
  "jsnext:main": "./index.es.js",

I have always heard that tree shaking is unreliable as the implementations try to be as safe as possible. I would love to know what's the story with the alpha release. I would rather invest effort on that branch.
I will try to find time to look into it.

[oliviertassinari]

I was able to reproduce the same issue with the @latest and @next npm tag of Material-UI.
As anyone an example of a library that is supporting tree shaking correctly?

Just to make sure, I have tried with react-router-dom and I have a similar behavior.
I'm importing Link.js

import { Link } from 'react-router-dom'

But end up with everything touching Link.js while those modules aren't imported inside it.

[oliviertassinari]

I'm giving up, after 30 min digging into it, I have no clue around what's wrong. I have tried all I could to be as close as the more or less working react-router-dom project with no success.

[oliviertassinari]

I wish we had a safe/unsafe tree shaking mode as well as a debugger. @deehuey Use a direct import or one of the Babel plugins I have linked.

[Andrew1431]

Thanks for poking around with it. Since it is working with react-router-dom I too will poke around with it when I get some free time to see the major differences between the two. I'll report back any discoveries here!

[vincentbel]

I think it is the issue of webpack(webpack/webpack#2867).

[gaearon]

Just to be clear, tree shaking is not a magic thing. It works only if Uglify determines it is safe to omit those declarations in the final bundle. If they look side-effecty (even for example applying mixin to a class), it won’t be safe. So look at the bundles, and search for side-effecty things. Even class expression generated by Babel might be.

[pshrmn]

I believe that part of the tree shaking issues with the current version of React Router is related to the re-exports that we use (react-router-dom re-exports everything from react-router) remix-run/react-router#5095.

Another issue that appears to exist with tree shaking React components is the way that babel compiles class <C> extends React.Component. The current implementation cannot be (safely) tree shaken. I have opened up this issue babel/babel#5983 in hopes of determining the correct steps to getting a side effect free way of compiling classes.

[gaearon]

Thanks for investigating. This sounds very plausible.

[avocadowastaken]

This webpack/webpack#5435 PR should help to eliminate more dead code by optimizing re-exports for most of react packages.

Or if you can't wait - you can use babel plugin:

babel-transform-imports
babel-plugin-import
babel-plugin-direct-import

[jliebrand]

Is there any update on this? We too are seeing a larger bundle than expected, and are wondering if there are any updates on the horizon that will improve things?
(Would prefer not to have to eject and move to rollup if we can avoid it!)

[oliviertassinari]

@jliebrand We have been doing one iteration on Material-UI side to help with the issue. I'm also assuming that latest babel@7-beta with the /* PURE */ comment for class generation is going to help.

[gaearon]

@jliebrand Most libraries provide separate entry points for individual components. Have you tried using those? Seems like it would be more reliable than tree shaking anyway.

[jliebrand]

Thanks @oliviertassinari; we've actually noticed this behaviour in other modules than material-ui as well, which is why I thought I'd ask.

@gaearon yeah I saw that, but that just felt wrong to me.

import { SomeModule } from 'some-bag';

should just pull in that module and nothing else IMHO. Having to do this:

import SomeModule from 'some-bag/lib/some/dir/some-module';

means I have to know where that module lives inside some-bag; which really should just be implementation detail and breaks encapsulation IMHO.

But for now, I s'pose that will allow us to work around these things. In the end though, if rollup can do it, surely CRA and webpack should be able to do it ?

[gaearon]

means I have to know where that module lives inside some-bag; which really should just be implementation detail and breaks encapsulation IMHO.

It only breaks encapsulation if package authors didn’t anticipate this use case. If they, however, provide a Babel plugin to convert imports to this form, they are intentionally supporting it. Therefore you might as well use it directly.

In the end though, if rollup can do it, surely CRA and webpack should be able to do it ?

If webpack is not doing it, please file an issue with webpack.

[gaearon]

I'll close this. Webpack will get better support for tree shaking in future releases with opt-in purity declarations for packages.

[eluchsinger]

What's happened to this?

[pshrmn]

@eluchsinger For Babel transforms, babel/babel#6963 is the PR to watch. With that, class components will be transformed into pure functions that are easy to tree shake (at which point it will be up to library authors to release packages with pure components).