Skip to content

Parse reserved identifiers as dotted names of ambient modules #55282

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Aug 28, 2023
Merged

Parse reserved identifiers as dotted names of ambient modules #55282

merged 4 commits into from
Aug 28, 2023

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented Aug 6, 2023

fixes #7840

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Aug 6, 2023
@sandersn sandersn self-assigned this Aug 24, 2023
sandersn
sandersn requested changes Aug 24, 2023
View reviewed changes
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Do we really need this fix? Namespaces are rarely written these days -- they're still automatically generated but I don't think this fix will make a difference there. And there's a well-understood workaround available.

tests/baselines/reference/ambientModuleDeclarationWithReservedIdentifierInDottedPath.errors.txt
==== ambientModuleDeclarationWithReservedIdentifierInDottedPath.ts (4 errors) ====
// https://github.com/microsoft/TypeScript/issues/7840

declare module chrome.debugger {
Copy link
Member

Choose a reason for hiding this comment

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

we haven't (yet) deprecated the module keyword, but please use declare namespace for new tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While the syntax is supported - shouldn't I add extra tests with declare namespace instead of changing this one?

@jakebailey
Copy link
Member

Honestly, I don't mind fixing it; esbuild gets this right: https://jakebailey.dev/esbuild-playground/#v3=PTAEAEDMEsBsFMB2BDAtvAXKAxge0TAOYB0AVgM74BQA3laKAERIAuATgJ4AKu0iL5RlgDajYHwAm8AB7EB0xgF0ANPSaRcbVMhZCm8cqkZUAvlSogIMBCnRZJMueWlUp2WMjbxQqXBICuCDgAFmy46MRSAEb+hITwbKB0DAxuHl6gAG6eoCzIUQCSEliI-qhRCQDcpuYyAA6aLDj45E15hRKgALwhYRHRsfFscvlF1a7w7p7evgFBLAYsxFPk5ElmE1MZs4HeA3EJ66CWrXCwoMiIoAlhbEA

@sandersn sandersn merged commit fe70ec9 into microsoft:main Aug 28, 2023
@Andarist Andarist deleted the fix/parse-reserved-identifiers-in-declared-dotted-names branch August 28, 2023 22:22
snovader pushed a commit to EG-A-S/TypeScript that referenced this pull request Sep 23, 2023
@Andarist @snovader
Parse reserved identifiers as dotted names of ambient modules (micros…
661251c 
…oft#55282)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sandersn sandersn sandersn approved these changes

@jakebailey jakebailey Awaiting requested review from jakebailey

Assignees

@sandersn sandersn

Labels
For Backlog Bug PRs that fix a backlog bug
Projects
PR Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Should be able to declare dotted name with debugger
4 participants
@Andarist @jakebailey @sandersn @typescript-bot