Skip to content

Conversation

RyanCavanaugh
Copy link
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
Member Author

@typescript-bot test top800

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 19, 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

@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
Member Author

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 21, 2025

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
Collaborator

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
Collaborator

@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
Copy link
Member Author

RyanCavanaugh commented Mar 21, 2025

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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