Skip to content

Make it work with Babel 7#55

Merged
kentcdodds merged 4 commits into
kentcdodds:masterfrom
tleunen:patch-1
Jun 22, 2018
Merged

Make it work with Babel 7#55
kentcdodds merged 4 commits into
kentcdodds:masterfrom
tleunen:patch-1

Conversation

@tleunen

@tleunen tleunen commented Jun 20, 2018

Copy link
Copy Markdown
Contributor

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

codecov Bot commented Jun 20, 2018

Copy link
Copy Markdown

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

tleunen commented Jun 20, 2018

Copy link
Copy Markdown
Contributor Author

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

ping @kentcdodds @oliviertassinari

@kentcdodds

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

tleunen commented Jun 20, 2018

Copy link
Copy Markdown
Contributor Author

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

@kentcdodds

Copy link
Copy Markdown
Owner

Super! Thank you very much :D

@tleunen

tleunen commented Jun 21, 2018

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Owner

Alrighty, that should do it!

@kentcdodds
kentcdodds merged commit 2868210 into kentcdodds:master Jun 22, 2018
@tleunen

tleunen commented Jun 22, 2018

Copy link
Copy Markdown
Contributor Author

Thank you @kentcdodds 😄

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

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