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

Babel 7 support #5101

Closed
laurentpayot opened this issue Aug 28, 2018 · 15 comments
Closed

Babel 7 support #5101

laurentpayot opened this issue Aug 28, 2018 · 15 comments

Comments

@laurentpayot
Copy link

laurentpayot commented Aug 28, 2018

Babel 7 is out!

For transpilation CoffeeScript still looks for a babel-core package instead of the new Babel 7 @babel/core.
More details: http://babeljs.io/blog/2017/12/27/nearing-the-7.0-release#renames-scoped-packages-babel-x

  • CoffeeScript version: 2.3.1
  • Node.js version: 10.8.0
@bazineta
Copy link

So far as I can see, the new version seems to work fine with CS 2.3.1. In a Unix environment, you can do a

ln -s @babel/core babel-core

in your node_modules directory if you'd like to do a quick sanity check with the new babel-core version.

@GeoffreyBooth
Copy link
Collaborator

This is probably as easy as updating

try
babel = require 'babel-core'
catch

to also try to require @babel/core. We should keep the check for babel-core as well, for users who haven’t upgraded to Babel 7 yet. CoffeeScript might as well work with both 6 and 7 if they’re both compatible.

Anyone up for an easy PR?

@bazineta
Copy link

@GeoffreyBooth There are three files affected; index.coffee, repl.coffee, and command.coffee.

The changes are all straightforward, seemingly along the lines of:

 try
    babel = require '@babel/core'
  catch
    try
      babel = require 'babel-core'
    catch
      # This error is only for Node, as CLI users will see a different error
      # earlier if they don’t have Babel installed.
      throw new Error 'To use the transpile option, you must have the \'@babel/core\' or \'babel-core\' module installed'

Though it gets quite duplicative and therefore might be something that should have a common function for, especially given the nested try blocks. However, it's indeed a quick and easy change to all 3, though I'm not sure what the tests would be, other than 'does not explode in flames', I suppose.

@bazineta
Copy link

@GeoffreyBooth PR #5102, though as mentioned, an appropriate test other than does not go bang is unclear to me at present.

@GeoffreyBooth
Copy link
Collaborator

Any tests that the old version had would apply to the new one. Though I’m not sure if we wrote tests for the transpile option; we probably should if we haven’t.

I guess if you wanted to abstract this, you could make a getBabel function that returns the babel object, or throws. Then the original locations could be rewritten like:

 try 
   babel = getBabel()
 catch

And you could put the double-try stuff inside getBabel.

@bazineta
Copy link

I think that upon further consideration, this is probably not something that'll need to be dual-pathed for an extended period of time, as those who use Babel are, frankly, likely to upgrade quickly, at least in my opinion.

@laurentpayot
Copy link
Author

@bazineta thanks for the PR. I'm using the symbolic link workaround (in postinstall script) for months now, as Babel 7 has been in beta/RC for a long time. Babel users also didn't wait for the official v7 release to use it. So I guess it is safe now for CS to use Babel 7.

@Inve1951
Copy link
Contributor

I don't see the need to break old setups.

those who use Babel are, frankly, likely to upgrade quickly

The transpile option is ment for ppl who really don't use babel.

@laurentpayot
Copy link
Author

laurentpayot commented Aug 30, 2018

The transpile option is ment for ppl who really don't use babel.

I use Babel a lot, via babel-loader in Webpack. Still I have to run scripts in CS with the transpile option…

@bazineta
Copy link

I use Babel a lot, via babel-loader in Webpack

Same here; that's our use of it, via webpack loaders. The PR supports both pre-7 and 7+ babel.

@loganfsmyth
Copy link

I'll note that, in the short term anyway, users can probably install babel-core@7.0.0-bridge.0, which is a package that just redirects to @babel/core.

@GeoffreyBooth
Copy link
Collaborator

Done in #5105.

@max-degterev
Copy link

@GeoffreyBooth could we get a quick bump to 2.3.2 to have this fix available in npm?

@GeoffreyBooth
Copy link
Collaborator

Please see #5110. Anyone available to review that?

@laurentpayot
Copy link
Author

Thanks everyone 👍

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

No branches or pull requests

6 participants