Skip to content

Upgrade to babel 7 #273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 22, 2019
Merged

Conversation

ignatiusreza
Copy link
Contributor

@ignatiusreza ignatiusreza commented Feb 21, 2019

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
Copy link
Contributor

vcarl commented Feb 21, 2019

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
Copy link
Contributor

vcarl commented Feb 22, 2019

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
Copy link
Contributor Author

Sure, i'll try to dig into it

before:

  7123 Container.js
   647 index.js
  7772 Sticky.js

after:

  5113 Container.js
   637 index.js
  5874 Sticky.js
@ignatiusreza
Copy link
Contributor Author

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:

  • before:

    7123 Container.js
    647 index.js
    7772 Sticky.js

  • after:

    5113 Container.js
    637 index.js
    5874 Sticky.js

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
Copy link
Contributor

vcarl commented Feb 22, 2019

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

@vcarl vcarl merged commit c51b834 into captivationsoftware:master Feb 22, 2019
@ignatiusreza ignatiusreza deleted the upgrade/babel branch February 23, 2019 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants