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(es/resolver): Handle TsInterfaceDecl and UsingDecl correctly #8403

Merged
merged 5 commits into from
Dec 8, 2023
Merged

fix(es/resolver): Handle TsInterfaceDecl and UsingDecl correctly #8403

merged 5 commits into from
Dec 8, 2023

Conversation

l4yton
Copy link
Contributor

@l4yton l4yton commented Dec 7, 2023

@l4yton l4yton requested a review from a team as a code owner December 7, 2023 20:23
@CLAassistant
Copy link

CLAassistant commented Dec 7, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@l4yton
Copy link
Contributor Author

l4yton commented Dec 7, 2023

This should fix the issue, but I'm not sure how to properly add tests for this.

@kdy1
Copy link
Member

kdy1 commented Dec 7, 2023

First of all, thank you for the contribution!

For testing, you can add a file named input.js to somewhere in https://github.com/swc-project/swc/tree/35fb6528f54b11192dd967297294cdc1852aa57b/crates/swc_ecma_transforms_base/tests/ts-resolver and run UPDATE=1 cargo test from your terminal.

Note that you may need to touch https://github.com/swc-project/swc/blob/35fb6528f54b11192dd967297294cdc1852aa57b/crates/swc_ecma_transforms_base/tests/ts_resolver.rs

@kdy1
Copy link
Member

kdy1 commented Dec 7, 2023

Also, can you sign the CLA?

@l4yton
Copy link
Contributor Author

l4yton commented Dec 7, 2023

Sure, will sign the CLA. Just noticed that this fix doesn't work if typescript is true when using resolver(Mark::new(), Mark::new(), true), though it does if it's false...

@l4yton l4yton marked this pull request as draft December 7, 2023 21:49
@l4yton l4yton marked this pull request as ready for review December 7, 2023 22:36
@kdy1 kdy1 added this to the Planned milestone Dec 8, 2023
@kdy1 kdy1 requested review from magic-akari and Austaras December 8, 2023 04:00
@kdy1
Copy link
Member

kdy1 commented Dec 8, 2023

I requested review from other team members because I'm not sure about the changes

@Austaras
Copy link
Member

Austaras commented Dec 8, 2023

This looks fine. What concerns you?

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_transforms_base

}

let mark = self.current.mark;

self.current
Copy link
Member

Choose a reason for hiding this comment

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

Removal of this logic seems dangerous to me at that time, but now I'm not sure why I feel it dangerous.

@kdy1 kdy1 changed the title fix(es/transforms): Handle TsInterfaceDecl and UsingDecl correctly fix(es/resolver): Handle TsInterfaceDecl and UsingDecl correctly Dec 8, 2023
@kdy1 kdy1 enabled auto-merge (squash) December 8, 2023 13:19
Copy link
Collaborator

@swc-bot swc-bot left a comment

Choose a reason for hiding this comment

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

Automated review comment generated by auto-rebase script

@kdy1 kdy1 merged commit f8ce316 into swc-project:main Dec 8, 2023
277 checks passed
@kdy1 kdy1 modified the milestones: Planned, v1.3.101 Dec 18, 2023
@swc-project swc-project locked as resolved and limited conversation to collaborators Jan 17, 2024
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.

TypeScript identifier resolver incorrectly handles block scopes
5 participants