Skip to content

Ban non-ambient namespaces written with module keyword#61450

Closed
RyanCavanaugh wants to merge 4 commits into
microsoft:mainfrom
RyanCavanaugh:banSomeModules
Closed

Ban non-ambient namespaces written with module keyword#61450
RyanCavanaugh wants to merge 4 commits into
microsoft:mainfrom
RyanCavanaugh:banSomeModules

Conversation

@RyanCavanaugh

Copy link
Copy Markdown
Member

Draft PR to assess the prevalence of these in the wild (error message is reusing the ESO one).

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 19, 2025
@RyanCavanaugh

Copy link
Copy Markdown
Member Author

@typescript-bot test top800

@typescript-bot

typescript-bot commented Mar 19, 2025

Copy link
Copy Markdown
Contributor

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

Command Status Results
test top800 ✅ Started 👀 Results

@typescript-bot

Copy link
Copy Markdown
Contributor

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

Something interesting changed - please have a look.

Details

TypeStrong/typedoc

1 of 4 projects failed to build with the old tsc and were ignored

src/test/converter/tsconfig.json

@RyanCavanaugh RyanCavanaugh changed the title Ban non-ambient modules written with module keyword Ban non-ambient namespaces written with module keyword Mar 21, 2025
@RyanCavanaugh

Copy link
Copy Markdown
Member Author

@typescript-bot test top800
@typescript-bot run dt

@typescript-bot

typescript-bot commented Mar 21, 2025

Copy link
Copy Markdown
Contributor

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

Command Status Results
test top800 ✅ Started 👀 Results
run dt ✅ Started ✅ Results

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey @RyanCavanaugh, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot

Copy link
Copy Markdown
Contributor

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

Something interesting changed - please have a look.

Details

blitz-js/blitz

25 of 27 projects failed to build with the old tsc and were ignored

packages/generator/tsconfig.json

TypeStrong/typedoc

1 of 4 projects failed to build with the old tsc and were ignored

src/test/converter/tsconfig.json

visgl/react-map-gl

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

modules/main/tsconfig.json

@RyanCavanaugh

RyanCavanaugh commented Mar 21, 2025

Copy link
Copy Markdown
Member Author

gl-matrix builds index.d.ts from JS source (with --module amd --outFile!), but then patches out declare module "sub" { with declare module sub { with a custom build script

robpalme added a commit to robpalme/gl-matrix that referenced this pull request Jul 12, 2025
Previously some namespace types were generated using the legacy `module` keyword from ten years ago. Now we emit the modern TS `namespace` keyword that has been the preferred keyword since TypeScript 1.5 in 2015.

Please review the resulting diff for the generated `dist/index.d.ts` file:  https://gist.github.com/robpalme/a320dc3f0cb50bcd14962bca46827dae/revisions

Note that the outer _Ambient Module Declaration_ intentionally remains untouched because it is not a namespace.  These are differentiated by using a quoted string rather than a bare identifier

`module "quoted" {`
vs
`module bare {}`

---

Background:  This usage of the legacy keyword was found by [a TypeScript real-world test suite](microsoft/TypeScript#61450 (comment)) that checks the compatibility of proposed changes.  Using the `module` keyword for namespaces is [proposed for deprecation in TypeScript 6.0](microsoft/TypeScript#54500 (comment)) so it's worth getting ahead of this.
toji pushed a commit to toji/gl-matrix that referenced this pull request Jul 14, 2025
Previously some namespace types were generated using the legacy `module` keyword from ten years ago. Now we emit the modern TS `namespace` keyword that has been the preferred keyword since TypeScript 1.5 in 2015.

Please review the resulting diff for the generated `dist/index.d.ts` file:  https://gist.github.com/robpalme/a320dc3f0cb50bcd14962bca46827dae/revisions

Note that the outer _Ambient Module Declaration_ intentionally remains untouched because it is not a namespace.  These are differentiated by using a quoted string rather than a bare identifier

`module "quoted" {`
vs
`module bare {}`

---

Background:  This usage of the legacy keyword was found by [a TypeScript real-world test suite](microsoft/TypeScript#61450 (comment)) that checks the compatibility of proposed changes.  Using the `module` keyword for namespaces is [proposed for deprecation in TypeScript 6.0](microsoft/TypeScript#54500 (comment)) so it's worth getting ahead of this.
@microsoft microsoft locked as resolved and limited conversation to collaborators Jun 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants