Enable await in ES5 and ES2015 script mode#5852
Conversation
Even though strictly generators are an ES6 feature the real world support is large enough to use the feature in well known environments like node.js or Electron app. Since the previous output was not working at all anyway it feels like a good compromise to at least emit working code while still having the warning in place. The user would also need to add "use strict" on top of her .ts file to make it work with node.js.
|
Hi @holtwick, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
There was a problem hiding this comment.
Here is the actual change. Sorry, my IDE modified some whitespace ;)
There was a problem hiding this comment.
please change the condition to just if (isAsync) and i would appreciate it if you fix the indentation issue.
|
I disagree. When targeting ES5, only ES5 should be emitted, not ES5.5. This is related to the issue #4692, Granular Targeting. Your best bet would be to target ES6 and use babel to transpile just what you need to (transpile classes, but leave generators as is). |
|
@lbguilherme thanks for pointing to #4692. Indeed some target specific code emission sounds reasonable to me. Using Babel or the like instead as a second pass sounds like overkill and probably adds another source of failures for the end user. As I mentioned before, the transpiler currently emits completely invalid code anyway. So this change would not harm and the error is still presented in my minimal change. To illustrate what actually changes in the emitted code. This would be TS: With ES5 option the current implementation would emit: This code can fail for 2 reasons: 1. Because Instead with this patch applied you get the same output as for ES6, which is: The IMHO it would be great to have that little change, because for projects only targetting node.js, Electron or CEF this is a huge win. |
|
@holtwick, stupid question, why are you emitting to ES5 and not ES6 if you are running on node v4+ or Electron? |
|
@mhegazy, some ES6 features are not supported by node or Chrome/CEF/Electron right now, like default parameters |
|
not sure this is the right solution though. we make no guarantees in the case of errors. the correct fix is to address #4692. i am open to taking this PR, but i do not think it solves the issue, it just kicks the can down the road until you run into the next blocking issue. |
|
As I see from https://github.com/Microsoft/TypeScript/wiki/Roadmap async/await support is on the roadmap for 2.0 anyway? That's great news. I agree that my patch is just a quick fix and that a solution in the context of #4692 or adding some other kind of other flag would be a more elegant and conform solution. Although this little patch would already get most use cases satisfied without any major drawbacks, since the errors are still in place. So if you could merge it, I would be very grateful. If you like to I can elaborate more on the topic and present a follow up PR. For example the next issue down the road would be the use of |
|
ok then, I had one comment, please update the PR. |
|
Done. Thanks a lot. |
Enable await in ES5 and ES2015 script mode
|
thanks! |
Even though strictly generators are an ES6 feature the real world support is large enough to use the feature in well known environments like node.js or Electron app.
Since the previous output was not working at all anyway it feels like a good compromise to at least emit working code while still having the warning in place.