-
Notifications
You must be signed in to change notification settings - Fork 12.8k
local types are considered exported in .d.ts
files unlike local types in .ts
files
#57764
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
Comments
This is the intended behavior unless you have an |
|
This doesn't work in module augmentation. // react.d.ts
export = React;
export as namespace React;
export {}
declare namespace React { }
// augmentation.d.t.s
export {}
declare module '.' {
// can't put `export {}` here
interface Whatever {}
} will cause emissions of Also: |
In module augmentation, you can already refer to types in the outer block |
This issue has been marked as "Working as Intended" and has seen no recent activity. It has been automatically closed for house-keeping purposes. |
@RyanCavanaugh I don't understand what to make of this statement? I understand, that I can do that. How is that relevant here? Edit: You mean I should move it outside the module augmentation block and then it becomes hidden? Do we have lint rules for that in DT? |
Correct
I'm not sure how/why we would? Both locations are valid depending on intent. |
If you mean to flag unused ones that aren't exposed, those will get flagged once I have time to work on typescript-eslint/typescript-eslint#8611 and add it to DT (once I reimplement these scoping rules there), but yeah, there's no way to say "are you sure you meant to export that". |
I'm pretty sure if you write Why we need treat |
Unless explicitly asked, I'm not here to discuss what we'd do differently from a blank slate perspective. Those responses are too long to fit in these tiny textboxes. |
"Intended" can mean "this sucks but we can't change it and we are intentionally not changing it". But on DT, we can certainly try and create a lint rule that complains about mixed-exported types in modules without |
I learned today that TypeScript treats everything in a .d.ts file as if it was exported, whether it says `export` or not, unless you add an `export {}` declaration somewhere in the file. See microsoft/TypeScript#57764 We had a bunch of ts-prune-ignore-next declarations in internaltypes.d.ts to work around this. Adding `export {}` allows ts-prune to work as expected without workarounds.
I learned today that TypeScript treats everything in a .d.ts file as if it was exported, whether it says `export` or not, unless you add an `export {}` declaration somewhere in the file. See microsoft/TypeScript#57764 We had a bunch of ts-prune-ignore-next declarations in internaltypes.d.ts to work around this. Adding `export {}` allows ts-prune to work as expected without workarounds.
π Search Terms
ts2460 export type
π Version & Regression Information
β― Playground Link
No response
π» Code
Full repro: https://github.com/eps1lon/ts-module-auto-export
index.tsx
declaration.d.ts
library.ts
π Actual behavior
Import of local types is allowed in
.d.ts
files. OnlyModule '"./library"' declares 'Private' locally, but it is not exported.ts(2459)
is raisedπ Expected behavior
Local types are not exposed from
.d.ts
files unless they have anexport
modifier. We should getModule '"./declaration"' declares 'Private' locally, but it is not exported.ts(2459)
as well.The other issue is that TypeScript removes the local types from the declaration for
library.ts
instead of keeping it as a local type.This is a problem for libraries since every type is automatically part of the public API and we can't easily remove implementation details.
If TypeScript would hide local types, it would also need to make sure that emitted declarations don't reference those types.
The real-world issue is with React types. If TypeScript has to infer the type it may decide to either reference
React.ReactNode
explicitly (good) or inline all its members (bad) e.g.will be emitted as
Additional information about the issue
Original issue: vercel/next.js#63185
Maybe related to
The text was updated successfully, but these errors were encountered: