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

Support Babel in all of the same ways as CoffeeScript #8416

Closed
maxbrunsfeld opened this Issue Aug 17, 2015 · 17 comments

Comments

Projects
None yet
7 participants
@maxbrunsfeld
Contributor

maxbrunsfeld commented Aug 17, 2015

We want to move toward writing new code in javascript, not coffeescript. For example: https://github.com/atom/line-ending-selector. But before we start bundling packages written in Babel, we need to improve some infrastructure around it.

Tasks

The CI script

  • Make sure it runs specs properly for packages that use babel
  • Somehow detect when packages are using Standard Style and lint them properly

Bundled Packages

  • Make the build script treat babel source files the same as coffee-script files
  • Make the Gruntfile compile babel files

Opting in to Babel

  • Create an alternative to the 'use babel' string that fits in better with other babel directives. Maybe this:
/** use babel */
/** @jsx dom */

@maxbrunsfeld maxbrunsfeld changed the title from Allow for to Support Babel in all of the same ways as Coffee-script Aug 17, 2015

@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Aug 17, 2015

Contributor

/cc @kevinsawicki @nathansobo feel free to clarify or correct the above task list.

Contributor

maxbrunsfeld commented Aug 17, 2015

/cc @kevinsawicki @nathansobo feel free to clarify or correct the above task list.

@maxbrunsfeld maxbrunsfeld self-assigned this Aug 17, 2015

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Aug 17, 2015

Contributor

I'm unsure about the wording of the comment header... I see where you're going with @atom but I also think it's just a declaration that Babel is being used for the current document, not necessarily specific to Atom. But I suppose there are a bunch of options to Babel that would need to be specified if this were to be a general flag... I guess it would be nice if those could be enabled by their own directives.

Honestly just supporting /** use babel */ might be enough to blend in more cleanly with their existing header syntax.

Contributor

nathansobo commented Aug 17, 2015

I'm unsure about the wording of the comment header... I see where you're going with @atom but I also think it's just a declaration that Babel is being used for the current document, not necessarily specific to Atom. But I suppose there are a bunch of options to Babel that would need to be specified if this were to be a general flag... I guess it would be nice if those could be enabled by their own directives.

Honestly just supporting /** use babel */ might be enough to blend in more cleanly with their existing header syntax.

@mnquintana

This comment has been minimized.

Show comment
Hide comment
@mnquintana

mnquintana Aug 17, 2015

Member

Just a thought - do we need to have an opt-in for Babel? Would there be any significant harm in passing all JS through the Babel parser? In principle I get why we would want to avoid any unnecessary compilation, but if we're really considering making developing with Babel feel more like developing with Coffee, then it would be nice from a "user" perspective to be able to omit the pragma.

Member

mnquintana commented Aug 17, 2015

Just a thought - do we need to have an opt-in for Babel? Would there be any significant harm in passing all JS through the Babel parser? In principle I get why we would want to avoid any unnecessary compilation, but if we're really considering making developing with Babel feel more like developing with Coffee, then it would be nice from a "user" perspective to be able to omit the pragma.

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Aug 17, 2015

Contributor

@mnquintana My main concern is the overhead of always assuming the need for Babel for every single JS file, both in terms of performance and complexity. I haven't fully thought through the implications of dropping the pragma, but if you or anyone else wants to do an impact analysis on it, I'd be open to discussing it. It just has to be as close to free as possible.

Contributor

nathansobo commented Aug 17, 2015

@mnquintana My main concern is the overhead of always assuming the need for Babel for every single JS file, both in terms of performance and complexity. I haven't fully thought through the implications of dropping the pragma, but if you or anyone else wants to do an impact analysis on it, I'd be open to discussing it. It just has to be as close to free as possible.

@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Aug 17, 2015

Contributor

/** use babel */ might be enough to blend in

Yeah, that probably makes the most sense; use the same wording as the other style.

Contributor

maxbrunsfeld commented Aug 17, 2015

/** use babel */ might be enough to blend in

Yeah, that probably makes the most sense; use the same wording as the other style.

@paulcbetts

This comment has been minimized.

Show comment
Hide comment
@paulcbetts

paulcbetts Aug 17, 2015

Contributor

electron-compile uses 'use nobabel' as its directive. One thing you can do is modify your Babel compile-cache to look more like this, where we early-exit from any path involving node_modules:

