Bundle size is bigger than necessary due to ES6 imports

[ruiaraujo]

I am using react-router-dom and webpack 2 and it seems the tree shaking does not work well enough to remove unused modules.

I managed to fix it locally by changing the import style from

import { Router } from 'react-router' 

to

import Router from 'react-router/Router' 

everywhere.

Version

4.0.0

Is this something that would be interesting to do a PR on?

[pshrmn]

I don't know enough about tree shaking to give any real input on this, but I will mention that this is related to #4432.

[ruiaraujo]

The ES build with webpack 2.3 does not solve the issue. Only changing all the imports improved the situation since any import to react-router will bring in all the modules.

[timdorr]

Are you sure you're using the ES build? It leaves import/exports untouched, so Webpack should be able to tree-shake it.

I'm thinking this is either some misconfiguration of webpack or babel on your end. Unfortunately, it's easy to mess up because Webpack doesn't make it obvious when you're actually using ES modules or not.

[ruiaraujo]

Yes I am sure, see the attached image. The StaticRouter, Prompt and createMemoryHistory should be removed they stay there.

When I check the reason using the webpack stats analyser, the reason for those files being included is the index file of react-router.

[ruiaraujo]

These issues might be relevant.

webpack/webpack#4080
mishoo/UglifyJS#1261

Also the image above was with using a babel plugin (babel-plugin-transform-imports) to do direct import otherwise the bundle looks like this. Nothing gets removed.

[timdorr]

But are you sure it's using the es build that's included in the npm package? That's the key thing. What if you alias react-router to react-router/es?

[ruiaraujo]

I am sure (notice the 'es' folder above in the screenshots) and have tried the alias with the same result.

So I am pretty sure that the build is pointing to the ES build.

My question is whether a PR would be accepted with a change to fix this problem.

[timdorr]

Sure, but what would be the appropriate change? AFAIK, we're doing the same thing on Redux and it works fine there.

[ruiaraujo]

The change would the react router dom export refer to direct import instead of importing the top level of react-router.

I have tried this locally and it did fixed it.

[chrisdl]

Would fixing this issue solve this you think? @ruiaraujo.

I've recently upgraded to webpack 3 and am trying to optimize my bundle with the new webpack.optimize.ModuleConcatenationPlugin().

# In my build command.
[35] ../node_modules/react-router-dom/es/index.js + 23 modules 41.6 kB {0} [built]
       ModuleConcatenation (inner): module is used with non-harmony imports from ...[list of my components importing { STUFF } from 'react-router'dom']

[ruiaraujo]

@chrisdl That is odd. You can try that branch in your local project and report back.

[chrisdl]

@ruiaraujo im actually having some issues installing the PR locally. Is there a better way than using to your knowledge?

yarn add ReactTraining/react-router#5095/head

[comerc]

Do you know about babel-plugin-import?