captivationsoftware / react-sticky

<Sticky /> component for awesome React apps
MIT License
2.64k stars 386 forks source link

Upgrade to babel 7 #273

Closed ignatiusreza closed 5 years ago

ignatiusreza commented 5 years ago

This is a first step PR for fixing travis build (second PR at #274)..

Both webpack and webpack-dev-server in master depends transitively on upath@1.0.2 which locked node engine at >=4 <=9, even though travis is using v11.10.0..

vcarl commented 5 years ago

Awesome, thanks! Let me confirm that builds still work tonight and I'll merge. Looks good just from code though. I especially want to confirm that preset-env with no browserlist works correctly, don't want to pull in a bunch of redundant polyfills.

vcarl commented 5 years ago

It looks like this is pulling in a few additional polyfills @ignatiusreza. After npm run compile, lib/'s contents

master:

 5.3K Feb 21 21:31 Container.js
 6.2K Feb 21 21:31 Sticky.js
 531B Feb 21 21:31 index.js

this branch:

 7.0K Feb 21 21:29 Container.js
 7.6K Feb 21 21:29 Sticky.js
 647B Feb 21 21:29 index.js

Would you mind digging into that a little bit? Ideally this wouldn't increase bundle size, consumers of the library should supply polyfills.

ignatiusreza commented 5 years ago

Sure, i'll try to dig into it

ignatiusreza commented 5 years ago

spent some time trying to figure it out.. I have a good news and a bad news..

the bad news: rather than polyfill, I think the extra bytes came from babel runtime itself doing more check than it was before..

the good news: babel 7 comes with @babel/plugin-transform-runtime which can be used to remove duplicated runtime injected into each files and replace it with import statements from @babel/runtime

build size different:

as seen above, the build size is now slightly smaller than master.. though, that's not counting the imported bytes from @babel/runtime..

I did a quick check into some other popular packages, and found that at least react-redux and react-router uses the same technique.. meaning that the extra bytes from runtime will be shared across these packages..

wdyt?

vcarl commented 5 years ago

Cool, looks like that saves a few hundred bytes on each file compared to before! Looks good to me.