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

Null-Check is not fully recognized for Partial<T> generic mapped types #42537

Closed
JakenVeina opened this issue Jan 28, 2021 · 2 comments
Closed
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@JakenVeina
Copy link

Bug Report

🔎 Search Terms

mapped
type
null
check
function
parameter

🕗 Version & Regression Information

First encountered today, when attempting to write some code utilizing generic mapped types.

This is the behavior in every version I tried, including 3.3.3 through Nightly, and I reviewed the FAQ for entries about mapped types.

⏯ Playground Link

Here

💻 Code

type Foo<TAnchor extends HTMLElement> = { readonly anchor: TAnchor; };
type FooMap = { readonly [fooKey: string]: Foo<HTMLElement>; };
type FooAnchorMap<TMap extends FooMap> = { readonly [TKey in keyof TMap]: TMap[TKey]["anchor"]; };

function baz(anchor: HTMLElement) { anchor.hidden = false; }

function bar<TMap extends FooMap>(fooKey: keyof TMap, fooAnchors: Partial<FooAnchorMap<TMap>>) {
    const fooAnchor = fooAnchors[fooKey];

    if (fooAnchor == null) {
        throw new Error(`Unable to find key '${fooKey}' in map`);
    }

    // The compiler clearly recognizes that fooAnchor is HTMLElement, as confirmed by intellisense.
    fooAnchor.hidden = true;

    // Error here, indicating that the inferred type of fooAnchor is 'TMa[[keyof TMap]["anchor"] | undefined', as confirmed by intellisense
    baz(fooAnchor);
}

🙁 Actual behavior

The variable fooAnchor is inferred to be possibly undefined, almost immediately after a clear null-check. However, it's somehow only inferred to be possibly undefined when passed as a function argument. When used directly, the null-check is recognized.

🙂 Expected behavior

Ideally, the null-check should be recognized fully, by all uses of fooAnchor after the null-check occurs. However, if this is somehow not feasible, the inferred type of fooAnchor should at least be consistent. I.E. it should not be possible for the compiler to infer the variable's differently at different points within the same narrowing scope (I.E. when no type-narrowing or type-widening operations have occurred).

This seems to be very closely related to #31908. However, I thought it was appropriate to make a separate issue, because this situation doesn't completely line up with the situations described in that issue. Most significantly...

TypeScript doesn't know that partial[key] is equivalent to T[K] | undefined, only that it's assignable to that type.

This doesn't seem to be true here, because A) the error, and intellisense clearly report the inferred type to be ... | undefined and B) the variable is able to be used directly as if it's not possibly undefined. It's only when passing the variable as an argument that the error occurs.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jan 29, 2021
@RyanCavanaugh
Copy link
Member

There are two competing objectives here and we're not able to fulfill both of them simultaneously:

  • Lookup types should fully preserve the relation between input and output types
  • Narrowings should just remove types from unions or intersect in new types, not construct new monstrosity types

In the case of a property write/access, the value is treated as if it were just the constrained type, so you get non-indirected behavior. This is convenient (though also unsound)

The core problem is that the narrowed type of fooAnchor is not really speakable. Its unnarrowed type is Partial<FooAnchorMap<TMap>>[keyof TMap], which isn't a union that we can remove undefined and null from to produce some other type that would be provably-assignable to HTMLElement. I believe this could be fixed in a world with #22348 but the perf/complexity implications of that one are pretty stark.

A good workaround is to write this

    const fooAnchor: HTMLElement | undefined = fooAnchors[fooKey];

to just fully opt-in to the non-lookup-preserving behavior

@JakenVeina
Copy link
Author

I can certainly get behind both of those goals.

Feel free to close if no direct action is going to be taken on this. Working around it is easy enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

2 participants