Skip to content
This repository has been archived by the owner on Apr 1, 2023. It is now read-only.

upgrade to babel 7 #103

Merged
merged 1 commit into from
Dec 3, 2018
Merged

upgrade to babel 7 #103

merged 1 commit into from
Dec 3, 2018

Conversation

sag1v
Copy link

@sag1v sag1v commented Nov 22, 2018

resolve #86

@sag1v sag1v mentioned this pull request Nov 22, 2018
@transitive-bullshit
Copy link
Owner

Thanks a ton for doing this work @sag1v. Sorry it's taken awhile to get back to you, as I've been traveling a lot recently.

This looks perfect if we just want to switch as-is.

Babel 7 does blow up the deps quite a bit, unfortunately. I was thinking of using microbundle to hide the complexity of rollup & babel, but I guess this would have the downside of being less easy to customize.

Thoughts on this tradeoff?

@sag1v
Copy link
Author

sag1v commented Dec 2, 2018

If we go with the create-react-app / react-scripts route i guess we should implement eject.
This can be a great abstraction over the dependencies and may make upgrading dependencies easier for the end user while providing an escape hatch to manually do it by hand.

Not sure this PR is affected by this decision though, we can upgrade to babel 7 now and in the near future go for the microbundle approach. The downside of doing this in 2 steps is that we will have at least 2 releases with breaking changes (babel v7 is a breaking change as far as i know).

@transitive-bullshit transitive-bullshit merged commit e8c0591 into transitive-bullshit:master Dec 3, 2018
@avdeev
Copy link

avdeev commented Dec 12, 2018

@sag1v @transitive-bullshit I see next warning with this changes.

(!) babel plugin: Using "external-helpers" plugin with rollup-plugin-babel is deprecated, as it now automatically deduplicates your Babel helpers.

@sag1v
Copy link
Author

sag1v commented Dec 12, 2018

@avdeev did you forked the repo and used it or just copy the changes?
As far as i know these changes were not published to npm yet.

Can you post the steps to reproduce the warning?

@avdeev
Copy link

avdeev commented Dec 12, 2018

@sag1v Yes, I just copied the changes to my project. I will try to reproduce with the fork.

@avdeev
Copy link

avdeev commented Dec 12, 2018

@sag1v Reproduced from the fork.

Mac-mini-Alexey:projects alexeyavdeev$ node create-react-library/
? Package Name test
? Package Description test
? Author's GitHub Handle avdeev
? GitHub Repo Path avdeev/test
? License MIT
? Package Manager npm
? Template default
✔ Copying default template to /Users/alexeyavdeev/projects/test
✔ Running npm install and npm link
✔ Initializing git repo


Your module has been created at /Users/alexeyavdeev/projects/test.

To get started, in one tab, run:
$ cd test && npm start

And in another tab, run the create-react-app dev server:
$ cd test/example && npm start

Mac-mini-Alexey:projects alexeyavdeev$ cd test
Mac-mini-Alexey:test alexeyavdeev$ npm i

> [email protected] prepare /Users/alexeyavdeev/projects/test
> npm run build


> [email protected] build /Users/alexeyavdeev/projects/test
> rollup -c


src/index.js → dist/index.js, dist/index.es.js...
(!) babel plugin: Using "external-helpers" plugin with rollup-plugin-babel is deprecated, as it now automatically deduplicates your Babel helpers.
src/index.js
created dist/index.js, dist/index.es.js in 1.2s
npm WARN [email protected] requires a peer of babel-eslint@^7.2.3 but none is installed. You must install peer dependencies yourself.
npm WARN [email protected] requires a peer of eslint@^4.1.1 but none is installed. You must install peer dependencies yourself.
npm WARN [email protected] requires a peer of eslint@>=1.6.0 <5.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN [email protected] requires a peer of eslint@^2.10.2 || ^3 || ^4 but none is installed. You must install peer dependencies yourself.

audited 19331 packages in 13.824s
found 1 high severity vulnerability
  run `npm audit fix` to fix them, or `npm audit` for details

@sag1v
Copy link
Author

sag1v commented Dec 12, 2018

@transitive-bullshit I think that rollup-plugin-babel should run with plugin-transform-runtime instead of external-helpers. see this issue for example. maybe @Andarist could help us?

@Andarist
Copy link

If you bundle libraries you should aim for using @babel/plugin-transform-runtime, but your library has to have @babel/runtime as dependency in such case - that's why rollup-plugin-babel doesnt use @babel/plugin-transform-runtime by default.

@sag1v
Copy link
Author

sag1v commented Dec 13, 2018

Thanks @Andarist !
@avdeev Can you confirm the changes in #109 solves the warning?

@ravecat
Copy link

ravecat commented Mar 26, 2019

@Andarist could you clarify new rollup config with @babel/plugin-transform-runtime. Can I exclude transform-runtime Babel plugin and I reduce bundle size?

@Andarist
Copy link

@ravecat Could you clarify what's your goal? Do you want to bundle a library?

@ravecat
Copy link

ravecat commented Mar 27, 2019

@Andarist yep, I already create bundle with that config https://github.com/ravecat/styled-components-toolbox/blob/master/rollup.config.js , but I'm interested in this part of the documentation https://github.com/rollup/rollup-plugin-babel#helpers

If you do not wish the babel helpers to be included in your bundle at all (but instead reference the global babelHelpers object), you may set the externalHelpers option to true

  plugins: [
    babel({
      plugins: ['external-helpers'],
      externalHelpers: true
    })
  ]

But current config doesn't work with babel 7. I want to use external-helpers. Is it reduce size of my package?

@brownieboy
Copy link

I just created a new project from create-react-library but still get Babel 6. What am I missing?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrading to babel 7
6 participants