https://github.com/electronjs/compile-cache/blob/master/src/compile-cache.js#L35

It also might be worth looking into just adopting electron-compile wholesale, it solves a lot of the problems that Atom is trying to one-off per-language, in a more general way, and Atom gets the benefit of improvements that the general Electron Community is using.

Contributor

paulcbetts commented Aug 17, 2015

electron-compile uses 'use nobabel' as its directive. One thing you can do is modify your Babel compile-cache to look more like this, where we early-exit from any path involving node_modules:

https://github.com/electronjs/compile-cache/blob/master/src/compile-cache.js#L35

It also might be worth looking into just adopting electron-compile wholesale, it solves a lot of the problems that Atom is trying to one-off per-language, in a more general way, and Atom gets the benefit of improvements that the general Electron Community is using.

@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Aug 17, 2015

Contributor

@paulcbetts electron-compile seems nice. A couple of thoughts:

  1. We would need to ship the compilers with Atom in order to provide dynamic compilation for third-party-packages. But we would want to avoid a dependency on node-scss, since we don't support that language. What do you think about splitting electron-compilers into separate npm modules for the different languages, so that we could depend on some but not all of them?
  2. We probably would want to keep babel as an opt-in compiler, for the sake of consistency for package authors. Is there a way that makes sense to configure that, or do you think we'd want to override the built-in babel adapter?
Contributor

maxbrunsfeld commented Aug 17, 2015

@paulcbetts electron-compile seems nice. A couple of thoughts:

  1. We would need to ship the compilers with Atom in order to provide dynamic compilation for third-party-packages. But we would want to avoid a dependency on node-scss, since we don't support that language. What do you think about splitting electron-compilers into separate npm modules for the different languages, so that we could depend on some but not all of them?
  2. We probably would want to keep babel as an opt-in compiler, for the sake of consistency for package authors. Is there a way that makes sense to configure that, or do you think we'd want to override the built-in babel adapter?

@maxbrunsfeld maxbrunsfeld changed the title from Support Babel in all of the same ways as Coffee-script to Support Babel in all of the same ways as Coffeescript Aug 17, 2015

@thomasjo thomasjo changed the title from Support Babel in all of the same ways as Coffeescript to Support Babel in all of the same ways as CoffeeScript Aug 18, 2015

@paulcbetts

This comment has been minimized.

Show comment
Hide comment
@paulcbetts

paulcbetts Aug 18, 2015

Contributor

But we would want to avoid a dependency on node-scss, since we don't support that language. What do you think about splitting electron-compilers into separate npm modules for the different languages, so that we could depend on some but not all of them?

It'd be a pretty big PITA to be honest - how about instead we allow you to reach into node_modules and delete node-sass, then we'll make it so that loading the default compiler set will tolerate failures

Is there a way that makes sense to configure that, or do you think we'd want to override the built-in babel adapter?

We could make that configurable, yeah

Contributor

paulcbetts commented Aug 18, 2015

But we would want to avoid a dependency on node-scss, since we don't support that language. What do you think about splitting electron-compilers into separate npm modules for the different languages, so that we could depend on some but not all of them?

It'd be a pretty big PITA to be honest - how about instead we allow you to reach into node_modules and delete node-sass, then we'll make it so that loading the default compiler set will tolerate failures

Is there a way that makes sense to configure that, or do you think we'd want to override the built-in babel adapter?

We could make that configurable, yeah

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Aug 25, 2015

Contributor

If we can support .babelrc files properly, then I think we can get rid of the header. Then every package can use whichever Babel options it prefers.

Contributor

bolinfest commented Aug 25, 2015

If we can support .babelrc files properly, then I think we can get rid of the header. Then every package can use whichever Babel options it prefers.

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Aug 25, 2015

Contributor

@bolinfest Are you thinking the presence of .babelrc in a parent directory would trigger Babel being used? Still seems a bit broad to me. I do like picking up the options of the nearist .babelrc.

Contributor

nathansobo commented Aug 25, 2015

@bolinfest Are you thinking the presence of .babelrc in a parent directory would trigger Babel being used? Still seems a bit broad to me. I do like picking up the options of the nearist .babelrc.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Aug 25, 2015

Contributor

@nathansobo Yes, I think that's completely reasonable since that's what the intent of the presence of a .babelrc should be. only and ignore can be used to filter the paths that get transpiled: http://babeljs.io/docs/usage/options/.

