Rollup config correct?

[ryanflorence]

Just grabbed the UMD build for react-router-dom and its 184kb ... looks like React is being bundled into it?

https://unpkg.com/react-router-dom@4.2.2/umd/react-router-dom.js

I would expect this file do bundle all deps, including react-router and its deps, but not bundle up react.

[pshrmn]

Looks like something happened between 4.2.1 and 4.2.2 because 4.2.1 is 73.51 kB.

[pshrmn]

I walked through the build and while React wasn't included, but there was duplicate code that I was not expecting. Some of it is related to history, which might be fixed with #5589. There was also a lot of duplicate babel code, which I was surprised to see since we are using babel's external helpers.

I'll have to do some more digging to figure out what is going on with that.

[pshrmn]

Alright, so we're getting duplicate code in the react-router-dom UMD build because Rollup is reading from two different node_modules for the react-router and react-router-dom code. For example, the code in packages/react-router imports one copy of prop-types, while code in packages/react-router-dom imports a different copy of prop-types. If we can ensure that the dependencies are all loaded from the same, then this duplication should go away.

[pshrmn]

One final comment for the night.

I made a couple changes to the repo and got the UMD down to ~127 kB. These include making the history import changes made in the PR above, using lerna bootstrap --hoist to make sure that dependencies are available at the root level, and telling Rollup to resolve from the root.

// rollup.config.js
resolve({
  customResolveOptions: {
    moduleDirectory: ['../../node_modules', '../']
  }
}),

We also might need to update the postinstall.js script to ensure that we are hoisting for local installs.

[timdorr]

BTW, 4.2.1 was a bad build, hence 4.2.2. prop-types wasn't included in that build and wasn't imported as a global, so the byte savings came at the cost of it being non-functional 😄

~130k is in line with the previous versions, so it looks like the hoisting should fix this up.