-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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 homomorphism of inlined mapped types in declaration emit #48091
Preserve the homomorphism of inlined mapped types in declaration emit #48091
Conversation
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.
Couple of background questions, plus a couple of high-level comments:
- I think this is a good approach. Are there others you thought of?
- How often will this apply to mapped types? Are nearly all of them both homomorphic and inlined? Does inlining only happen when a private type is used in an exported type?
@@ -5092,7 +5098,19 @@ namespace ts { | |||
const templateTypeNode = typeToTypeNodeHelper(removeMissingType(getTemplateTypeFromMappedType(type), !!(getMappedTypeModifiers(type) & MappedTypeModifiers.IncludeOptional)), context); | |||
const mappedTypeNode = factory.createMappedTypeNode(readonlyToken, typeParameterNode, nameTypeNode, questionToken, templateTypeNode, /*members*/ undefined); | |||
context.approximateLength += 10; | |||
return setEmitFlags(mappedTypeNode, EmitFlags.SingleLine); | |||
const result = setEmitFlags(mappedTypeNode, EmitFlags.SingleLine); | |||
if (isMappedTypeWithKeyofConstraintDeclaration(type) && !(getModifiersTypeFromMappedType(type).flags & TypeFlags.TypeParameter) && context.flags & NodeBuilderFlags.GenerateNamesForShadowedTypeParams) { |
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.
it probably doesn't make sense to precalculate this predicate; on the other hand, it's confusing to read as-is and a name would help.
In particular, I don't understand why the modifiers type needs to be not-a-type-parameter. Can you give an example?
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.
If the modifiers type is a type parameter, it'll preserve the homomorphism of the mapped type on its own - we don't need to substitute in a temporary type parameter.
They key issue is that we need to emit a type parameter in the mapped type keyof position to preserve homomorphism. So something like inline type aliases may also be usable to generate similar emit, pending exact implementation. But this is where what's possible with existing type constructors.
Any time we have a generic mapped type that would lose its homomorphism otherwise. Generally it requires inlining a private or otherwise unnammed type in a still-generic position, yes. The node builder flags check prevents us from applying the logic for quick info or error type printback, so it'll only print like this in declaration emit. |
Fixes #46655 - declaration emit now preserves the homomorphism of mapped types by emitting a conditional type with an
infer
type parameter when needed. So instead of, eg,{ [K in keyof T[any]]: T[any][K]; }
we now emitT[any] extends infer T_1 ? { [K in keyof T_1]: T[any][K]; } : never
.