Fix partial type relations#17382
Conversation
|
|
||
| function isPartialMappedType(type: Type) { | ||
| return getObjectFlags(type) & ObjectFlags.Mapped && !!(<MappedType>type).declaration.questionToken; | ||
| } |
There was a problem hiding this comment.
This seems like an awfully specific check - does it work on homomorphic mapped types in which all properties of the type being mapped are already optional? For example:
type Shapeify<T> = {
[K in keyof T]: any;
}
interface MyInterface {
something?: number;
}
class MyClass<T extends MyInterface> {
doIt(data: Shapeify<T>) {}
}
class MySubClass extends MyClass<MyInterface> {}
function fn(arg: typeof MyClass) {};
fn(MySubClass);The same question applies to if you used Readonly<T> or others.
There was a problem hiding this comment.
I'm not sure what you're getting at. The only way we can be certain that all properties of the type that results from a mapping operation are optional is if the mapped type includes a ? in the template specification. That's what we're checking here.
There was a problem hiding this comment.
As we discussed offline, that example doesn't apply because a constraint being all-optional (e.g. MyInterface) doesn't guarantee that T itself will be all-optional.
| result = Ternary.True; | ||
| } | ||
| else if (isGenericMappedType(target)) { | ||
| result = isGenericMappedType(source) ? mappedTypeRelatedTo(<MappedType>source, <MappedType>target, reportStructuralErrors) : Ternary.False; |
There was a problem hiding this comment.
If you made isGenericMappedType a type predicate, you could avoid the type assertions.
There was a problem hiding this comment.
No, because something for which isGenericMappedType returns false could still be a MappedType. We would need to introduce a new unique type representing generic mapped types, but it isn't worth the hassle.
|
@DanielRosenwasser If you're otherwise done with the review, please close it out and I will get this PR merged. |
|
@DanielRosenwasser Ping! |
With this PR the following is no longer an error:
Fixes #16985.