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

Transform namespaces declared with module keyword to namespace keyword in d.ts files #54134

Conversation

CC972
Copy link
Contributor

@CC972 CC972 commented May 4, 2023

Addresses part of #51825.

With this PR, we encourage people to move away from declaring namespaces with the module keyword.

Currently, if we have foo.ts:

module foo {}

This leads to the following foo.d.ts file:

declare module foo {}

This PR changes the declaration emit to transform module to namespace:

declare namespace foo {}

"Ambient module declarations", such as the below, will be unaffected:

declare module "bar" {}

CC972 and others added 6 commits April 24, 2023 16:28
Signed-off-by: Chi Leung <chiguan.leung@gmail.com>
Signed-off-by: Chi Leung <chiguan.leung@gmail.com>
Signed-off-by: Chi Leung <chiguan.leung@gmail.com>
Signed-off-by: Chi Leung <chiguan.leung@gmail.com>
…ith-module-keyword

Transform namespaces declared with module keyword to namespace keyword in d.ts files
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label May 4, 2023
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #51825. If you can get it accepted, this PR will have a better chance of being reviewed.

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.

  1. This issue doesn't fix all of Consider deprecating namespaces declared with the module keyword #51825, so I'd recommend swapping the wording to "Addresses part of Consider deprecating namespaces declared with the module keyword #51825" so that github doesn't close the original issue.
  2. The implementation seems straightforward and the test changes look right to me, but @rbuckton should probably sign off on the API change.

@sandersn sandersn moved this from Not started to Waiting on reviewers in PR Backlog May 22, 2023
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

PR Backlog automation moved this from Waiting on reviewers to Waiting on author Jun 8, 2023
@CC972 CC972 requested a review from rbuckton June 16, 2023 14:04
PR Backlog automation moved this from Waiting on author to Needs merge Jun 26, 2023
@rbuckton rbuckton merged commit a94eb31 into microsoft:main Jun 26, 2023
17 checks passed
PR Backlog automation moved this from Needs merge to Done Jun 26, 2023
@dragomirtitian dragomirtitian deleted the CC972/deprecate-namespaces-declared-with-module-keyword branch November 17, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants