-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Add optionality to mapped type substitutions based on existence of an optional modifiers prop #57493
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
Add optionality to mapped type substitutions based on existence of an optional modifiers prop #57493
Conversation
… optional modifiers prop
@@ -18514,10 +18514,20 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
return !!(getUnionType([intersectTypes(type1, type2), neverType]).flags & TypeFlags.Never); | |||
} | |||
|
|||
function hasOptionalModifiersProperty(mappedType: MappedType) { | |||
const modifiersType = getApparentType(getModifiersTypeFromMappedType(mappedType)); | |||
if (!(modifiersType.flags & TypeFlags.Object)) { |
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.
TODO: might also have to handle intersection (that's what isWeakType
is doing) and I need to recheck behavior with unions
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.
recheck behavior with union types later
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.
@Mateusnasciment Please try to keep comments actionable and on-topic
@@ -205,6 +206,9 @@ keyofAndIndexedAccess2.ts(108,5): error TS2322: Type '123' is not assignable to | |||
type StrictExtract<T, U> = T extends U ? U extends T ? T : never : never; | |||
type StrictExclude<T, U> = T extends StrictExtract<T, U> ? never : T; | |||
type A<T> = { [Q in { [P in keyof T]: P; }[keyof T]]: T[Q]; }; |
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.
I'm not sure what's the best strategy to avoid circularity here. I didn't look into it yet. I'm hitting the 🛌 but maybe somebody would chime in with some suggestions in the meantime ;p
function substituteIndexedMappedType(objectType: MappedType, index: Type) { | ||
const mapper = createTypeMapper([getTypeParameterFromMappedType(objectType)], [index]); | ||
const templateMapper = combineTypeMappers(objectType.mapper, mapper); | ||
return instantiateType(getTemplateTypeFromMappedType(objectType.target as MappedType || objectType), templateMapper); | ||
const type = instantiateType(getTemplateTypeFromMappedType(objectType.target as MappedType || objectType), templateMapper); | ||
return addOptionality(type, /*isProperty*/ true, hasOptionalModifiersProperty(objectType)); |
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.
TODO: this should get the list of known optional properties and check if index
is assignable to that, if the substitution happens using a known non-optional property then the optionality shouldn't be added
I have what I think it the right fix in #57549. The main difference is that my PR is based on computing combined modifier flags, which I think is a better approach. It avoids resolving the members, which may cause circularities as you've observed. Also, member resolution is based on what members are known to be present, not which members might be present. |
fixes #57487