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

Optional transpilation with Babel #475

Merged
merged 5 commits into from
Jul 24, 2015
Merged

Optional transpilation with Babel #475

merged 5 commits into from
Jul 24, 2015

Conversation

nycdotnet
Copy link
Contributor

I became despondent the other day when I saw that the let keyword's special scoping rules in for-loop iterators wouldn't be transpiled properly by TypeScript 1.5 to ES5. microsoft/TypeScript#3915

While I am pleased that @danquirk seems amenable to implementing this in TypeScript, and I agree with @basarat that doing so is the best long-term solution, it could take some time.

Since this feature of let is transpiled properly by Babel today, I wondered if I could put together an enhancement for atom-typescript to optionally run the JS emitted by the TypeScript language service through Babel during the compile-on-save process. As Bas predicted, it was actually fairly straight-forward, and this pull request is the result.

This PR utilizes a new optional key in tsconfig.json:

{
    "externalTranspiler": "babel"
}

If key exists exactly as above, the emitted JS from the TS language service will be run through Babel; if not, it's not.

I would love your feedback on this. Bas - would you consider merging? I am open to any and all suggestions with regards to the config file, key names, etc.

ToDo:

  • Transpilation with Babel works based on the presence of a key in tsconfig.json.
  • Confirm source maps and other options are passed appropriately
  • Document
  • Test install/uninstall atom-typescript works as expected. (Everything worked when I deleted the contents of my %userprofile%\.atom\packages\atom-typescript\node_modules folder and then ran apm install, so I am hopeful this is fine.

@basarat
Copy link
Member

basarat commented Jul 18, 2015

@nycdotnet I'd be happy to merge and support this till typescript has emits for "let with closure" as well as "async await"

Till then typescript currently supports both these things with target es6. I think it provides great value to allow people to chain the typescript type checker and babel transpiler ie others have asked for it as well. So merge at will and thanks! 🌹

@basarat
Copy link
Member

basarat commented Jul 18, 2015

And the name you have chosen seems to be worth supporting till infinity!

@johnnyreilly
Copy link
Member

+1 for this!

@nycdotnet
Copy link
Contributor Author

@basarat I believe that this is ready. It actually seems to work shockingly smoothly for something I coded, especially with the fixes you made yesterday. Would you please eyeball this and merge/release if you feel happy?

Note: I'm not quite sure of the best way to test uninstall/reinstall of atom-typescript without actually releasing it. Are all the modules referenced in package.json always automatically downloaded by a package?

@nycdotnet nycdotnet changed the title [WIP] Optional transpilation with Babel Optional transpilation with Babel Jul 23, 2015
@basarat
Copy link
Member

basarat commented Jul 24, 2015

Are all the modules referenced in package.json always automatically downloaded by a package

node_modules? Yes. As a part of apm update which is what the users use (either manually or through the settings view). For us (git clone users), we need to run npm install as mentioned : https://github.com/TypeStrong/atom-typescript/blob/master/CONTRIBUTING.md#pull (could be improved on second look)

@basarat
Copy link
Member

basarat commented Jul 24, 2015

:FANCY:

basarat added a commit that referenced this pull request Jul 24, 2015
@basarat basarat merged commit f2a55cf into master Jul 24, 2015
@basarat basarat deleted the external-transpiler branch July 24, 2015 00:25
@basarat
Copy link
Member

basarat commented Jul 24, 2015

Pushed with 5.2.0. Lets see how it goes in userland 👍

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

Successfully merging this pull request may close these issues.

3 participants