Transpile all .js files beginning with the "use 6to5"; pragma with 6to5. #5299

Merged
merged 7 commits into from Feb 2, 2015

Conversation

Projects
None yet
@bolinfest
Contributor

bolinfest commented Jan 29, 2015

Transpile all .js files with 6to5.

In the spirit of supporting JavaScript development for Atom packages,
this adds default support for es.next transpilation support in the way
that Atom already has default support for CoffeeScript transpilation.
There are many new features in ES6+ that make JavaScript development
easier and more enjoyable, particularly in terms of support for async code.

For reference, this was a much faster way to iterate on this than running ./script/build
each time:

cp /Users/mbolin/src/atom/static/index.js /Applications/Atom.app/Contents/Resources/app/static/index.js
coffee --output /Applications/Atom.app/Contents/Resources/app/src --compile /Users/mbolin/src/atom/src/esnext.coffee

Run the following in the console to see how warm the cache was after startup:

global.require('../src/esnext/').getCacheHits()
global.require('../src/esnext/').getCacheMisses()
@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

See if it's possible to have apm publish do the transpilation so users of the package do not have to waste startup time on transpilation.

Usually we prefer to warm the cache on apm install so publishes can always just be regular tarballs from the repo.

I'd be happy to add this into apm install when the time comes, https://github.com/atom/apm/blob/30569a16d87916f191ffb99415f61dac99bc0802/src/install.coffee#L473-L487

Member

kevinsawicki commented Jan 29, 2015

See if it's possible to have apm publish do the transpilation so users of the package do not have to waste startup time on transpilation.

Usually we prefer to warm the cache on apm install so publishes can always just be regular tarballs from the repo.

I'd be happy to add this into apm install when the time comes, https://github.com/atom/apm/blob/30569a16d87916f191ffb99415f61dac99bc0802/src/install.coffee#L473-L487

src/esnext.coffee
+path = require 'path';
+to5 = require '6to5'
+
+ES_DIRECTORY = 'atom-es'

This comment has been minimized.

@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Could you explain a little about what this directory is for?

@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Could you explain a little about what this directory is for?

This comment has been minimized.

@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Nevermind, I see it is described in the body of the PR.

@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Nevermind, I see it is described in the body of the PR.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

As an initial starting point for discussion, this change requires
developers to store all files that need to be transpiled in an atom-es
directory under the module root.

Is there a file extension that could be used/introduced instead?

Member

kevinsawicki commented Jan 29, 2015

As an initial starting point for discussion, this change requires
developers to store all files that need to be transpiled in an atom-es
directory under the module root.

Is there a file extension that could be used/introduced instead?

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

This change also supports reading options to pass to 6to5 from the source file's
package.json in case the default 6to5 options used by Atom are
not appropriate.

Do you think this will be common? Just wondering if we can get away with a single configuration to start.

Member

kevinsawicki commented Jan 29, 2015

This change also supports reading options to pass to 6to5 from the source file's
package.json in case the default 6to5 options used by Atom are
not appropriate.

Do you think this will be common? Just wondering if we can get away with a single configuration to start.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Is the reason to not just transpile all .js files that the performance is noticeable even for files where nothing would be changed?

Member

kevinsawicki commented Jan 29, 2015

