-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Preserve the distributivity of inlined conditional types in declaration emit #48592
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
Preserve the distributivity of inlined conditional types in declaration emit #48592
Conversation
export {}; | ||
//// [api.d.ts] | ||
export declare const dropPrivateProps1: <Obj>(obj: Obj) => { [K in import("./internal").PublicKeys1<keyof Obj>]: Obj[K]; }; | ||
export declare const dropPrivateProps2: <Obj>(obj: Obj) => { [K in keyof Obj extends infer T ? T extends keyof Obj ? T extends `_${string}` ? never : T : never : never]: Obj[K]; }; |
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.
- Is it not a problem that the middle conditional type (the inner of the two synthesized ones) is distributive itself? It seems like it comes out the same, but in a way the semantics are slightly different here, right?—the
T extends `_${string}`
bit will never actually have anything to distribute over because the distribution is done one level up. - Whether or not (1) actually matters, you could use Ron’s new infer constraints here instead, which would reduce a level of nesting. That seems preferable to me, unless we’re specifically trying to avoid emitting that for back compat reasons, but... I think that’s a job for downlevel-dts.
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.
- Extra layers of distributivity don't matter, so long as the right type variable is distributed over before the conditional logic occurs.
- Yeah, I mention that in a comment in the code - we could, but we'd start producing declaration files incompatible with older versions of TS and I don't know if we wanna do that just yet (considering they don't come from any input code positions). It's always something we could change down the line.
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 other thing I was curious about is whether we could ever just copy the unreachable type alias declaration to a local one. If we’re already copying its contents, why not just drop the whole declaration nearby with a unique name?
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.
Wherever we can, we do (specifically if we can preserve it as-is, in it's existing location). We only do inlining like this when the type comes from somewhere unreachable or elided (like inside a different module's locals or an elided function body). We don't copy from those scopes into the current scope because then we'd have to do 1. pull in its whole dependency tree of types (which may, in turn, depend on things we just can't copy, like signature type parameters), and 2. ensure everything is renamed in such a way where there's no name conflicts. It's just not something we do much of right now.
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 in its whole dependency tree of types
This is what I thought at first, but don’t we end up having to do that if we inline it too? Like the only part of the type that we don’t copy is the declaration.
By wrapping them in two more layers of conditionals - one to establish a local type parameter for distributivity, one to constrain said type parameter so it remains usable as the instantiated check type.
This means something like
type PrivateFields<Keys> = Keys extends _${string} ? Keys : undefined;
when used asPrivateFields<keyof Obj>
will be inlined tokeyof Obj extends infer T ? T extends keyof Obj ? T extends _${string} ? T : never
, to retain the distributive behavior, rather than the erroneouskeyof Obj extends _${string} ? keyof Obj : never
.Fixes #48140