-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Deprecate module keyword for namespace declarations #58007
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 keyword for namespace declarations #58007
Conversation
@typescript-bot test it I'm going to make a second PR that uses this to issue a full error so we can gauge how bad things are at the moment (and verify that this is working). |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@jakebailey Here are the results of running the user tests comparing Everything looks good! |
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
@jakebailey Here are the results of running the top 400 repos comparing Everything looks good! |
Seems like it works? #58040 (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.
This looks good to me, but deferring to @DanielRosenwasser / @RyanCavanaugh for final wording.
One thing I'd like to check is whether Can you ensure that it doesn't, and provide a test as well? |
src/compiler/checker.ts
Outdated
@@ -46365,6 +46366,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
|
|||
if (isIdentifier(node.name)) { | |||
checkCollisionsForDeclarationName(node, node.name); | |||
if (!(node.flags & (NodeFlags.Namespace | NodeFlags.GlobalAugmentation))) { |
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.
This will only report errors in editor and there is no way to report these errors on command line. I think we need to handle "deprecation" version here instead to issue error
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 intent is to not report the in tsc yet, per #57913
Tests added. |
@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 |
… for ambient module declaration.
Fixes #57913