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

Can't exclude undefined from indexed Partial with simple guard #31908

Closed
dgreensp opened this issue Jun 14, 2019 · 9 comments · Fixed by #49119
Closed

Can't exclude undefined from indexed Partial with simple guard #31908

dgreensp opened this issue Jun 14, 2019 · 9 comments · Fixed by #49119
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Fix Available A PR has been opened for this issue

Comments

@dgreensp
Copy link

TypeScript Version: 3.5.1

Search Terms:

Indexed, Partial, undefined, guard

Code

function getValue<T, K extends keyof T>(partial: Partial<T>, key: K): T[K] | null {
  const value = partial[key]
  if (value !== undefined) {
    return value // error: TypeScript thinks this can be undefined...
  }
  return null
}

Expected behavior:
Inside the if statement, value should have type T[K]. Especially since partial[key] is assignable to T[K] | undefined, and in fact TypeScript reports the type in the error message as T[K] | undefined, inside the if statement! It's like it just doesn't simplify Partial<T>[K] until it tries returning it.

Here is the workaround, which doesn't involve any casting, just hints:

function getValue<T, K extends keyof T>(partial: Partial<T>, key: K): T[K] | null {
  const value: T[K] | undefined = partial[key]
  if (value !== undefined) {
    return value // OK
  }
  return null
}

It would be nice if this hint wasn't necessary.

Actual behavior:

TypeScript complains that T[K] | undefined is not ok to return, even though the type should be T[K].

Playground Link:

(requires strictNullChecks)

https://www.typescriptlang.org/play/#src=function%20getValue%3CT%2C%20K%20extends%20keyof%20T%3E(partial%3A%20Partial%3CT%3E%2C%20key%3A%20K)%3A%20T%5BK%5D%20%7C%20null%20%7B%0D%0A%20%20const%20value%20%3D%20partial%5Bkey%5D%0D%0A%20%20if%20(value%20!%3D%3D%20undefined)%20%7B%0D%0A%20%20%20%20return%20value%20%2F%2F%20error%3A%20TypeScript%20thinks%20this%20can%20be%20undefined...%0D%0A%20%20%7D%0D%0A%20%20return%20null%0D%0A%7D

Related Issues:

@jack-williams
Copy link
Collaborator

jack-williams commented Jun 16, 2019

Duplicate of #11483.

EDIT: Nope, sorry. Misread the example. I'm guessing that maybe the narrowing code doesn't do the same level of probing to the expression type compared to when types are related. Is this a perf thing or just a missing feature?

@dgreensp
Copy link
Author

dgreensp commented Jun 19, 2019

That's right, it's not that TypeScript doesn't narrow the type of the expression partial[key], it doesn't narrow the type of value (which = partial[key]) either.

@RyanCavanaugh
Copy link
Member

TypeScript doesn't know that partial[key] is equivalent to T[K] | undefined, only that it's assignable to that type. Knowing that it's equivalent basically requires higher-order understanding of what Partial<T> does by working backwards through the mapped type and reasoning about what kind of values come out the other side. If we had #22348 (which is a scary bag of worms), this would probably end up working out OK.

Anyway with all that said, having the only inference position for T be a Partial<T> is going to be a serious damper on your inference. It'd be better to write the signature as

type NotUndefined<T> = T extends undefined ? never: T;
function getValue<T, K extends keyof T>(partial: T, key: K): NotUndefined<T[K]> | null {
  const value = partial[key]
  if (value !== undefined) {
    return value as NotUndefined<T[K]>;
  }
  return null
}

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jun 26, 2019
@ThomasKruegl
Copy link

I might have a similar issue:

type MyEvents = 'start' | 'stop' | 'suspend';

type EventCounts = { [key in MyEvents]?: number};

const eventCounts: EventCounts = { start: 0 };

function foo<T extends MyEvents>(event: T) {
    
    // not working  - undefined is not assignable to number
    const count: number = eventCounts[event]  || 0;

    // working
    const foo: number | undefined = eventCounts[event];
    const count2: number = foo || 0;


    // other workarounds
    const count3: number = (eventCounts[event] || 0)!;
    const count4: number = eventCounts[event] as number | undefined || 0;

}

I didn't really understand the reasoning about

requires higher-order understanding of what Partial does

In my case this is more direct (optional index signature). Does this hit the same design limitation?

