-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Deprecate module syntax
#62876
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
Deprecate module syntax
#62876
Conversation
There was a problem hiding this 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
moduledeclarations - Updated the condition to exclude ambient contexts from the error
- Updated all test cases to use
namespaceinstead ofmodule
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
declarekeyword was added to convert this non-ambientmoduledeclaration 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 tonamespace moduleType\u0032instead.
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".
|
@typescript-bot test top800 |
| @@ -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. | |||
There was a problem hiding this comment.
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"
|
@RyanCavanaugh Here are the results of running the top 800 repos with tsc comparing Everything looks good! |
|
@typescript-bot pack this |
|
Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
Thanks for the pack. 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 🤦 |
|
@typescript-bot test top800 |
|
Hey @RyanCavanaugh, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
The new pack works great. No crashes! And it correctly errors on the 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. |
|
@typescript-bot test top800 |
|
@jakebailey Here are the results of running the top 800 repos with tsc comparing Everything looks good! |
jakebailey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So long, module
See #54500
Fixes #62211
This changes the existing suggestion diagnostic to an error when you write forms like
module fooormodule foo.bar {The ambient formedit: no, we agreed in #62211 to also deprecate that form, as is the ambient module declaration formdeclare module foo {is still okdeclare module "specifier" {