nytimes / react-tracking

🎯 Declarative tracking for React apps.
https://open.nytimes.com/introducing-react-tracking-declarative-tracking-for-react-apps-2c76706bb79a
Other
1.88k stars 123 forks source link

Improve bundlesize #100

Closed tizmagik closed 6 years ago

tizmagik commented 6 years ago

The latest update to use Babel 7 ( #94 #98 ) caused our package size to increase almost 6x (5.5.1 vs 5.4.1)

https://bundlephobia.com/result?p=react-tracking@5.5.1

Maybe core-js should instead be a peerDep? Or maybe we need to look into using Rollup or microbundle or something.

cc @damassi

damassi commented 6 years ago

This is an interesting issue in that

a) most people that use babel will have core-js within their dependency tree and will be using it in some way; and

b) those who aren't using babel still need these ES6 features transpiled down / shimmed for es5 environments. If core-js was moved to a peerDep then the proper installation instructions for this second category of people would be yarn add react-tracking core-js, rather than just one package (because to use RT they need both). Looking in the compiled sourcecore-js functions seem to be cherry-picked out, so not the full lib.

Pre babel 7 there was a peerDep for babel-runtime. If core-js were to move into that slot then the bundle size would remain the same as before and the behavior preserved, but it also seems slightly incorrect. Moving it into deps is for the small percentage of people who don't use babel and are on old environments. Most folks will I suspect fall into category A, however, so I think this is mostly a non-issue.

@tizmagik - Can update update https://github.com/NYTimes/react-tracking/pull/99 depending on what you'd like to do.

tizmagik commented 6 years ago

Yea, unfortunately this gets into the whole "should node_modules be transpiled" debate? One approach is to use a bundler like Rollup and build to multiple targets, but that gets messy fast and I'd rather defer that complexity for later. I'm hopeful, now that create-react-app has decided to transpile node_modules, that this approach will become more ubiquitous and we wouldn't have to solve for that here.

I think the instructions don't necessarily need to mention to install core-js as npm semantics already require that peerDeps be installed and will show a warning when they're not.

So, for now, I think let's replace babel-runtime peerDep with core-js peerDep. What do you think?

damassi commented 6 years ago

Will do 👍

tizmagik commented 6 years ago

Released v5.5.3