(I know that in this example the generic is only constructed, i could just use "event: MyEvents" non generic, that's also working, but not the point :)).

Example on Playground:
https://www.typescriptlang.org/play/?target=6#code/C4TwDgpgBAsiCiA3CA7YBnKBeKBydwAhgE7C5QA+eBA9mOVfgK7qQoAmuA3AFA+iQoSVMADCNJmkw4A3lADaAawggoASxSwEyKQF0A-AC4oKJgFsARhGIBfXjwDGNFASgQdYiVOPC04yRjYUHIEJMDGAAxQdnwAZpIOwGrOULE0NAA8ACpuAB7AqOyYcL4YAHwAFO4ixlkAlME8UM1QTS0A9O0mNMBQAO40xIoaAObNALRQkuwQsRoQ7OqYKD1QhOjoaiMohBYANtDANCbmVsRtzU4uvU4BxqaW1kHVfl4Y8i-Aus0UVBH2LSgnX6g2GKBGFygV1caRo91OTyo01m80WOE+-ikHw8ul4gOhNzeACZ4Y9iEFYZQ-vZIcCegALJ4DIYkLxFSEEqFvADMpLOQSqHkx70+31+UAidQAhHiWpzbmgACx8p7ooVvdDYkTfdYnMmUKYcFEoBZUiX2GxAA

@dgreensp
Copy link
Author

dgreensp commented Jun 27, 2019

In the following example, it seems like the compiler does know that Dict<K>[K] is equivalent to number | undefined, not just assignable, because the compiler itself generates the type number | undefined for value, just not in time to narrow it:

type Dict<K extends string> = { [key in K]?: number }

function getValue<K extends string>(dict: Dict<K>, key: K): number {
    const value = dict[key] // value: Dict<K>[K]
    if (value) { // or: if (value !== undefined)
        // Error: Type 'number | undefined' is not assignable to type 'number'.
        return value
    }
    throw new Error()
}

@RyanCavanaugh Thanks for the input. PR #22348 does seem related, and in particular issue #22137 that it aims to fix. The function in my example is artificial, like the function in this example; it's a reduced case, not necessarily a good helper function I would write.

So my understanding is there is one solution, which is scary, which is to make type guards basically introduce an Exclude/Extract (though I could have sworn this happens in some cases).

What about making it so that indexing a mapped type checks if you are indexing with the key type, or something assignable to the key type, something like that? Something like this must be happening for non-generics, because indexing a Dict<string> by string works. In other words, would it be incorrect to eagerly simplify Dict<K>[K] to number | undefined?

@akomm
Copy link

akomm commented Jul 11, 2019

@dgreensp
I have encountered the same issue. Given the example you provided:

function getValue<T, K extends keyof T>(partial: Partial<T>, key: K): T[K] | null {
  const value = partial[key]
  if (value !== undefined) {
    return value // error: TypeScript thinks this can be undefined...
  }
  return null
}

Basically the type of value is Partial<T>[K]. Note, it is not Partial<T>[K] | undefined. So narrowing down, removing undefined, does not change anything on the type. And since structure of Partial<T> is not known yet, the type can at this point only be described in the generic way Partial<T>[K].

@RyanCavanaugh
Given this:

type Partial<T> = {
    [P in keyof T]?: T[P];
}

What if focus is not on general type narrowing, but only related to indexed types and ?-indices, which should be equivalent for any indexed type (correct me if I am wrong). So you can narrow down undefined !== t[p] based on indexed type and index is ?. Does it change the situation/simplify it?

@stepancheg
Copy link

I have an issue which probably has the same cause:

interface Todo {
    title: string,
}

function foo(x: {[name: string]: string}) {
}

function bar(todo: Partial<Todo>) {
    foo(todo);
}

When compiled with --strictNullChecks it fails to compile with error:

t.ts:9:9 - error TS2345: Argument of type 'Partial<Todo>' is not assignable to parameter of type '{ [name: string]: string; }'.
  Property 'title' is incompatible with index signature.
    Type 'string | undefined' is not assignable to type 'string'.
      Type 'undefined' is not assignable to type 'string'.

9     foo(todo);
          ~~~~


Found 1 error.

@MartinJohns
Copy link
Contributor

@stepancheg Your type in foo says it has arbitrary properties that are all of the type string. But your Partial<Todo> type has a property that is string | undefined. You can't assign undefined to string.

@stepancheg
Copy link

@MartinJohns then I don't understand how Partial works.

So my problem can be written like this then:

interface Todo {
    title: string,
}

function foo(x: {[name: string]: string}) {
}

function bar<K extends keyof Todo>(todo: Pick<Todo, K>) {
    foo(todo);
}

function baz() {
    bar({});
    bar({title: "a"});
    // these fail as expected
    bar({title: 1});
    bar({rrr: "a"});
}

Thank you! And sorry for distraction.

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 Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants