Improve type narrowing for `.includes`

[essenmitsosse]

Given the following code:

(["a", "b", "c"] as const).includes(SOME_VALUE as 'a' | 'b' | 'd' )

we get a Typeguard that SOME_VALUE is 'a' | 'b' | 'c' — but in this case we know more about: we know it is not 'c' so it would be great to not have 'c' show up in the result again. The typeguard should narrow to 'a' | 'b'

One naive way to implement it would be something like:

interface ReadonlyArray<T> {
  includes<TSearch extends T | (TSReset.WidenLiteral<T> & {})>(
    searchElement: TSearch,
    fromIndex?: number,
  ): searchElement is T & TSearch;
}

Diff:

interface ReadonlyArray<T> {
-  includes(
-    searchElement: T | (TSReset.WidenLiteral<T> & {}),
+  includes<TSearch extends T | (TSReset.WidenLiteral<T> & {})>(
+    searchElement: TSearch,
    fromIndex?: number,
-  ): searchElement is T;
+  ): searchElement is T & TSearch;
}

Is there any reason this shouldn't be done? If not I'd be glad to open a PR.

[helmturner]

Oh, this is good!

const test = [1, 2, 3] as const;
const test2 = [...test, 4] as const;
const test3 = [0, 2, 3] as const;

let val = 0;

if (test2.includes(val)) {
  console.log(val) // 1 | 2 | 3 | 4
  if (!(test.includes(val))) {
    console.log(val) // 4
  }
}

if (test.includes(val)) {
  console.log(val) // 1 | 2 | 3
  if (!(test2.includes(val))) {
    console.log(val) // never
  }
}

if (test3.includes(val)) {
  console.log(val) // 0 | 2 | 3
  if (!(test2.includes(val))) {
    return val // 0
  }
  console.log(val) // 2 | 3
}

[mattpocock]

This seems like a good idea! I don't see why this couldn't be included. I'm going to let this sit for a while to percolate to see if I can think of any issues with this.

[essenmitsosse]

I could also do a PR and update the tests. That might surface a few issues. Should be pretty easy.

[mattpocock]

@essenmitsosse Do it!

[essenmitsosse]

I am a bit confused. I just added the tests that were supposed to fail, but it is actually already working, even without any changes to the implementation. It looks like TypeScript is creating the union by itself in those cases — there is no need for the type predicate to narrow.

I wonder why it didn't seem to work when I first tried it.

Another thing that might be answered somewhere, but I couldn't find it: Why doesn't .include on a non-read-only array have a type predicate? I am sure there is a reason, so it wouldn't make sense to worry about improving type narrowing for those arrays.

[mattpocock]

@essenmitsosse It got raised on a Twitter thread somewhere. Basically, on an array with an unknown number of members you can't guarantee that a member of the union is or isn't in the array. So you end up with some WEIRD situations where it gets inferred as 'never'.

[essenmitsosse]

For someone stumbling over this in the future: I tracked down the tweet you were talking about

function dedupe<T>(items: readonly T[]): T[] {
    const newArray: T[] = []

    for (const item of items) {
        if (newArray.includes(item)) continue
        newArray.push(item)
        //             ^? const item: never
    }

    return newArray
}

This is with the typing of Array.include prior to 7460c2ffaffe202e44e698c1613ebbf214b1b0ce, where non-readonly Arrays still returned a type predicate. via @thetarnav

[essenmitsosse]

@mattpocock What I am wondering about above code: If that is a bug, why isn't this one a bug as well?

function dedupe2<T>(items: readonly T[]): readonly T[] {
    const newArray: readonly T[] = [1 as T]

    for (const item of items) {
        if (!newArray.includes(item)) {
          console.log(item)
          //           ^? const item: never
        }
    }

    return newArray
}

While I see the problem, it seems unrelated to the Array being writable.

[essenmitsosse]

A simpler example:

const validNumbers: readonly number[] = [1, 2, 3]

const checkNumber = (items: readonly number[]): readonly string[] =>
    items.map(item => validNumbers.includes(item) ? `${item}` : `!${item}`)
    //                                                               ^? const item: never

TS Playground

[thetarnav]

imo both readonly T[] and T[] should behave the same If an array has type readonly T[] it doesn't mean that it includes ALL of T in it for the narrowing to always make sense. Try making an array that has all numbers in it. I'll wait 😄

[essenmitsosse]

That was my thought as well, but the current implementation makes a difference between readonly T[] and T[].

[essenmitsosse]

Here is an even worse example:

const validNum: ReadonlyArray<number> = [1, 5]
const example234 = [2, 3, 4] as const

example234.map((item) => validNum.includes(item) 
    ? `${item}` 
    //    ^? const item: 2, 3, 4
    : `!${item}`)
    //    ^? const item: never

In this case, even the initial type narrowing is wrong, not just the inverse.

[mattpocock]

My heuristic was that if you're creating a readonly T[], it makes sense that it includes all the members of the union. This isn't necessarily true, but it is true enough for the predicate to be useful.

It's extremely rare that you're creating ReadonlyArray<number> manually (I've never seen a use case for it).

BUT I am willing to be talked around if we can figure out a practical example.

[essenmitsosse]

This might be a coding style issue. For me, almost all Arrays are readonly (because FP), so having a readonly T[] that doesn't include all T isn't so unusual. My intuition would be that the switch for the return type between a boolean and a type predicate shouldn't happen based on whether the array is read-only. It should be determined by whether the array type is a widened type (number) or something more specific.

[essenmitsosse]

Or to put it another way: if you happen to have a readonly T[], it most certainly doesn't include all T — unless T is just a Union of a finite amount of specific members.

[essenmitsosse]

interface Order {
    listIdProduct: ReadonlyArray<number>,
}

const removeId = (order: Order, id: number) => {
    if (order.listIdProduct.includes(id) ) {
        return order.listIdProduct.filter(/* yadayadayada */);
    }

    return `ID ${id.toString()} not included in order`;
    // Property 'toString' does not exist on type 'never'
}

This example is a bit contrived, but the general idea here shouldn't be that uncommon. All you need to do is get your read-only Array from an object where it isn't supposed to be mutated, but you don't know what the specific members are.

Or just write all functions to take read-only arrays as arguments so you don't mutate them.

[schummar]

As far as I understand it, it is not solvable for general arrays because of how type guards work in TypeScript. If you check a type guard in one branch of your code (e.g. if), the type in the other branch (e.g. else) will be the "opposite".

So for a type guard implementation it's not enough to say: If the check is true, the variable has the type T. It has to be: If and only if the check is true, the variable has the type T. Or put another way. A => B is not enought, it must be A <=> B

And that is simply not the case for general arrays, as shown by the many examples in this thread.

I think however there is a subclass where it is possible to narrow: If the array is a tuple with well known entries. And that is probably the most common use case where narrowing with Array.includes is useful, so we might be in luck. E.g.

const possibleValues = ['a', 'b', 'c'] as const // readonly ['a', 'b', 'c']
declare const x: string

if (possibleValues.includes(x3)) {
  console.log(x3);
  //          ^? // 'a' | 'b' | 'c'
} else {
  console.log(x3);
  //          ^? // string
}

I have made a pull request #130. In the PR I have described a case where this doesn't work well, which might or might not be a deal breaker. Let me know what you think.

[johan]

array-index-of.d.ts shares this problem, and could use the same fix.