-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Emit arrow function es6 #1627
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
Emit arrow function es6 #1627
Conversation
@@ -3210,6 +3210,13 @@ module ts { | |||
emitTrailingComments(node); | |||
} | |||
|
|||
function isES6ArrowFunction(node: FunctionLikeDeclaration): boolean { | |||
if (node.kind === SyntaxKind.ArrowFunction && compilerOptions.target >= ScriptTarget.ES6) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return node.kind === ... && compilerOptions.target ...;
5b1cc60
to
cb48a5e
Compare
@@ -0,0 +1,29 @@ | |||
//// [emitArrowFunction.ts] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be really helpful if the baseline stated the target, just like the original test.
Are you still working off of this branch @yuit? |
@DanielRosenwasser Yes. Fixing up thing now. Almost done |
Address #1609. I think we should address this issue about arguments in Typescript Spec and align it with ES6 |
// arguments objects will be inner bound while emitting arrow function natively in ES6, arguments objects | ||
// will be bound to non-arrow function that contain this arrow function. This results in inconsistent bahaviour. | ||
// To avoid that we will give an error to users if they use arguments objects in arrow function so that they | ||
// can explicitly bound arguments objects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tabs -> Spaces
needToCaptureLexicalThis = true; | ||
|
||
// When targeting es6, arrow function lexically bind "this" so we do not need to do the work of binding "this" in emitted code | ||
if (compilerOptions.target >= ScriptTarget.ES6) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reduce this to one branch, it's already set to false
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needToCaptureLexicalThis = compilerOptions.target >= ScriptTarget.ES6
@@ -3263,6 +3263,10 @@ module ts { | |||
emitTrailingComments(node); | |||
} | |||
|
|||
function isES6ArrowFunction(node: FunctionLikeDeclaration): boolean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just consolidate this into a local variable.
Never mind, that's not feasible.
function emitSignatureParametersES6(node: FunctionLikeDeclaration) { | ||
// Check the node's parameters whether it contains flags indicating that it has no parenthesis around the parameters | ||
// Preserver no-parenthesis | ||
if (node && node.flags & NodeFlags.SimpleArrowFunction) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parens around the & expression please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also why would node
be undefined
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with you on that. I just check to make sure but you are right this shouldn't happen
@@ -3303,6 +3313,20 @@ module ts { | |||
decreaseIndent(); | |||
} | |||
|
|||
function emitSignatureParametersForArrow(node: FunctionLikeDeclaration) { | |||
// Check whether the parameter list needs parentheses and preserve no-parenthesis | |||
if (node.flags & NodeFlags.SimpleArrowFunction) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i do not think you need this flag.
|
@DanielRosenwasser Yes they are tabs |
👍 |
@basarat I have updated the error message in the last few commits |
sorry. my bad. |
@basarat no worry :) Thanks for code reviewing! |
Emit Arrow function natively in ES6 and capture "this"