Skip to content
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

Inconsistent narrowing from unions with overlapping possibly-optional properties #26551

Closed
jcalz opened this issue Aug 20, 2018 · 4 comments
Closed
Labels
Question An issue which isn't directly actionable in code

Comments

@jcalz
Copy link
Contributor

jcalz commented Aug 20, 2018

TypeScript Version: 3.1.0-dev.20180813

Search Terms:
type guard, narrowing, union, optional, overlapping

Code

// --strictNullChecks on, please
declare function guard(w: any): w is { a: string | undefined };

declare const x: { a?: string };
x.a; // string | undefined
if (guard(x)) {
  x.a; // string | undefined
}

declare const y: { a?: string } | { a: string };
y.a // string | undefined
if (guard(y)) {
  y.a; // string ??
}

Expected behavior:
With --strictNullChecks on: after being narrowed by the type guard, both x.a and y.a should be string | undefined.

Actual behavior:
The type guard narrows y to {a: string} for some reason. The {a?: string} constituent is presumably being eliminated from the union, but I'm not sure why.

Playground Link:
Link

Related Issues:
Didn't find anything obvious... maybe #11403 or #17561?

Noticed in a comment in #21732.

@RyanCavanaugh
Copy link
Member

This is the relevant code

// If the current type is a union type, remove all constituents that couldn't be instances of
// the candidate type. If one or more constituents remain, return a union of those.
if (type.flags & TypeFlags.Union) {
    const assignableType = filterType(type, t => isRelated(t, candidate));
    if (!(assignableType.flags & TypeFlags.Never)) {
        return assignableType;
    }
}
// If the candidate type is a subtype of the target type, narrow to the candidate type.
// Otherwise, if the target type is assignable to the candidate type, keep the target type.
// Otherwise, if the candidate type is assignable to the target type, narrow to the candidate
// type. Otherwise, the types are completely unrelated, so narrow to an intersection of the
// two types.
return isTypeSubtypeOf(candidate, type) ? candidate :
    isTypeAssignableTo(type, candidate) ? type :
        isTypeAssignableTo(candidate, type) ? candidate :
            getIntersectionType([type, candidate]);

Basically, unions and non-unions narrow differently. For a union, we remove non-assignable constituents; for a non-union, we intersect in the guarded type if it's disjoint from the original type. This generally produces the best behavior in both cases, at the cost of creating an "inconsistency"

@jcalz
Copy link
Contributor Author

jcalz commented Aug 20, 2018

Thanks. I guess I don't see how { a?: string } is removed from the union though... since we're testing if it's assignable to { a?: string }, right?

@RyanCavanaugh
Copy link
Member

isRelated is isRelated(source, target) - we're testing assignability from each union constituent (actually relation is subtype in this case)

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Aug 20, 2018
@jcalz
Copy link
Contributor Author

jcalz commented Aug 20, 2018

Ah, okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

2 participants