Have getAssignmentReducedType use the comparable relation instead of typeMaybeAssignableTo. #26143
Conversation
typeMaybeAssignableTo. typeMaybeAssignableTo decomposed unions at the top level of the assigned type but didn't properly handle other unions that arose during assignability checking, e.g., in the constraint of a generic lookup type. Fixes microsoft#26130.
|
@mattmccutchen Thanks for looking into it. I like this fix. The concept of "is maybe assignable to" is precisely what the comparable relation represents, so it makes sense to use that here. The baseline change isn't really significant as it happens only in the context of something that is an error. It's caused by the fact that distinct enum types are comparable, but not assignable. |
|
@ahejlsberg we've gotten an error in our RWC suite from this change. Specifically in a case like type AOrArrA<T> = T | T[];
const arr: AOrArrA<{x?: "ok"}> = [{ x: "ok" }]; // weak type
arr.push({ x: "ok" });is broken now. |
|
We can try to patch that example back to where it was before by checking for weak types in some manner, but it points to a broader problem: let src = {a: 4, b: 5};
let tgt: {a: number, b: number};
let narrow1: {a: number, c: number} | {a: number, b: number} = src;
tgt = narrow1; // OK
let narrow2: {a: number} | {a: number, b: number} = src;
tgt = narrow2; // Error: `{a: number}` is kept in the type of `narrow2` because `{a: number, b: number}` is assignable to it.What we really want for the narrowed type is a minimal sub-union of the declared type to which the assigned type is assignable. Retaining all union members to which the assigned type is comparable is a poor approximation to that in general. Is there anything better we can do? I assume the reason we can't just take the assigned type as the narrowed type is because assignability is nontransitive and users will be upset if the narrowed type is not usable in a context that the declared type would be. What about letting the narrowed type be the intersection of the declared type and the assigned type? Shall I open a new issue? |
|
I filed #26405 to make sure we don't forget about the regression. |
…microsoft#26130 by skipping narrowing if the old algorithm produces a type to which the assigned type is not assignable. This also means we'll no longer narrow for erroneous assignments where the assigned type is not assignable to the declared type. This is the reason for the numericLiteralTypes3 baseline change. Fixes microsoft#26405.
typeMaybeAssignableTo decomposed unions at the top level of the assigned
type but didn't properly handle other unions that arose during
assignability checking, e.g., in the constraint of a generic lookup
type.
Fixes #26130.
There is a change to an existing baseline that I don't understand. Do we care?