Declaration emit for inlined mapped types preserves modifier-preserving behavior #55054
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
| @@ -6639,6 +6639,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| const questionToken = type.declaration.questionToken ? factory.createToken(type.declaration.questionToken.kind) as QuestionToken | PlusToken | MinusToken : undefined; | |||
| let appropriateConstraintTypeNode: TypeNode; | |||
| let newTypeVariable: TypeReferenceNode | undefined; | |||
| // If the mapped type isn't `keyof` constraint-declared, _but_ still has modifiers preserved, and its' naive instantiation won't preserve modifiers because its' constraint isn't `keyof` constrained, we have work to do | |||
There was a problem hiding this comment.
| // If the mapped type isn't `keyof` constraint-declared, _but_ still has modifiers preserved, and its' naive instantiation won't preserve modifiers because its' constraint isn't `keyof` constrained, we have work to do | |
| // If the mapped type isn't `keyof` constraint-declared, _but_ still has modifiers preserved, and its naive instantiation won't preserve modifiers because its constraint isn't `keyof` constrained, we have work to do |
| @@ -6639,6 +6639,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| const questionToken = type.declaration.questionToken ? factory.createToken(type.declaration.questionToken.kind) as QuestionToken | PlusToken | MinusToken : undefined; | |||
| let appropriateConstraintTypeNode: TypeNode; | |||
| let newTypeVariable: TypeReferenceNode | undefined; | |||
| // If the mapped type isn't `keyof` constraint-declared, _but_ still has modifiers preserved, and its' naive instantiation won't preserve modifiers because its' constraint isn't `keyof` constrained, we have work to do | |||
| const needsModifierPreservingWrapper = !isMappedTypeWithKeyofConstraintDeclaration(type) && !(getModifiersTypeFromMappedType(type).flags & TypeFlags.Unknown) && context.flags & NodeBuilderFlags.GenerateNamesForShadowedTypeParams | |||
| && !(getConstraintTypeFromMappedType(type).flags & TypeFlags.TypeParameter && getConstraintOfTypeParameter(getConstraintTypeFromMappedType(type))?.flags! & TypeFlags.Index); | |||
There was a problem hiding this comment.
dprint when (this is hard to read with 2 predicates per line, each of which are so long that they wrap on my screen)
| else if (needsModifierPreservingWrapper) { | ||
| // and step 3: once the mapped type is reconstructed, create a `ConstraintType extends infer T_1 extends keyof ModifiersType ? {[K in T_1]: Template} : never` | ||
| // subtly different from the `keyof` constraint case, by including the `keyof` constraint on the `infer` type parameter, it doesn't rely on the constraint type being itself | ||
| // constrained to a `keyof` type to preserve it's modifier-preserving behavior. This is all basically because we preserve modifiers for a wider set of mapped types than |
There was a problem hiding this comment.
| // constrained to a `keyof` type to preserve it's modifier-preserving behavior. This is all basically because we preserve modifiers for a wider set of mapped types than | |
| // constrained to a `keyof` type to preserve its modifier-preserving behavior. This is all basically because we preserve modifiers for a wider set of mapped types than |
double-checking: is the effect of this PR that we now preserve modifiers for these mapped types, and didn't before?
There was a problem hiding this comment.
without reading the code of the fix itself - yes, the issue that I reported is specifically about the fact that modifiers are lost but they shouldn't be so the fix for that issue should preserve them
|
IIUC the test case from my issue is meant to preserve modifiers but is not meant to make the given mapped type distributive. This is handled correctly by this fix. Is there an extra test case to be written here that would test that both are preserved? OTOH, probably when distributivity is preserved, it also implies preserving the modifiers so in practice. We don't have 4 combinations but rather just 3 (nothing is preserved, modifiers are preserved, both are preserved). |
There was a problem hiding this comment.
Is there a way today to test this through the evaluator~ tests? That could be quite nice for a stronger validation of how distribution and modifiers are preserved (or not) in cases like this.
Distributivity of mapped types comes from homomorphic-ness (which also preserves modifiers), which we have a separate emit form to preserve when inlining breaks it, which already has its' own tests. You're correct that there's 3 relevant states (distributive and modifiers preserved, only modifiers preserved, nothing preserved) and that this PR covers the missing one (only modifiers preserved)~ |
Fixes #55002
This is somewhat similar to our custom declaration emit that wraps mapped types to preserve homomorphic-ness (circa #48091), if needs be, but is a slightly different (more verbose) wrapper to preserve just the modifier-preserving behavior of the emitted mapped type.