Skip to content

Conversation

RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented Sep 4, 2025

Setting this up so that the incoming "deprecate module foo {" PR has a more reasonable diff

If anyone knows which tests are intentionally doing this let me know, otherwise we can just add a handful more in the implementation PR

@Copilot Copilot AI review requested due to automatic review settings September 4, 2025 22:39
@github-project-automation github-project-automation bot moved this to Not started in PR Backlog Sep 4, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request replaces all instances of the module keyword with namespace in TypeScript test case baseline files. This change is preparatory work for a future PR that will deprecate the module foo { syntax in favor of the namespace keyword, which is the modern and preferred way to declare internal modules in TypeScript.

Key Changes:

  • Systematic replacement of module declarations with namespace declarations across test baseline files
  • Updated line numbers and symbol references in baseline files to reflect the keyword length change (3 additional characters)
  • No functional changes to TypeScript behavior, only syntax modernization in test files

Reviewed Changes

Copilot reviewed 300 out of 5766 changed files in this pull request and generated no comments.

File Description
Multiple .types files Updated type checking baselines with namespace replacing module declarations
Multiple .symbols files Updated symbol resolution baselines with adjusted line numbers due to keyword change
Multiple .js files Updated JavaScript compilation output baselines showing namespace syntax
Multiple .errors.txt files Updated error message baselines with corrected line references

@RyanCavanaugh
Copy link
Member Author

Output of git show on this commit https://gist.github.com/RyanCavanaugh/6c12002ad7615a9e771177a042e7ec20

// https://github.com/microsoft/TypeScript/issues/7840

declare module chrome.debugger {
declare namespace 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.

I wonder if they still use module?

Copy link
Member

Choose a reason for hiding this comment

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

Searching github shows no results other than this test.

@jakebailey
Copy link
Member

Do we have unit tests which hit this?

@RyanCavanaugh
Copy link
Member Author

We have a few, incrementalParser.ts and preProcessFile.ts for example

@jakebailey
Copy link
Member

Right, I didn't see them in the gist, so wasn't sure if they were intentionally missing or just out of scope for this already mega-big-large PR. Probably best to do it elsewhere.

@github-project-automation github-project-automation bot moved this from Not started to Needs merge in PR Backlog Sep 8, 2025
@RyanCavanaugh RyanCavanaugh merged commit 3f5c77f into microsoft:main Sep 8, 2025
33 checks passed
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Sep 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants