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

Nullable nested properties breaks inference for type guards #60244

Closed
leonaves opened this issue Oct 16, 2024 · 8 comments
Closed

Nullable nested properties breaks inference for type guards #60244

leonaves opened this issue Oct 16, 2024 · 8 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@leonaves
Copy link
Contributor

🔎 Search Terms

nullable items nested typeguard type guards

🕗 Version & Regression Information

  • This changed between versions 0 and 5.6.3

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.7.0-dev.20241016#code/ATAuE8AcFNgMQPYIEIEMBOwC8wDeAoEEAIwwC5gBnUdASwDsBzYAH2HoFcAbLgbkOABffgIgxgAOW5dUxLtACSoaAFtseASABmSCohQZW7afxDD8oqLAkJQUnrPlLV6gkWA6EepGnSmhIiAAxgj01MC0yioAEqiU+uoAFJGqFPYycopRAJQUKWq0lJK26Y5ZLlgAfBru6NCgHOj0EVEAdJ7AAIRYOJw8-uYCIWGgLapFOADamjXuHrqzc8Ck6BQA5AAW0DwIawA0M2YH7oLHRLjzXsY8QmcAuoHAw+FcCIwAgujoqOAA8lrOFQTYCJLQcehBQGUNIlaRlQGTO7ZbDVNxEZ4IeStV6MUHgyFRSjZAaPZ6jLS0LjKOoAEyh6nylHalOpySisXiSGJAhxn2+fwBhNBLOgtKhxKAA

💻 Code

  type FooBar = {
    bar: string | null;
  };

  type NullableItem = {
    foo: FooBar | null;
  };

  type NotNullableItem = {
    foo: FooBar;
  };

  const itemHasFoo = (item: NullableItem): item is NotNullableItem => {
    return item.foo !== null;
  };

  const items = [
    {
      foo: {
        bar: 'hello',
      },
    },
    { foo: null },
  ];

  const logArrayOfItems = (funcItems: NotNullableItem[]) => {
    console.log(funcItems);
  };

  const filteredItems = items.filter(itemHasFoo);
  logArrayOfItems(filteredItems);

🙁 Actual behavior

Type guard itemHasFoo does not narrow type to NotNullableItem, but when FooBar is changed to:

  type FooBar = {
    bar: string;
  };

It works fine, something about the nullable property of FooBar seems to stop the inference of that type correctly.

🙂 Expected behavior

I expect the typeguard to correctly narrow the type to NotNullableItem and therefore not error.

Additional information about the issue

No response

@jcalz
Copy link
Contributor

jcalz commented Oct 16, 2024

This has little to do with type guards, or even nullability. It's just assignability. The following is an error, for good reason:

declare const i: { foo: { bar: string | null } };
const j: ({ foo: { bar: string } } | { foo: null }) = i; // error!

but removing | null from i's type fixes it, also for good reason:

declare const i: { foo: { bar: string } };
const j: ({ foo: { bar: string } } | { foo: null }) = i; // okay

Those are the same types from your example, and the error in the top one is the thing that prevents TS from selecting the type guard overload for filter(). (Note, in all of the above, you can substitute null with just about any other type and the same behavior happens.)

TypeScript can't know that you intend items to be of type NullableItem[] so it doesn't recognize itemHasFoo as a valid narrowing of its elements. If you want that type, you can annotate it like const items: NullableItem[] = ⋯. I'm not seeing a TS bug anywhere here.

@leonaves
Copy link
Contributor Author

Hmm okay I get the question of assignability in this case, but why doesn’t the type guard error that I’m passing an argument of the wrong type to it if items can’t be coerced to NullableItem[]?

@jcalz
Copy link
Contributor

jcalz commented Oct 16, 2024

{ foo: { bar: string } } | { foo: null } is assignable to NullableItem, not vice versa. So filter() is happy to accept a callback of type (x: NullableItem) => void, since NullableItem is wider, and functions are contravariant in their parameters. But since NotNullableItem isn't seen as narrower than { foo: { bar: string } } | { foo: null }, then TS can't select the overload that deals with type predicates. In arr.filter(pred) where arr is of type T[] and pred is of type (x: U) => x is V, the call will succeed as long as T extends U, but it will only narrow arr if V extends T.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Oct 16, 2024
@leonaves
Copy link
Contributor Author

IMO, the typechecker should error if the type guard can't narrow. The way type guards behave in other situations very much lean towards them being the source of truth, and the syntax implies that. The return type being item is NotNullableItem to me means that in all cases if the function returns true then the item IS a NotNullableItem, if that statement isn't true, the type checker should error at the call site of the typeguard otherwise the syntax means it's telling a lie. If that's not the intention then the syntax shouldn't be a IS b because that's such a definitive statement, "oh I know it says "IS" but sometimes it isn't" is just bad user experience.

@jcalz
Copy link
Contributor

jcalz commented Oct 17, 2024

It's not that the type guard can't narrow, it's that filter() has two call signatures, one of which doesn't even attempt to narrow (not every use of filter() tries to affect the type of the array).

Maybe you're saying that the non-narrowing call signature should reject callbacks that return a type predicate, but that would make the already complicated filter() definitions even more complicated, if it's even possible.

Or maybe the narrowing call signature shouldn't care if S extends T when you call it with a callback of type value is S, but instead allow the callback parameter to be wider than T and then narrow to (S & T)[], as in

interface Array<T> {
  filter<S extends U, U extends ([T] extends [U] ? unknown : never)>(
    predicate: (value: U, index: number, array: U[]) => value is S, thisArg?: any
  ): (S & T)[];
  filter(predicate: (value: T, index: number, array: T[]) => unknown, thisArg?: any): T[];
}

which works for your use case

but again, makes the already complicated thing even more complicated. It especially hurts that you can't say U super T without #14520 so the constraint on U is just insane.

I'd say there's a valid request in here about filter(), but it's not a bug in type guards, or nullability.

@leonaves
Copy link
Contributor Author

leonaves commented Oct 17, 2024

No I agree it's not a bug in type guards, me opening this as a bug is just my lack of knowledge, but I appreciate your explanations. Though I think at a deeper level what it boils down to that feels wrong to me (which correct me if I'm wrong is why the filter overload I was expecting can't be chosen) is—to use another simpler example—that this:

  type NullableItem = {
    foo: string | null;
  };

  type NotNullableItem = {
    foo: string;
  };

  const itemHasFoo = (item: NullableItem): item is NotNullableItem => {
    return item.foo !== null;
  };

  const myThing = { foo: null };

  if (itemHasFoo(myThing)) {
    console.log(myThing.foo);
  }

Leads to myThing being reduced never, and erroring on the console.log. I don't think that makes sense to me. I think never as a type shouldn't come out of a type guard call, I think the type checker should complain instead. Maybe there's a good reason for that and or maybe that's just me, it feels like the "correct" place for that to be raised though.

@RyanCavanaugh
Copy link
Member

Leads to myThing being reduced never, and erroring on the console.log. I don't think that makes sense to me. I think never as a type shouldn't come out of a type guard call

This would just make it inconsistent with narrowing semantics. It's idiomatic and common for something to be reduced to never for one reason or another, e.g. exhaustive switch statements.

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Working as Intended" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants