Skip to content
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

Make it work with Babel 7 #55

Merged
merged 4 commits into from
Jun 22, 2018
Merged

Make it work with Babel 7 #55

merged 4 commits into from
Jun 22, 2018

Conversation

tleunen
Copy link
Contributor

@tleunen tleunen commented Jun 20, 2018

Fix #52 by changing babel-register with @babel/register

tleunen added 2 commits June 20, 2018 11:03
Fix kentcdodds#52 by changing babel-register with @babel/register
@codecov
Copy link

codecov bot commented Jun 20, 2018

Codecov Report

Merging #55 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #55   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         102    102           
  Branches       21     21           
=====================================
  Hits          102    102
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c47ce5...2547e3a. Read the comment docs.

@tleunen
Copy link
Contributor Author

tleunen commented Jun 20, 2018

I wonder if we should also explicitely add a peerDep on @babel/core 7 (or babel-core 7).

ping @kentcdodds @oliviertassinari

@kentcdodds
Copy link
Owner

Thanks @tleunen!

So actually, I'd prefer to go the route that babel-plugin-macros went which is that it doesn't require babel-register at all or even a specific version of babel: https://github.com/kentcdodds/babel-plugin-codegen/pull/12/files

It's a little large, but it basically amounts to passing the babel that our plugin function is called with and using that rather than any of the babel packages.

Would you be willing to do that?

@tleunen
Copy link
Contributor Author

tleunen commented Jun 20, 2018

Yes, it's actually a better way to solve it. I'll do that tomorrow.

@kentcdodds
Copy link
Owner

Super! Thank you very much :D

@tleunen
Copy link
Contributor Author

tleunen commented Jun 21, 2018

@kentcdodds Could you take a look at why the tagged template tests fail? Looks like the tests don't use the babelrc config?

@kentcdodds
Copy link
Owner

I'll try to get to it as soon as I'm able. No promises I'll have time anytime soon though :-(

That said, at work we're upgrading to babel 7 so I may have some worktime for this.

@kentcdodds
Copy link
Owner

I'm working on this now and almost finished.

We're going to have a breaking change and lose support for something, but it'll be better this way.

BREAKING CHANGE: We no longer run require('babel-register') so not all code will be transpiled with babel by preval. Some will, but it's advised to not write code that you want prevaled in a way that doesn't run in the version of node you're using
@kentcdodds
Copy link
Owner

Alrighty, that should do it!

@kentcdodds kentcdodds merged commit 2868210 into kentcdodds:master Jun 22, 2018
@tleunen
Copy link
Contributor Author

tleunen commented Jun 22, 2018

Thank you @kentcdodds 😄

@tleunen tleunen deleted the patch-1 branch June 22, 2018 16:24
@kentcdodds
Copy link
Owner

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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