Is the reason to not just transpile all .js files that the performance is noticeable even for files where nothing would be changed?

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@kevinsawicki I just addressed some of these issues in the forum, but I'll reiterate here as I'm hoping to keep the discussion on this pull request:

  • I'm rethinking that whole atom-es thing and believe that listing the paths in the package.json would be more appropriate. Then you can organize the files in your package however you like, tests can be transpiled, etc.
  • I don't think that requiring the use of a new file extension is a good idea. Especially if you're writing ES6, you shouldn't have to pretend that you aren't writing JavaScript.
  • I don't think that it will be common to need to override the default 6to5 options, but I wanted to give myself an escape hatch for easier testing. In particular, as we play with newer versions of React in Atom, I would set "reactCompat": false in my test packages.
  • I haven't profiled how long transpilation takes, but I guess I'm more of a "better safe than sorry" kinda guy in that respect. If it's fast enough now, but then gets slower over time, what do we do? For people who aren't using ES6, if 6to5 doesn't write out their code exactly as they wrote it originally, it seems like that could be confusing. But if you want to push that envelope, then I'm on board! (I was also a little hesitant because all of your transitive dependencies would also get transpiled, which includes everything from npm, so I don't know if that could possibly break anything.)
Contributor

bolinfest commented Jan 29, 2015

@kevinsawicki I just addressed some of these issues in the forum, but I'll reiterate here as I'm hoping to keep the discussion on this pull request:

  • I'm rethinking that whole atom-es thing and believe that listing the paths in the package.json would be more appropriate. Then you can organize the files in your package however you like, tests can be transpiled, etc.
  • I don't think that requiring the use of a new file extension is a good idea. Especially if you're writing ES6, you shouldn't have to pretend that you aren't writing JavaScript.
  • I don't think that it will be common to need to override the default 6to5 options, but I wanted to give myself an escape hatch for easier testing. In particular, as we play with newer versions of React in Atom, I would set "reactCompat": false in my test packages.
  • I haven't profiled how long transpilation takes, but I guess I'm more of a "better safe than sorry" kinda guy in that respect. If it's fast enough now, but then gets slower over time, what do we do? For people who aren't using ES6, if 6to5 doesn't write out their code exactly as they wrote it originally, it seems like that could be confusing. But if you want to push that envelope, then I'm on board! (I was also a little hesitant because all of your transitive dependencies would also get transpiled, which includes everything from npm, so I don't know if that could possibly break anything.)
@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

Come to think of it, how do you feel about an .atom.js suffix? Will that
play well with Node? Would foo.atom.js have to be pulled in via
require('foo.atom') instead of just require('foo')?
On Jan 28, 2015 5:55 PM, "Kevin Sawicki" notifications@github.com wrote:

Is the reason to not just transpile all .js files that the performance is
noticeable even for files where nothing would be changed?


Reply to this email directly or view it on GitHub
#5299 (comment).

Contributor

bolinfest commented Jan 29, 2015

Come to think of it, how do you feel about an .atom.js suffix? Will that
play well with Node? Would foo.atom.js have to be pulled in via
require('foo.atom') instead of just require('foo')?
On Jan 28, 2015 5:55 PM, "Kevin Sawicki" notifications@github.com wrote:

Is the reason to not just transpile all .js files that the performance is
noticeable even for files where nothing would be changed?


Reply to this email directly or view it on GitHub
#5299 (comment).

@lee-dohm

This comment has been minimized.

Show comment
Hide comment
@lee-dohm

lee-dohm Jan 29, 2015

Member

My main concern is whether or not the class model, module model, Unicode support and other features mesh well with the CoffeeScript-generated ones. As can be seen in this and other linked topics from Discuss there already are issues with extending CoffeeScript classes from JavaScript. I suppose if 6to5 support is limited to packages then as before it is up to the package author to ensure compatibility ... though with the planned ServiceHub ... that might be more complicated than before.

My secondary concern is that I prefer CoffeeScript to what I've seen of ES6, so on a personal level I would hope that this is adding an option for package developers ... not adding an option with the hope of eventually supplanting CoffeeScript 😀

I'm sure the "do we compile or don't we and when" and compile caching stuff will all get worked out by smarter people than me 😄

Member

lee-dohm commented Jan 29, 2015

My main concern is whether or not the class model, module model, Unicode support and other features mesh well with the CoffeeScript-generated ones. As can be seen in this and other linked topics from Discuss there already are issues with extending CoffeeScript classes from JavaScript. I suppose if 6to5 support is limited to packages then as before it is up to the package author to ensure compatibility ... though with the planned ServiceHub ... that might be more complicated than before.

My secondary concern is that I prefer CoffeeScript to what I've seen of ES6, so on a personal level I would hope that this is adding an option for package developers ... not adding an option with the hope of eventually supplanting CoffeeScript 😀

I'm sure the "do we compile or don't we and when" and compile caching stuff will all get worked out by smarter people than me 😄

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@lee-dohm I've done a bunch of subclassing of SpacePen's View class form ordinary JS and there's nothing fundamental that prevents it from working. I believe you just have to be a bit careful with how the super keyword is thrown around in CoffeeScript. I don't see any issues with mixing 6to5-generated classes with inheritance.

Also, in the JS console in Atom, I can already write class Foo {}, so it's probably not too long before CoffeeScript has an option to transpile to ES6 classes.

I don't mean for this diff to suggest that Atom should drop support for CoffeeScript, though once you start writing more async code, you may come around on your own :)

Contributor

bolinfest commented Jan 29, 2015

@lee-dohm I've done a bunch of subclassing of SpacePen's View class form ordinary JS and there's nothing fundamental that prevents it from working. I believe you just have to be a bit careful with how the super keyword is thrown around in CoffeeScript. I don't see any issues with mixing 6to5-generated classes with inheritance.

Also, in the JS console in Atom, I can already write class Foo {}, so it's probably not too long before CoffeeScript has an option to transpile to ES6 classes.

I don't mean for this diff to suggest that Atom should drop support for CoffeeScript, though once you start writing more async code, you may come around on your own :)

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@kevinsawicki Would this diff be more amenable if I changed it so I:

  • Removed the option to customize flags to 6to5.
  • Transpiled all files with a .js extension.
  • Added caching support. If all files with a .js extension will go through 6to5, then I don't think this can go in until caching is set up.

If that will move things along, then I'm happy to go ahead and make these changes.

Contributor

bolinfest commented Jan 29, 2015

@kevinsawicki Would this diff be more amenable if I changed it so I:

  • Removed the option to customize flags to 6to5.
  • Transpiled all files with a .js extension.
  • Added caching support. If all files with a .js extension will go through 6to5, then I don't think this can go in until caching is set up.

If that will move things along, then I'm happy to go ahead and make these changes.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

I can tell you this much. If I turn on transpilation for all .js files and don't implement caching, Atom takes almost 2 minutes to start up. It makes it hard to iterate on testing this feature...I guess I'll just have to get it right the first time?

Contributor

bolinfest commented Jan 29, 2015

I can tell you this much. If I turn on transpilation for all .js files and don't implement caching, Atom takes almost 2 minutes to start up. It makes it hard to iterate on testing this feature...I guess I'll just have to get it right the first time?

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Jan 29, 2015

Contributor

Potential extensions could be .es, .es6, .jsx or even .6to5. None of those are entirely suitable though.

You shouldn't really be concerned with class interopability as ES6 classes are just sugar around prototypes. ie.

class Foo {}

is basically equivalent to

function Foo() {}

Foo.prototype.bar = function () {};

Which is fully compatible with CoffeeScript.

Compiling every single file with the .js extension seems like an extreme overkill which is why you're seeing such long compile times.

Contributor

kittens commented Jan 29, 2015

Potential extensions could be .es, .es6, .jsx or even .6to5. None of those are entirely suitable though.

You shouldn't really be concerned with class interopability as ES6 classes are just sugar around prototypes. ie.

class Foo {}

is basically equivalent to

function Foo() {}

Foo.prototype.bar = function () {};

Which is fully compatible with CoffeeScript.

Compiling every single file with the .js extension seems like an extreme overkill which is why you're seeing such long compile times.

package.json
@@ -19,6 +19,7 @@
],
"atomShellVersion": "0.21.0",
"dependencies": {
+ "6to5": "3.0.14",

This comment has been minimized.

@kittens

kittens Jan 29, 2015

Contributor

6to5-core would be better here since it's a build of 6to5 without certain dependencies so it'll install faster. You can see the script that generates the 6to5-core package.json here.

@kittens

kittens Jan 29, 2015

Contributor

6to5-core would be better here since it's a build of 6to5 without certain dependencies so it'll install faster. You can see the script that generates the 6to5-core package.json here.

src/esnext.coffee
+ # Blacklisted features do not get transpiled. Features that are
+ # natively supported in the target environment should be listed
+ # here. Because Atom uses a bleeding edge version of Node/io.js,
+ # I think this can include es6.arrowFunctions, es6.classes, and

This comment has been minimized.

@kittens

kittens Jan 29, 2015

Contributor

v8 doesn't support arrow functions and classes are still behind a flag as they aren't quite spec compliant yet.

@kittens

kittens Jan 29, 2015

Contributor

v8 doesn't support arrow functions and classes are still behind a flag as they aren't quite spec compliant yet.

src/esnext.coffee
+ js = to5.transform(sourceCode, options).code
+ stats.misses++
+ catch error
+ console.error('Error compiling ' + filePath + ': ' + error)

This comment has been minimized.

@kittens

kittens Jan 29, 2015

Contributor

error here will be an instance of Error which wont coerce very well to a string and you wont be able to see the code frame. Should be getting err.stack which automatically includes the filename anyway. He's an example:

SyntaxError: script.js: Unexpected token (1:4)
> 1  | asdf!@#$
     |     ^
  2  |
@kittens

kittens Jan 29, 2015

Contributor

error here will be an instance of Error which wont coerce very well to a string and you wont be able to see the code frame. Should be getting err.stack which automatically includes the filename anyway. He's an example:

SyntaxError: script.js: Unexpected token (1:4)
> 1  | asdf!@#$
     |     ^
  2  |
@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@sebmck Thanks for the tip about 6to5-core! I updated the diff to use it.

@kevinsawicki I think I have addressed all of the issues you raised. Now all .js files go through 6to5 and have a cache path that is a function of:

  • The version of 6to5.
  • The options passed to 6to5. (See create6to5VersionAndOptionsDigest().)
  • The file contents.

The first time you start Atom (or if you change the version of 6to5 or its options), it will take about 2 minutes to start up, but after that, it is much better. Atom tells me that it loads 1,224 files from cache on startup with my current set of packages. That's quite a bit of sha1'ing, but I'll let you test drive this for yourself to decide how you feel about its performance.

I originally tried incorporating atom.appVersion into the hash, but then every time you make a local commit to Atom, the atom.appVersion changes, invalidating the cache. Then you hit a 2 minute startup time again, so that was impractical.

Contributor

bolinfest commented Jan 29, 2015

@sebmck Thanks for the tip about 6to5-core! I updated the diff to use it.

@kevinsawicki I think I have addressed all of the issues you raised. Now all .js files go through 6to5 and have a cache path that is a function of:

  • The version of 6to5.
  • The options passed to 6to5. (See create6to5VersionAndOptionsDigest().)
  • The file contents.

The first time you start Atom (or if you change the version of 6to5 or its options), it will take about 2 minutes to start up, but after that, it is much better. Atom tells me that it loads 1,224 files from cache on startup with my current set of packages. That's quite a bit of sha1'ing, but I'll let you test drive this for yourself to decide how you feel about its performance.

I originally tried incorporating atom.appVersion into the hash, but then every time you make a local commit to Atom, the atom.appVersion changes, invalidating the cache. Then you hit a 2 minute startup time again, so that was impractical.

@mark-hahn

This comment has been minimized.

Show comment
Hide comment
@mark-hahn

mark-hahn Jan 29, 2015

Contributor

The first time you start Atom (or if you change the version of 6to5 or its options), it will take about 2 minutes to start up

This seems unacceptable to me. If I saw this happening I'd kill the process.

Contributor

mark-hahn commented Jan 29, 2015

The first time you start Atom (or if you change the version of 6to5 or its options), it will take about 2 minutes to start up

This seems unacceptable to me. If I saw this happening I'd kill the process.

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Jan 29, 2015

Contributor

Completely agree with @mark-hahn. I think it should be available under an alternate extension, not sure what, but the additional load-time is pretty unacceptable to me. What about the possibility of adding a 6to5 flag on a packages package.json or some other flag somewhere that a package can use to advertise that it should be transpiled?

Contributor

kittens commented Jan 29, 2015

Completely agree with @mark-hahn. I think it should be available under an alternate extension, not sure what, but the additional load-time is pretty unacceptable to me. What about the possibility of adding a 6to5 flag on a packages package.json or some other flag somewhere that a package can use to advertise that it should be transpiled?

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Jan 29, 2015

Contributor

After sleeping on this I have an idea. What if we decide to preprocess files based on the presence of a .preprocess.json file in the directory? Directives in this file would apply recursively to all files the containing directory and descendant directories.

I'm thinking the file could contain metadata about how to preprocess files based on their extension. So for the 6to5 case it might look like this:

{
  ".js": ["6to5"]
}

That would run 6to5 on all files with the .js extension. Or if you had specific options to pass to the pre-processor, you could use an expanded form:

{
  ".js": [{
    "name": "6to5",
    "options": {}
  }]
}

The nice thing about a file is that it doesn't affect the directory structure. On package installation we could run the desired preprocessors up-front and then delete the file entirely. If the file isn't there, we pay nothing.

Keying on extension means we could opt in to preprocessing for files other than just .js. Using arrays makes room for multiple preprocessor steps such as sweet.js in the future. Also, I'm thinking that preprocessor settings wouldn't cascade through the node_modules directory.

What do you guys think?

Contributor

nathansobo commented Jan 29, 2015

After sleeping on this I have an idea. What if we decide to preprocess files based on the presence of a .preprocess.json file in the directory? Directives in this file would apply recursively to all files the containing directory and descendant directories.

I'm thinking the file could contain metadata about how to preprocess files based on their extension. So for the 6to5 case it might look like this:

{
  ".js": ["6to5"]
}

That would run 6to5 on all files with the .js extension. Or if you had specific options to pass to the pre-processor, you could use an expanded form:

{
  ".js": [{
    "name": "6to5",
    "options": {}
  }]
}

The nice thing about a file is that it doesn't affect the directory structure. On package installation we could run the desired preprocessors up-front and then delete the file entirely. If the file isn't there, we pay nothing.

Keying on extension means we could opt in to preprocessing for files other than just .js. Using arrays makes room for multiple preprocessor steps such as sweet.js in the future. Also, I'm thinking that preprocessor settings wouldn't cascade through the node_modules directory.

What do you guys think?

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

OK, I think we're agreed that categorically transpiling all .js files is a bad idea and that we would like to be able to robustly pre-transpile code where possible to help improve startup times.

@nathansobo's proposal generally sounds good to me since it seems to achieve both of those goals. We might want to check upwards for a .preprocess.json file until we find either it or a package.json file, which I think would address the "don't cascase through node_modules" thing. Though I guess if you insist on having a .preprocess.json in your root, then including node_modules would still be problematic. Moving your index.js to src/index.js to avoid this issue doesn't seem like that big of a deal to me, though. (Or we could support a "norecurse" option in .preprocess.json?)

The only hangup I have about supporting the options in the .preprocess.json is that they need to be hashable in order for the path in the compile-cache to be sound. The current iteration of my pull request supports this for the 6to5 options that are used, but is not a general hashing solution for arbitrary JSON. I suppose it wouldn't be that hard to extend it, though.

Contributor

bolinfest commented Jan 29, 2015

OK, I think we're agreed that categorically transpiling all .js files is a bad idea and that we would like to be able to robustly pre-transpile code where possible to help improve startup times.

@nathansobo's proposal generally sounds good to me since it seems to achieve both of those goals. We might want to check upwards for a .preprocess.json file until we find either it or a package.json file, which I think would address the "don't cascase through node_modules" thing. Though I guess if you insist on having a .preprocess.json in your root, then including node_modules would still be problematic. Moving your index.js to src/index.js to avoid this issue doesn't seem like that big of a deal to me, though. (Or we could support a "norecurse" option in .preprocess.json?)

The only hangup I have about supporting the options in the .preprocess.json is that they need to be hashable in order for the path in the compile-cache to be sound. The current iteration of my pull request supports this for the 6to5 options that are used, but is not a general hashing solution for arbitrary JSON. I suppose it wouldn't be that hard to extend it, though.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

The other thing that we probably have to verify is that checking for (and parsing) these .preprocess.json files is that it's "cheap enough."

Contributor

bolinfest commented Jan 29, 2015

The other thing that we probably have to verify is that checking for (and parsing) these .preprocess.json files is that it's "cheap enough."

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Jan 29, 2015

Contributor

The only hangup I have about supporting the options in the .preprocess.json is that they need to be hashable in order for the path in the compile-cache to be sound.

Maybe we start with the simple case of not allowing options and keep that idea in our back pocket?

The other thing that we probably have to verify is that checking for (and parsing) these .preprocess.json files is that it's "cheap enough."

Seems like checking for non-existence of the file should be pretty cheap for the fast-path case.

Contributor

nathansobo commented Jan 29, 2015

The only hangup I have about supporting the options in the .preprocess.json is that they need to be hashable in order for the path in the compile-cache to be sound.

Maybe we start with the simple case of not allowing options and keep that idea in our back pocket?

The other thing that we probably have to verify is that checking for (and parsing) these .preprocess.json files is that it's "cheap enough."

Seems like checking for non-existence of the file should be pretty cheap for the fast-path case.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

"Seems like checking for non-existence of the file should be pretty cheap for the fast-path case."

Right, I guess the question is whether it makes sense to check all the way up to the root of the filesystem, or check for .preprocess.json and then package.json to prevent going all the way to the root.

Contributor

bolinfest commented Jan 29, 2015

"Seems like checking for non-existence of the file should be pretty cheap for the fast-path case."

Right, I guess the question is whether it makes sense to check all the way up to the root of the filesystem, or check for .preprocess.json and then package.json to prevent going all the way to the root.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@sebmck Updated the PR to use the placeholders supported by Chrome's console API, e.g.:

console.error('Error compiling %s: %o', filePath, error)

The error is also re-thrown, so between these two things, I think it should be sufficient for debugging.

Contributor

bolinfest commented Jan 29, 2015

@sebmck Updated the PR to use the placeholders supported by Chrome's console API, e.g.:

console.error('Error compiling %s: %o', filePath, error)

The error is also re-thrown, so between these two things, I think it should be sufficient for debugging.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

The solution we have for CoffeeScript is to use .coffee as your extension and Atom just compiles it on the fly and caches it based on the contents. This same solution is what we've used for Less files as well, a custom extension, no configuration, and caching based on the SHA-1 of the raw file(s).

So my gut would say to just implement this the same exact way, a custom extension (I'd lean towards .es6), caching based on a SHA-1 of the raw content, and every package uses the same options that are controlled by core (just like Less and CoffeeScript).

Adding any configuration outside of the root package.json file will require extra synchronous file reads and JSON parses at startup and requires trickier cache strategies in order for busts to occur when options change but files are the same.

If packages need custom options during development, they can always handle the compilation themselves by opting out of the .es6 extension and doing whatever they need to do to generate .js files.

Member

kevinsawicki commented Jan 29, 2015

The solution we have for CoffeeScript is to use .coffee as your extension and Atom just compiles it on the fly and caches it based on the contents. This same solution is what we've used for Less files as well, a custom extension, no configuration, and caching based on the SHA-1 of the raw file(s).

So my gut would say to just implement this the same exact way, a custom extension (I'd lean towards .es6), caching based on a SHA-1 of the raw content, and every package uses the same options that are controlled by core (just like Less and CoffeeScript).

Adding any configuration outside of the root package.json file will require extra synchronous file reads and JSON parses at startup and requires trickier cache strategies in order for busts to occur when options change but files are the same.

If packages need custom options during development, they can always handle the compilation themselves by opting out of the .es6 extension and doing whatever they need to do to generate .js files.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@kevinsawicki How do you feel about .es since 6to5 supports features beyond those of ES6? (It seems more future proof to me.) It doesn't seem like it's used for many other things: http://filext.com/file-extension/ES.

Having to use a different suffix makes me a little sad, but I admit that it will help us achieve the performance we want and simplify the implementation of this feature.

Contributor

bolinfest commented Jan 29, 2015

@kevinsawicki How do you feel about .es since 6to5 supports features beyond those of ES6? (It seems more future proof to me.) It doesn't seem like it's used for many other things: http://filext.com/file-extension/ES.

Having to use a different suffix makes me a little sad, but I admit that it will help us achieve the performance we want and simplify the implementation of this feature.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

How do you feel about .es since 6to5 supports features beyond those of ES6?

I'm fine with that, I'm really flexible on the extension

Member

kevinsawicki commented Jan 29, 2015

How do you feel about .es since 6to5 supports features beyond those of ES6?

I'm fine with that, I'm really flexible on the extension

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@kevinsawicki Weird, Wikipedia says .erl and .hrl are the extensions.

Contributor

bolinfest commented Jan 29, 2015

@kevinsawicki Weird, Wikipedia says .erl and .hrl are the extensions.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Weird, Wikipedia says .erl and .hrl are the extensions.

Yeah, I have no idea if .es is "right" in the Erlang case, I just always consult that file in that repo when looking up extensions.

Member

kevinsawicki commented Jan 29, 2015

Weird, Wikipedia says .erl and .hrl are the extensions.

Yeah, I have no idea if .es is "right" in the Erlang case, I just always consult that file in that repo when looking up extensions.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@kevinsawicki would .jsx be acceptable?

Contributor

bolinfest commented Jan 29, 2015

@kevinsawicki would .jsx be acceptable?

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@kevinsawicki I've been sourcing suggestions at Facebook. New ones include .njs for "next JavaScript," or .nes for "next ECMAScript." As .njs is similar to "Ninjas," it seems apropos.

Contributor

bolinfest commented Jan 29, 2015

@kevinsawicki I've been sourcing suggestions at Facebook. New ones include .njs for "next JavaScript," or .nes for "next ECMAScript." As .njs is similar to "Ninjas," it seems apropos.

@mark-hahn

This comment has been minimized.

Show comment
Hide comment
@mark-hahn

mark-hahn Jan 29, 2015

Contributor

What is wrong with file.6to5 ?

On Thu, Jan 29, 2015 at 11:10 AM, bolinfest notifications@github.com
wrote:

@kevinsawicki https://github.com/kevinsawicki I've been sourcing
suggestions at Facebook. New ones include .njs for "next JavaScript," or
.nes for "next ECMAScript." As .njs is similar to "Ninjas," it seems
apropos.


Reply to this email directly or view it on GitHub
#5299 (comment).

Contributor

mark-hahn commented Jan 29, 2015

What is wrong with file.6to5 ?

On Thu, Jan 29, 2015 at 11:10 AM, bolinfest notifications@github.com
wrote:

@kevinsawicki https://github.com/kevinsawicki I've been sourcing
suggestions at Facebook. New ones include .njs for "next JavaScript," or
.nes for "next ECMAScript." As .njs is similar to "Ninjas," it seems
apropos.


Reply to this email directly or view it on GitHub
#5299 (comment).

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@mark-hahn It seems to tied to the tool and @sebmck, I thought I saw some talk of changing the project name?

Contributor

bolinfest commented Jan 29, 2015

@mark-hahn It seems to tied to the tool and @sebmck, I thought I saw some talk of changing the project name?

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

We've been discussing this a bit in meatspace as well and are coming around to an extension not being the best solution here.

What do you think about instead just using something like "use strict"; instead that marks what JS transpiler it should go through?

If a .js file starts with "use 6to5"; then it goes through the 6to5 transpiler. That way the natural-feeling .js extension is preserved but it comes with a directive of how to process the content.

And if someday we want to add traceur support, it could just be "use traceur"; at the front of the file.

Member

kevinsawicki commented Jan 29, 2015

We've been discussing this a bit in meatspace as well and are coming around to an extension not being the best solution here.

What do you think about instead just using something like "use strict"; instead that marks what JS transpiler it should go through?

If a .js file starts with "use 6to5"; then it goes through the 6to5 transpiler. That way the natural-feeling .js extension is preserved but it comes with a directive of how to process the content.

And if someday we want to add traceur support, it could just be "use traceur"; at the front of the file.

@mark-hahn

This comment has been minimized.

Show comment
Hide comment
@mark-hahn

mark-hahn Jan 29, 2015

Contributor

I meant use .6to5 as the file extension instead of .es6

On Thu, Jan 29, 2015 at 11:14 AM, bolinfest notifications@github.com
wrote:

@mark-hahn https://github.com/mark-hahn It seems to tied to the tool
and @sebmck https://github.com/sebmck, I thought I saw some talk of
changing the project name?


Reply to this email directly or view it on GitHub
#5299 (comment).

Contributor

mark-hahn commented Jan 29, 2015

I meant use .6to5 as the file extension instead of .es6

On Thu, Jan 29, 2015 at 11:14 AM, bolinfest notifications@github.com
wrote:

@mark-hahn https://github.com/mark-hahn It seems to tied to the tool
and @sebmck https://github.com/sebmck, I thought I saw some talk of
changing the project name?


Reply to this email directly or view it on GitHub
#5299 (comment).

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

FYI, I just pushed an update to the PR that implements a generic function for hashing a value parsed from JSON and included a test.

I'll try adding support for "use 6to5" now and see how that performs.

Contributor

bolinfest commented Jan 29, 2015

FYI, I just pushed an update to the PR that implements a generic function for hashing a value parsed from JSON and included a test.

I'll try adding support for "use 6to5" now and see how that performs.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

I'll try adding support for "use 6to5" now and see how that performs.

Yeah, for this I think making it a requirement that it be the very first line is okay, so I don't think we need to try to skip over comments and what not.

Member

kevinsawicki commented Jan 29, 2015

I'll try adding support for "use 6to5" now and see how that performs.

Yeah, for this I think making it a requirement that it be the very first line is okay, so I don't think we need to try to skip over comments and what not.

Transpile all .js files with 6to5.
In the spirit of supporting JavaScript development for Atom packages,
this adds default support for es.next transpilation support in the way
that Atom already has default support for CoffeeScript transpilation.
There are many new features in ES6+ that make JavaScript development
easier and more enjoyable, particularly in terms of support for async code.

For reference, this was a much faster way to iterate on this than running `./script/build`
each time:

```
cp /Users/mbolin/src/atom/static/index.js /Applications/Atom.app/Contents/Resources/app/static/index.js
coffee --output /Applications/Atom.app/Contents/Resources/app/src --compile /Users/mbolin/src/atom/src/esnext.coffee
```

Run the following in the console to see how warm the cache was after startup:

```
global.require('../src/esnext/').getCacheHits()
global.require('../src/esnext/').getCacheMisses()
```
@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@kevinsawicki Please take a look at my latest version of the PR. Startup times seem speedy again and my file with 'use 6to5'; at the top gets transpiled as expected.

Contributor

bolinfest commented Jan 29, 2015

@kevinsawicki Please take a look at my latest version of the PR. Startup times seem speedy again and my file with 'use 6to5'; at the top gets transpiled as expected.

src/esnext.coffee
+module.exports =
+ register: register
+ getCacheMisses: -> stats.misses
+ getCacheHits: -> stats.hits

This comment has been minimized.

@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Down the road, if you want to see these in Timecop, they can be added here

@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Down the road, if you want to see these in Timecop, they can be added here

This comment has been minimized.

@bolinfest

bolinfest Jan 29, 2015

Contributor

Good idea: I'll put up a separate PR for that once this is in.

@bolinfest

bolinfest Jan 29, 2015

Contributor

Good idea: I'll put up a separate PR for that once this is in.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@kevinsawicki Also, is there any way to do anything like apm test spec/esnext-spec.coffee?

And thanks, yes, https://github.com/atom/atom/pull/5299/files was the thing I was looking for.

Contributor

bolinfest commented Jan 29, 2015

@kevinsawicki Also, is there any way to do anything like apm test spec/esnext-spec.coffee?

And thanks, yes, https://github.com/atom/atom/pull/5299/files was the thing I was looking for.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Also, is there any way to do anything like apm test spec/esnext-spec.coffee?

Make the method you want to run fit intead of it or fdescribe instead of desribe and run Window: Run Package Specs from the command palette to launch the spec window.

The f stands for focused, only specs with an f will run when one is present.

Member

kevinsawicki commented Jan 29, 2015

Also, is there any way to do anything like apm test spec/esnext-spec.coffee?

Make the method you want to run fit intead of it or fdescribe instead of desribe and run Window: Run Package Specs from the command palette to launch the spec window.

The f stands for focused, only specs with an f will run when one is present.

@kevinsawicki kevinsawicki self-assigned this Jan 29, 2015

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Jan 29, 2015

Contributor

Would you be down to rename esnext.coffee 6to5.coffee? It seems like a better match for the library we're using and more clear.

Contributor

nathansobo commented Jan 29, 2015

Would you be down to rename esnext.coffee 6to5.coffee? It seems like a better match for the library we're using and more clear.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

Do all of the commits in the PR get squashed before it is accepted? If so, what should I edit to update the commit message for what will ultimately go in?

Contributor

bolinfest commented Jan 29, 2015

Do all of the commits in the PR get squashed before it is accepted? If so, what should I edit to update the commit message for what will ultimately go in?

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Jan 29, 2015

Contributor

Do all of the commits in the PR get squashed before it is accepted?

We've never done that sort of thing. I'll defer to @kevinsawicki for the merge decision.

Contributor

nathansobo commented Jan 29, 2015

Do all of the commits in the PR get squashed before it is accepted?

We've never done that sort of thing. I'll defer to @kevinsawicki for the merge decision.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

Do all of the commits in the PR get squashed before it is accepted?

Nope, we just click the merge button, so they end just how they look in the Commits tab of this PR.

Member

kevinsawicki commented Jan 29, 2015

Do all of the commits in the PR get squashed before it is accepted?

Nope, we just click the merge button, so they end just how they look in the Commits tab of this PR.

@bolinfest

This comment has been minimized.

Show comment
Hide comment
@bolinfest

bolinfest Jan 29, 2015

Contributor

@kevinsawicki Hrm, ok, I'll be more cognizant of that fact in the future. The commit at the bottom of the stack has references to esnext that are no longer accurate. I'm happy to sqaush 'n scrub or just leave things as-is.

Contributor

bolinfest commented Jan 29, 2015

@kevinsawicki Hrm, ok, I'll be more cognizant of that fact in the future. The commit at the bottom of the stack has references to esnext that are no longer accurate. I'm happy to sqaush 'n scrub or just leave things as-is.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Jan 29, 2015

Member

I'm happy to sqaush 'n scrub or just leave things as-is.

I'd say just leave it, it's all about the journey 🚵

Member

kevinsawicki commented Jan 29, 2015

I'm happy to sqaush 'n scrub or just leave things as-is.

I'd say just leave it, it's all about the journey 🚵

package.json
@@ -19,6 +19,7 @@
],
"atomShellVersion": "0.21.0",
"dependencies": {
+ "6to5-core": "3.0.14",

This comment has been minimized.

@kittens

kittens Jan 29, 2015

Contributor

Might be a good idea to have this as ^3.0.14 as I push out releases pretty often and don't want atom users to be disadvantaged.

@kittens

kittens Jan 29, 2015

Contributor

Might be a good idea to have this as ^3.0.14 as I push out releases pretty often and don't want atom users to be disadvantaged.

This comment has been minimized.

@nathansobo nathansobo changed the title from Transpile all .js files under a package's `atom-es` directory with 6to5. to Transpile all .js files beginning with the `"use 6to5";` pragma with 6to5. Jan 30, 2015

@nathansobo nathansobo changed the title from Transpile all .js files beginning with the `"use 6to5";` pragma with 6to5. to Transpile all .js files beginning with the "use 6to5"; pragma with 6to5. Jan 30, 2015

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Jan 30, 2015

Contributor

Updated the title of the PR for clarity.

Contributor

nathansobo commented Jan 30, 2015

Updated the title of the PR for clarity.

@paulcbetts

This comment has been minimized.

Show comment
Hide comment
@paulcbetts

paulcbetts Jan 30, 2015

Contributor

Pretty excited about this PR, great work @bolinfest!

Contributor

paulcbetts commented Jan 30, 2015

Pretty excited about this PR, great work @bolinfest!

@paulcbetts paulcbetts referenced this pull request in atom-archive/electron-starter Feb 1, 2015

Merged

Enable JavaScript ES6 #35

@kittens kittens referenced this pull request in babel/babel Feb 2, 2015

Closed

Name suggestions #596

kevinsawicki added a commit that referenced this pull request Feb 2, 2015

Merge pull request #5299 from bolinfest/6to5
Transpile all .js files beginning with the "use 6to5"; pragma with 6to5.

@kevinsawicki kevinsawicki merged commit 8365ccb into atom:master Feb 2, 2015

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Feb 2, 2015

Member

🚢 📈

Member

kevinsawicki commented Feb 2, 2015

🚢 📈

@paulcbetts

This comment has been minimized.

Show comment
Hide comment
@paulcbetts

paulcbetts Feb 2, 2015

Contributor

@bolinfest Are you going to do a follow-up with build precompilation for non-dev mode? That would be 😍 💎

Contributor

paulcbetts commented Feb 2, 2015

@bolinfest Are you going to do a follow-up with build precompilation for non-dev mode? That would be 😍 💎

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Feb 2, 2015

Member

Are you going to do a follow-up with build precompilation for non-dev mode?

apm install will automatically warm the cache at the end of install.

Member

kevinsawicki commented Feb 2, 2015

Are you going to do a follow-up with build precompilation for non-dev mode?

apm install will automatically warm the cache at the end of install.

@paulcbetts

This comment has been minimized.

Show comment
Hide comment
@paulcbetts

paulcbetts Feb 2, 2015

Contributor

@bolinfest @kevinsawicki I would still take a PR for that for atom/atom-shell-starter if you're Feeling Brave, so that people can write Atom Shell apps in ES6

Contributor

paulcbetts commented Feb 2, 2015

@bolinfest @kevinsawicki I would still take a PR for that for atom/atom-shell-starter if you're Feeling Brave, so that people can write Atom Shell apps in ES6

@kgrossjo

This comment has been minimized.

Show comment
Hide comment
@kgrossjo

kgrossjo Feb 3, 2015

Oh yes! Writing Atom Shell apps in ES6 would be awesome!

kgrossjo commented Feb 3, 2015

Oh yes! Writing Atom Shell apps in ES6 would be awesome!

@paulcbetts

This comment has been minimized.

Show comment
Hide comment
@paulcbetts

paulcbetts Feb 3, 2015

Contributor

@kgrossjo I Think So Too! atom/atom-shell-starter already has support for it today in live-compile mode!

Contributor

paulcbetts commented Feb 3, 2015

@kgrossjo I Think So Too! atom/atom-shell-starter already has support for it today in live-compile mode!

@steelbrain

This comment has been minimized.

Show comment
Hide comment
@steelbrain

steelbrain Feb 4, 2015

Contributor

I have a question, If Atom has switched to IoJs [1], then why do we need to transpile ES6, Can't we use it directly?
And If we really want to use a transpiler, Then why not use the react-tools transpiler. It supports ES6 stuff, as well as TypeHinting. (I use typehinting in all my JS projects, it makes stuff easier for Humans & IDEs)

Contributor

steelbrain commented Feb 4, 2015

I have a question, If Atom has switched to IoJs [1], then why do we need to transpile ES6, Can't we use it directly?
And If we really want to use a transpiler, Then why not use the react-tools transpiler. It supports ES6 stuff, as well as TypeHinting. (I use typehinting in all my JS projects, it makes stuff easier for Humans & IDEs)

@LegNeato

This comment has been minimized.

Show comment
Hide comment
@LegNeato

LegNeato Feb 4, 2015

Because the ES6 (and ES7!) support is uneven and will change as we update V8. This insulates package authors from that while letting them use more than native V8 support today.

@bolinfest will have to speak to react-tools but seeing how we are at FB and and work together with the React folks on tooling there likely is a good reason :-)

LegNeato commented Feb 4, 2015

Because the ES6 (and ES7!) support is uneven and will change as we update V8. This insulates package authors from that while letting them use more than native V8 support today.

@bolinfest will have to speak to react-tools but seeing how we are at FB and and work together with the React folks on tooling there likely is a good reason :-)

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 4, 2015

Contributor

@steelbrain You check out a compatibility table here for a list of supported features. react-tools and io.js don't even come close. 6to5 supports JSX out of the box and is completely compliant with the JSX transformer in the official react-tools.

Contributor

kittens commented Feb 4, 2015

@steelbrain You check out a compatibility table here for a list of supported features. react-tools and io.js don't even come close. 6to5 supports JSX out of the box and is completely compliant with the JSX transformer in the official react-tools.

@eyko

This comment has been minimized.

Show comment
Hide comment
@eyko

eyko Feb 4, 2015

Isn't anybody worried about adding such a specific implementation mode to all your es6 files? The user's (or team's) current transpiler preference shouldn't be reflected in the source code. This is something that should be decided by the build system and maybe specified via a setting (either in the atom settings panel or by reading package.json or whatever).

I'm not sure how much sense it makes to switch from 'use 6to5'; to 'use traceur'; when suddenly people decide they prefer traceur. It's a big search/replace job and it pollutes the commit history with what is essentially change in the build config. I personally think it would make more sense to just use 'use es6'; or something along those lines, and then abstract away the implementation details. If someday Atom decides to support switching between 6to5 and traceur, developers could simply add an entry in package.json. It also makes it easier to temporarily switch transpilers for testing/curiosity/whatever reasons.

eyko commented Feb 4, 2015

Isn't anybody worried about adding such a specific implementation mode to all your es6 files? The user's (or team's) current transpiler preference shouldn't be reflected in the source code. This is something that should be decided by the build system and maybe specified via a setting (either in the atom settings panel or by reading package.json or whatever).

I'm not sure how much sense it makes to switch from 'use 6to5'; to 'use traceur'; when suddenly people decide they prefer traceur. It's a big search/replace job and it pollutes the commit history with what is essentially change in the build config. I personally think it would make more sense to just use 'use es6'; or something along those lines, and then abstract away the implementation details. If someday Atom decides to support switching between 6to5 and traceur, developers could simply add an entry in package.json. It also makes it easier to temporarily switch transpilers for testing/curiosity/whatever reasons.

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 4, 2015

Contributor

@eyko You seem to be conflating 6to5 support with just ES6 support. 6to5 supports so much more.

Contributor

kittens commented Feb 4, 2015

@eyko You seem to be conflating 6to5 support with just ES6 support. 6to5 supports so much more.

@eyko

This comment has been minimized.

Show comment
Hide comment
@eyko

eyko Feb 4, 2015

@sebmck it was an example, I do know that 6to5 is not strictly ES6. Call it "use esnext"; if you would, hopefully you get my point.

eyko commented Feb 4, 2015

@sebmck it was an example, I do know that 6to5 is not strictly ES6. Call it "use esnext"; if you would, hopefully you get my point.

@benogle

This comment has been minimized.

Show comment
Hide comment
@benogle

benogle Feb 4, 2015

Contributor

Isn't anybody worried about adding such a specific implementation mode to all your es6 files?

We talked about this quite a bit. We wanted to be more general about the naming. "use esnext"; would have been nice, but would have been misleading since the chosen transpiler supports only a subset of es6, and even some of es7. With a generic pragma, the author wouldnt know which subset was available, and may expect that all of es6 or es7 is supported. Then what if we decided to change the transpiler under the hood? It would have different levels of support and code written against the original transpiler may not work, or work differently.

With "use 6to5"; you know exactly what you're getting. If we decide to support traceur in the future, it can be alongside 6to5, and you can migrate when you're ready.

I agree it sucks to have the a pragma tied to a specific tool, but we believe it is the best way forward given the spotty support for es6.

Contributor

benogle commented Feb 4, 2015

Isn't anybody worried about adding such a specific implementation mode to all your es6 files?

We talked about this quite a bit. We wanted to be more general about the naming. "use esnext"; would have been nice, but would have been misleading since the chosen transpiler supports only a subset of es6, and even some of es7. With a generic pragma, the author wouldnt know which subset was available, and may expect that all of es6 or es7 is supported. Then what if we decided to change the transpiler under the hood? It would have different levels of support and code written against the original transpiler may not work, or work differently.

With "use 6to5"; you know exactly what you're getting. If we decide to support traceur in the future, it can be alongside 6to5, and you can migrate when you're ready.

I agree it sucks to have the a pragma tied to a specific tool, but we believe it is the best way forward given the spotty support for es6.

@eyko

This comment has been minimized.

Show comment
Hide comment
@eyko

eyko Feb 4, 2015

Okay, so that's a very convincing argument with which I agree 👍, I guess. (now I'm wondering what this will mean for developers already using other pragmas but wanting to take advantage of the build feature...)

eyko commented Feb 4, 2015

Okay, so that's a very convincing argument with which I agree 👍, I guess. (now I'm wondering what this will mean for developers already using other pragmas but wanting to take advantage of the build feature...)

@josh

This comment has been minimized.

Show comment
Hide comment
@josh

josh Feb 4, 2015

Contributor

FWIW, Rails 5 + Sprockets is shipping 6to5 with the .es6 extension approach. The extension name maybe subject to change, but for performance reasons detecting with just the filename available works better than a contents scan. Not that Atom needs to do the same thing, but just wanted to mention that.

Contributor

josh commented Feb 4, 2015

FWIW, Rails 5 + Sprockets is shipping 6to5 with the .es6 extension approach. The extension name maybe subject to change, but for performance reasons detecting with just the filename available works better than a contents scan. Not that Atom needs to do the same thing, but just wanted to mention that.

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Feb 4, 2015

Contributor

@josh Yeah we went back and forth on it. The main issue is we couldn't come up with an extension that felt truthful, since 6to5 isn't necessarily ES6. An extension does have the advantage of being detectable from the file path alone, but since since reading the file anyway in every circumstance in which it matters, looking at the first line isn't an issue.

Contributor

nathansobo commented Feb 4, 2015

@josh Yeah we went back and forth on it. The main issue is we couldn't come up with an extension that felt truthful, since 6to5 isn't necessarily ES6. An extension does have the advantage of being detectable from the file path alone, but since since reading the file anyway in every circumstance in which it matters, looking at the first line isn't an issue.

@kittens kittens referenced this pull request in babel/babel Feb 15, 2015

Closed

Name change #780

45 of 45 tasks complete

@basarat basarat referenced this pull request Mar 8, 2015

Merged

Transpile all .ts files #5898

2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment