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(parser): Handle trailing comma and bracket after arrow fn in conditional in TS #3685

Merged

Conversation

williamtetlow
Copy link
Contributor

@williamtetlow williamtetlow commented Feb 22, 2022

Description:
fixes #3672

fixes syntax

// case 1
{
  prop: a ? (): void => { } : (): void => { },
}

// case 2
f(a ? (): void => { } : (): void => { })

BREAKING CHANGE:

Related issue (if exists):
#3672

@kdy1 kdy1 added this to the v1.2.144 milestone Feb 22, 2022
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Thank you!


swc-bump:

  • swc_ecma_parser

@kdy1 kdy1 enabled auto-merge (squash) February 22, 2022 19:35
@kdy1 kdy1 disabled auto-merge February 22, 2022 19:39
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

CI is failing due to parsing error.
I think we should use some context to see if we can eat ,

@williamtetlow
Copy link
Contributor Author

Oops missed that, let me take a look

@kdy1 kdy1 modified the milestones: v1.2.144, v1.2.145 Feb 22, 2022
@magic-akari
Copy link
Member

could you add another test case?

f(a ? (): void => { } : (): void => { })

Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Can you add the test?

@williamtetlow
Copy link
Contributor Author

Yep will do. Had a busy day with work yesterday but will do it today

@kdy1 kdy1 removed this from the v1.2.145 milestone Feb 24, 2022
@williamtetlow
Copy link
Contributor Author

@kdy1 sorry busy week with work only just got round to fixing this.

Added test requested by @magic-akari in f6f0623

Also had to remove test for #2174. This is actually invalid syntax in TS see https://www.typescriptlang.org/play?#code/MYewdgzgLgBAHjAvDA3gKBpmAHATibALhgEsIBhEXXAU2CgyyYH4YAzMJAPhgAoUYAXwCUjJuJjEO3GFFwBXGgBo0ggNxA

@williamtetlow
Copy link
Contributor Author

williamtetlow commented Feb 25, 2022

Hmm is exec_tests__deno_exec__deno_9464__case1__input__entry_ts a bit flaky? I'm getting overflowed stack error on CI but works locally?

https://github.com/swc-project/swc/runs/5331608290?check_suite_focus=true

Edit:
Looks like it might be, worked second time 🙂

@williamtetlow williamtetlow changed the title fix(parser): Handle trailing comma after arrow fn in TS fix(parser): Handle trailing comma and bracket after arrow fn in TS Feb 25, 2022
@williamtetlow williamtetlow changed the title fix(parser): Handle trailing comma and bracket after arrow fn in TS fix(parser): Handle trailing comma and bracket after arrow fn in conditional in TS Feb 25, 2022
@@ -1,5 +0,0 @@
const x = {
Copy link
Member

Choose a reason for hiding this comment

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

Why did you delete this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kdy1 I commented above. This isn't valid TS syntax

Also had to remove test for #2174. This is actually invalid syntax in TS see https://www.typescriptlang.org/play?#code/MYewdgzgLgBAHjAvDA3gKBpmAHATibALhgEsIBhEXXAU2CgyyYH4YAzMJAPhgAoUYAXwCUjJuJjEO3GFFwBXGgBo0ggNxA

Copy link
Contributor

Choose a reason for hiding this comment

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

It's valid JavaScript though. This is just a failing of the TS compiler.

@kdy1
Copy link
Member

kdy1 commented Feb 25, 2022

Sorry, I missed it. Thanks!

Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Thank you!


swc-bump:

  • swc_ecma_parser

@kdy1 kdy1 added this to the v1.2.146 milestone Feb 25, 2022
@kdy1 kdy1 enabled auto-merge (squash) February 25, 2022 12:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Parser error on TypeScript function with return in a ternary falsy case
4 participants