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

Fix parsing typescript function types with destructuring #9035

Merged
merged 2 commits into from Nov 19, 2018

Conversation

danez
Copy link
Member

@danez danez commented Nov 17, 2018

Q                       A
Fixed Issues? Fixes #8745, fixes #8099
Patch: Bug Fix? y
Major: Breaking Change? n
Minor: New Feature? n
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

Allows parsing of destructured params in function type declarations.

@danez danez added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: parser area: typescript labels Nov 17, 2018
@babel-bot
Copy link
Collaborator

babel-bot commented Nov 17, 2018

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9401/

@@ -747,6 +753,22 @@ export default (superClass: Class<Parser>): Class<Parser> =>
this.next();
return true;
}

if (this.match(tt.braceL)) {
const braceStack = [tt.braceL];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be an integer instead of an array, since the check only uses its length.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

const braceStack = [tt.braceL];
this.next();

while (braceStack.length > 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to skip the code between braces without parsing it?

e.g., wouldn't this loop accept something like {bar: {}{foo = 2=>{} }}?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does ignore everything, even if its invalid, but this function is only used in a lookahead() and it's job is to try to skip the first param after (.

it is used here:

tsIsUnambiguouslyStartOfFunctionType(): boolean {

and this in turn is used here:

tsIsStartOfFunctionType() {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok 👍

Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 👍

@danez danez merged commit a2afb97 into master Nov 19, 2018
@nicolo-ribaudo nicolo-ribaudo deleted the typescript-fix-function-types branch November 19, 2018 22:39
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TypeScript] Unexpected token, expected ")" Syntax error with a TS interface using syntax-typescript
4 participants