Skip to content

Conversation

@RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented Dec 10, 2025

See #54500
Fixes #62211

This changes the existing suggestion diagnostic to an error when you write forms like module foo or module foo.bar {

The ambient form declare module foo { is still ok edit: no, we agreed in #62211 to also deprecate that form, as is the ambient module declaration form declare module "specifier" {

Copilot AI review requested due to automatic review settings December 10, 2025 21:56
@github-project-automation github-project-automation bot moved this to Not started in PR Backlog Dec 10, 2025
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Dec 10, 2025
Copy link

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 PR deprecates the non-ambient module keyword syntax in TypeScript, changing it from a suggestion diagnostic to an error. The change enforces that developers use namespace instead of module for namespace declarations, while still allowing declare module for ambient declarations and declare module "specifier" for module augmentation.

Key changes:

  • Changed the diagnostic from a suggestion to an error for non-ambient module declarations
  • Updated the condition to exclude ambient contexts from the error
  • Updated all test cases to use namespace instead of module

Reviewed changes

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

Show a summary per file
File Description
src/compiler/checker.ts Modified the check to emit an error instead of a suggestion diagnostic for non-ambient module declarations, and removed unused import
tests/cases/fourslash/moduleDeclarationDeprecated_suggestion1.ts Removed test file that checked for suggestion diagnostics
tests/cases/fourslash/moduleDeclarationDeprecated_suggestion2.ts Removed test file that checked for empty suggestion diagnostics
tests/cases/compiler/moduleKeywordDeprecated.ts Added new test file to verify error behavior for non-ambient module declarations
tests/cases/conformance/internalModules/DeclarationMerging/*.ts Updated test cases to use namespace instead of module
tests/cases/compiler/*.ts Updated multiple test cases to use namespace instead of deprecated module keyword
tests/baselines/reference/*.{types,symbols,js,errors.txt} Updated baseline files to reflect changes from module to namespace
Comments suppressed due to low confidence (2)

tests/cases/compiler/escapedIdentifiers.ts:1

  • The declare keyword was added to convert this non-ambient module declaration to an ambient one. However, this changes the semantics of the code beyond just updating the keyword. If this was previously a non-ambient module declaration, it should be changed to namespace moduleType\u0032 instead.
    tests/baselines/reference/moduleKeywordDeprecated.errors.txt:1
  • The error diagnostic is still labeled as "suggestion" in the baseline output, but according to the PR description and the code changes, it should now be an error. The diagnostic level should be "error" instead of "suggestion".

@RyanCavanaugh
Copy link
Member Author

@typescript-bot test top800

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 10, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top800 ✅ Started ✅ Results

@@ -0,0 +1,58 @@
dottedModuleName2.ts(22,8): suggestion TS1540: A 'namespace' declaration should not be declared using the 'module' keyword. Please use the 'namespace' keyword instead.
Copy link
Member

Choose a reason for hiding this comment

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

The message in diagnosticMessage.json should be updated to "error"

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are the results of running the top 800 repos with tsc comparing main and refs/pull/62876/merge:

Everything looks good!

@typescript-bot typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Dec 11, 2025
@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 11, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 11, 2025

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/166787/artifacts?artifactName=tgz&fileId=5B5A1DC78D2CF32FC54B5EBC8ACD09579FF86E294083153584D973D89884D14D02&fileName=/typescript-6.0.0-insiders.20251211.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@6.0.0-pr-62876-5".;

@robpalme
Copy link

Thanks for the pack. A quick try out suggest that when you provoke the intended checker error, it crashes.

Error: Debug Failure. Should never get an Info diagnostic on the command line.
    at getCategoryFormat (/home/projects/stackblitz-starters-ybxu3fvk/node_modules/typescript/lib/_tsc.js:121656:20)
    at formatDiagnosticsWithColorAndContext (/home/projects/stackblitz-starters-ybxu3fvk/node_modules/typescript/lib/_tsc.js:121721:71)

@RyanCavanaugh
Copy link
Member Author

A quick try out suggest that when you provoke the intended checker error, it crashes.

Well, that's one way to get people to change their code 🤦

@RyanCavanaugh
Copy link
Member Author

@typescript-bot test top800
@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 11, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top800 ✅ Started 👀 Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 11, 2025

Hey @RyanCavanaugh, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/166789/artifacts?artifactName=tgz&fileId=F7B95EBDF36B0403901B7D4CB436D9FE693D2E5245B7A658F1362E103A40D18402&fileName=/typescript-6.0.0-insiders.20251211.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@6.0.0-pr-62876-10".;

@robpalme
Copy link

The new pack works great. No crashes!

And it correctly errors on the declare form. Thank you for that update 💙

The only issue I observe is that for all forms, the error squiggles are on the identifier, not the keyword. Which seems a bit misleading.

index.ts:3:16 - error TS1540: A 'namespace' declaration should not be declared using the 'module' keyword. Please use the 'namespace' keyword instead.

3 declare module types {
                 ~~~~~


Found 1 error in index.ts:3

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are the results of running the top 800 repos with tsc comparing main and refs/pull/62876/merge:

Something interesting changed - please have a look.

Details

microsoft/vscode

11 of 63 projects failed to build with the old tsc and were ignored

src/tsconfig.tsec.json

src/tsconfig.monaco.json

build/checker/tsconfig.worker.json

build/checker/tsconfig.electron-utility.json

build/checker/tsconfig.electron-main.json

build/checker/tsconfig.electron-browser.json

@jakebailey
Copy link
Member

@typescript-bot test top800

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 11, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top800 ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top 800 repos with tsc comparing main and refs/pull/62876/merge:

Everything looks good!

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

So long, module

@github-project-automation github-project-automation bot moved this from Not started to Needs merge in PR Backlog Dec 12, 2025
@RyanCavanaugh RyanCavanaugh added this pull request to the merge queue Dec 12, 2025
Merged via the queue into microsoft:main with commit 16b933f Dec 12, 2025
33 checks passed
@RyanCavanaugh RyanCavanaugh deleted the deprecateModules branch December 12, 2025 00:30
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Dec 12, 2025
jakebailey pushed a commit to jakebailey/TypeScript that referenced this pull request Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Deprecate, remove support for using module in place of namespace

5 participants