-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Add support for parsing binding patterns in arrow function parameters #2680
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
Conversation
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.
can we add a --target=es6 test as well
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.
Rather, take all of these tests and duplicate them as ES6 tests.
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.
Already done.
|
👍 |
|
I simplified the look-ahead to go no more than 3 tokens to find some cases where we can shortcut out with |
src/compiler/parser.ts
Outdated
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.
Thse seem so rare as to not need special handling. I would just do: if (second === OpenBracket || second === OpenBrace) ... return TriState.Unknown
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.
True, I picture arrow functions and destructuring parameters to not be very common in conjunction.
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 picture arrow functions and destructuring parameters to not be very common in conjunction.
I tried writing one 2 days ago for the record.
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'm seeing it often in conjunction with Promise.all:
Promise.all([a, b, c]).then(([a, b, c]) => {
});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.
But won't your new code return ' unknown' on this case? I'm curious what cases were Will see that are common where we'll be able to effectively bail out early with this code b
-- Cyrus
From: Ron Bucktonmailto:notifications@github.com
Sent: 4/10/2015 9:20 AM
To: Microsoft/TypeScriptmailto:TypeScript@noreply.github.com
Cc: Cyrus Najmabadimailto:cyrusn@microsoft.com
Subject: Re: [TypeScript] Add support for parsing binding patterns in arrow function parameters (#2680)
In src/compiler/parser.tshttps://github.com//pull/2680#discussion_r28158593:
@@ -3221,6 +3221,37 @@ module ts {
}
}
// If encounter "([", this could be the start of an array binding pattern.// Examples:// ([ x ]) => { }// ([ x ])if (second === SyntaxKind.OpenBracketToken) {
I'm seeing it often in conjunction with Promise.all:
Promise.all([a, b, c]).then(([a, b, c]) => {
});
—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2680/files#r28158593.
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 went ahead and just made OpenBrace/OpenBracket return Tristate.Unknown. The tryParse will already be able to detect this after reading two tokens, so its not much of an added cost to be worth it.
Add support for parsing binding patterns in arrow function parameters
Fixes #2678, adds some additional tests to the
isParenthesizedArrowFunctionExpressionWorkerlookahead to allow for array and object patterns in the first parameter of a parenthesized arrow function.