I think that the main tricky bit is figuring out how to construct the cache key correctly, as mentioned in #8182.

Contributor

bolinfest commented Aug 25, 2015

@nathansobo Yes, I think that's completely reasonable since that's what the intent of the presence of a .babelrc should be. only and ignore can be used to filter the paths that get transpiled: http://babeljs.io/docs/usage/options/.

I think that the main tricky bit is figuring out how to construct the cache key correctly, as mentioned in #8182.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Aug 25, 2015

Contributor

I can't believe I'm saying this, but maybe we could tolerate some amount of un-soundness in the caching?

@sebmck What's the best way to let people use .babelrc files while still being able to produce an appropriate cache key? Or rather, what's Babel's process for resolving .babelrc files? It's unclear from https://babeljs.io/docs/usage/babelrc/. Does it keep looking in parent directories until it gets to /? Could we maybe have it stop at the first package.json it finds?

Contributor

bolinfest commented Aug 25, 2015

I can't believe I'm saying this, but maybe we could tolerate some amount of un-soundness in the caching?

@sebmck What's the best way to let people use .babelrc files while still being able to produce an appropriate cache key? Or rather, what's Babel's process for resolving .babelrc files? It's unclear from https://babeljs.io/docs/usage/babelrc/. Does it keep looking in parent directories until it gets to /? Could we maybe have it stop at the first package.json it finds?

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Aug 25, 2015

Contributor

@nathansobo @sebmck Or maybe we could require people to specify it via the package.json, though that might be a little obnoxious: https://babeljs.io/docs/usage/babelrc/#use-via-package-json.

Contributor

bolinfest commented Aug 25, 2015

@nathansobo @sebmck Or maybe we could require people to specify it via the package.json, though that might be a little obnoxious: https://babeljs.io/docs/usage/babelrc/#use-via-package-json.

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Aug 25, 2015

Contributor

I'd prefer it to support and use the same config resolution logic that Babel does for consistency. You can look up the options each time and use it in the hash key like what babel/register does (relevant source here), I don't think the config resolution logic is exposed though.

Contributor

kittens commented Aug 25, 2015

I'd prefer it to support and use the same config resolution logic that Babel does for consistency. You can look up the options each time and use it in the hash key like what babel/register does (relevant source here), I don't think the config resolution logic is exposed though.

@gnestor

This comment has been minimized.

Show comment
Hide comment
@gnestor

gnestor Feb 10, 2016

@bolinfest @nathansobo @kittens I would like to use a couple Babel plugins that are specific to my package but it looks like it's currently not possible to bring your own Babel config (using babelrc or package.json). I'd be happy to work on a PR to enable this, but I'm not sure where to start...

gnestor commented Feb 10, 2016

@bolinfest @nathansobo @kittens I would like to use a couple Babel plugins that are specific to my package but it looks like it's currently not possible to bring your own Babel config (using babelrc or package.json). I'd be happy to work on a PR to enable this, but I'm not sure where to start...

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Feb 10, 2016

Contributor

@gnestor The entry point for all on-the-fly transpilation is the compile-cache. You'll see we have a Babel handler listed for .js files. Currently we only process files with a specific header. If you could find an efficient way to detect the presence Babel metadata in a standard location that follows Babel's typical rules (a .babelrc file in a containing directory or in the package.json perhaps), that would open the door to enabling specific options. We'd probably need a way to control what version of Babel to apply as well.

Contributor

nathansobo commented Feb 10, 2016

@gnestor The entry point for all on-the-fly transpilation is the compile-cache. You'll see we have a Babel handler listed for .js files. Currently we only process files with a specific header. If you could find an efficient way to detect the presence Babel metadata in a standard location that follows Babel's typical rules (a .babelrc file in a containing directory or in the package.json perhaps), that would open the door to enabling specific options. We'd probably need a way to control what version of Babel to apply as well.

@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot Apr 14, 2018

This issue has been automatically locked since there has not been any recent activity after it was closed. If you can still reproduce this issue in Safe Mode then please open a new issue and fill out the entire issue template to ensure that we have enough information to address your issue. Thanks!

lock bot commented Apr 14, 2018

This issue has been automatically locked since there has not been any recent activity after it was closed. If you can still reproduce this issue in Safe Mode then please open a new issue and fill out the entire issue template to ensure that we have enough information to address your issue. Thanks!

@lock lock bot locked and limited conversation to collaborators Apr 14, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.