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

[ts] allow redeclaring a var/type with the same name as import #14900

Merged
merged 8 commits into from Sep 13, 2022

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Sep 1, 2022

Q                       A
Fixed Issues? fixes #10015
Patch: Bug Fix?
Tests Added + Pass?
License MIT

Doesn't seem to fix the original issue, but fixes #10015 (comment)
Also found new similar bugs, this is in the second commit.
Not sure if flow is affected, but I guess I can ignore it for now.

This also fixes #14795 which has been closed.
repl
repl2

@liuxingbaoyu liuxingbaoyu added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: parser area: typescript labels Sep 1, 2022
@babel-bot
Copy link
Collaborator

babel-bot commented Sep 1, 2022

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

@liuxingbaoyu
Copy link
Member Author

@liuxingbaoyu
Copy link
Member Author

liuxingbaoyu commented Sep 1, 2022

ts repl
babel repl

Seems to expose another bug?

@JLHwung
Copy link
Contributor

JLHwung commented Sep 1, 2022

It seems to me that type Global = ... can shadow import type { Global } from "./module", we may need a new binding type.

@liuxingbaoyu
Copy link
Member Author

liuxingbaoyu commented Sep 1, 2022

Yes, it seems we can fix the original problem at the same time, which is to treat import as a new type.

ts repl
babel repl

@liuxingbaoyu
Copy link
Member Author

liuxingbaoyu commented Sep 2, 2022

I haven't written complex ts types, it would be great if someone could help add some tests!😃

✘ 4 valid programs parsed without error (in violation of the allowlist file)

looks good!

@liuxingbaoyu liuxingbaoyu marked this pull request as ready for review September 2, 2022 16:30
@liuxingbaoyu liuxingbaoyu changed the title fix: ts cannot redeclare a variable with the same name as type import fix: ts cannot redeclare a var/type with the same name as import Sep 2, 2022
@JLHwung
Copy link
Contributor

JLHwung commented Sep 2, 2022

✘ 4 valid programs parsed without error (in violation of the allowlist file)

That's great! Can you run node scripts/parser-tests/typescript --update-allowlist?

@liuxingbaoyu
Copy link
Member Author

✘ 4 valid programs parsed without error (in violation of the allowlist file)

That's great! Can you run node scripts/parser-tests/typescript --update-allowlist?

OK, I'll do this after the review is complete.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo 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!

@nicolo-ribaudo nicolo-ribaudo changed the title fix: ts cannot redeclare a var/type with the same name as import [ts] allow redeclaring a var/type with the same name as import Sep 13, 2022
@nicolo-ribaudo nicolo-ribaudo merged commit 642adbc into babel:main Sep 13, 2022
@liuxingbaoyu
Copy link
Member Author

Please don't merge yet, I found that the allowlist comment seems to be missing a bit, I'll take a look later.

@nicolo-ribaudo
Copy link
Member

Too late 😬

@liuxingbaoyu
Copy link
Member Author

Nevermind, I will open a new PR.😃

@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Dec 13, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 13, 2022
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
4 participants