[feature request] Better `Array.prototype.includes`

[ssssota]

The standard TypeScript type requires the first argument of the includes function to be a value in an array. This requires that the type of the argument be known beforehand for union-type arrays (e.g. [1,2,3] as const). We can see that this is not a semantic error, but we may find this inconvenient.

So please consider introducing the following types into this project:

interface Array<T> {
  /**
   * Determines whether an array includes a certain element, returning true or false as appropriate.
   * @param searchElement The element to search for.
   * @param fromIndex The position in this array at which to begin searching for searchElement.
   */
  includes(searchElement: any, fromIndex?: number): searchElement is T;
}

interface ReadonlyArray<T> {
  /**
   * Determines whether an array includes a certain element, returning true or false as appropriate.
   * @param searchElement The element to search for.
   * @param fromIndex The position in this array at which to begin searching for searchElement.
   */
  includes(searchElement: any, fromIndex?: number): searchElement is T;
}

[uhyo]

Thank you for suggestion!

I see that the suggested signatures make include much more convenient. It would indeed start to allow some mistakes (like strings.includes(num)), but it wouldn't deviate from type safety.

Let's add this.

[graphemecluster]

@uhyo Wait – By adding this it means that we will also need to deal with methods like findIndex – see https://github.com/microsoft/TypeScript/issues/36554#issuecomment-1196915820 and https://github.com/microsoft/TypeScript/issues/26255. To maintain type safety, perhaps we can use a generic until https://github.com/microsoft/TypeScript/issues/9252 is implemented:

interface Array<T> {
  includes<U>(searchElement: T extends U ? U : T, fromIndex?: number): searchElement is T extends U ? T : T;
}

[uhyo]

@graphemecluster Thank you for pointing this out. Personally I wouldn't extend this to methods like findIndex because we need to corrently type callback parameters. Anyway the generics approach is nice 🙂

[uhyo]

I tacked this and couldn't get to a perfectly working implementation. 😇 As mentioned above, we should only narrow in then-branch of include checks but it doesn't seem currently possible. Concretely the below testcase cannot pass.

  function checkNarrowing(val: "pika" | "chu" | "pikachu") {
    const strarr: ("pika" | "chu")[] = [];
    if (strarr.includes(val)) {
      expectType<"pika" | "chu">(val);
    } else {
      expectType<"pika" | "chu" | "pikachu">(val);
    }
  }

[aaditmshah]

I tacked this and couldn't get to a perfectly working implementation. :innocent: As mentioned above, we should only narrow in then-branch of include checks but it doesn't seem currently possible. Concretely the below testcase cannot pass.

  function checkNarrowing(val: "pika" | "chu" | "pikachu") {
    const strarr: ("pika" | "chu")[] = [];
    if (strarr.includes(val)) {
      expectType<"pika" | "chu">(val);
    } else {
      expectType<"pika" | "chu" | "pikachu">(val);
    }
  }

I think it's all right to narrow the type in the else branch as well. If strarr.includes(val) is false then we know that val can't be "pika" or "chu". Hence, the type of val can be narrowed to just "pikachu".

Consider the following example. Playground Link

const expectType = <A>(a: A): A => a;

type Subtype<A, B> = A extends B ? A : never;

type Supertype<A, B> = A extends B ? B : unknown;

interface Includes<A> {
    includes: <B>(searchElement: Supertype<A, B>) => searchElement is Subtype<A, B>;
}

declare const array: Includes<"foo" | "bar">;

declare const subtypeElement: "foo";

declare const sametypeElement: "foo" | "bar";

declare const supertypeElement: "foo" | "bar" | "baz";

declare const othertypeElement: "baz";

if (array.includes(subtypeElement)) {
    expectType<"foo">(subtypeElement);
} else {
    expectType<never>(subtypeElement);
}

if (array.includes(sametypeElement)) {
    expectType<"foo" | "bar">(sametypeElement);
} else {
    expectType<never>(sametypeElement);
}

if (array.includes(supertypeElement)) {
    expectType<"foo" | "bar">(supertypeElement);
} else {
    expectType<"baz">(supertypeElement);
}

if (array.includes(othertypeElement)) {
    expectType<never>(othertypeElement);
} else {
    expectType<"baz">(othertypeElement);
}

[aaditmshah]

Oh, wait. No, I'm mistaken. Didn't see that strarr is empty here. Nevermind. Disregard my previous comment.

[aaditmshah]

@ssssota You might not require narrowing at all. Consider using the following find function instead of Array#includes.

Playground Link

type Option<A> = { some: A } | null;

const find = <A>(needle: unknown, haystack: A[]): Option<A> => {
    for (const element of haystack) {
        if (element === needle) {
            return { some: element };
        }
    }
    return null;
};

declare const haystack: ("foo" | "bar")[];

declare const needle: "foo" | "bar" | "baz";

const option = find(needle, haystack);

const expectType = <A>(a: A): A => a;

if (option !== null) {
    expectType<{ some: "foo" | "bar" }>(option);
    expectType<"foo" | "bar" | "baz">(needle);
} else {
    expectType<null>(option);
    expectType<"foo" | "bar" | "baz">(needle);
}

[ssssota]

@aaditmshah Of course! This Issue was created only as a "nice to have". Naturally, it cannot be used in actual coding, so we have taken workarounds. @uhyo If this issue is not feasible, please feel free to close it. The discussion here has been very useful to me. Thank you!

[graphemecluster]

I just revisited this and thought of a solution (which’s overly ordinary that you may even have thought of it before) while replying @cm-ayf’s ~post~ tweet: https://twitter.com/graphemecluster/status/1700470364000956844 However, the defect is that this doesn’t give any error directly; it just types the parameter never instead if T is not related to U. I am not sure if there are any pitfalls though. I have tried producing an error if T & U is never but none of my trials work. So I guess it’s still the best to play around with the compiler – my first thought is to add type ErrorIfNever<T> = T; to a library file and let the compiler handle the remaining things. _{お二人とも東大生だ、憧れます……}

[aaditmshah]

I don't think it's possible to refine the type of the searchElement. However, we can at least widen the type of the input.

interface Array<T> {
  includes<U>(searchElement: T extends U ? U : T, fromIndex?: number): boolean;
}

This would allow you to write the following.

declare const array: ("foo" | "bar")[];

declare const searchElement: "foo" | "bar" | "baz";

if (array.includes(searchElement)) {
    expectType<"foo" | "bar" | "baz">(searchElement);
} else {
    expectType<"foo" | "bar" | "baz">(searchElement);
}

This works because "foo" | "bar" | "baz" is a supertype of "foo" | "bar".

However, it would prevent the following.

declare const array: ("foo" | "bar")[];

declare const searchElement: "baz";

if (array.includes(searchElement)) { // type error
    expectType<"baz">(searchElement);
} else {
    expectType<"baz">(searchElement);
}

This doesn't work because "baz" is neither a supertype nor a subtype of "foo" | "bar".

Playground Link

[ehoogeveen-medweb]

I think the above might be too permissive, because it allows search elements like string | { some: "random object" } as they extend the array type. From reading other discussions on this topic, that seems to be what the typescript developers are trying to avoid.

I'd like to suggest the following modification, which allows search elements like string for string literal tuples while disallowing unrelated union types:

type IfLiteralGetPrimitive<T> =
  T extends string ? string : T extends number ? number : T extends bigint ? bigint : T extends boolean ? boolean : T;

interface Array<T> {
  includes<U extends IfLiteralGetPrimitive<T>>(searchElement: T extends U ? U : T, fromIndex?: number): boolean;
}

Playground Link (I modified the original playground because it worked a bit differently and didn't permit the union - this playground matches the behavior I see when I add the overload to the global Array interface)

[graphemecluster]

@ehoogeveen-medweb This would prevent searching an array of strings for a PropertyKey.

[ehoogeveen-medweb]

PropertyKey as in string | number | symbol? Well, I guess you could modify IfLiteralGetPrimitive to something like

type IfLiteralGetPrimitive<T> =
  T extends string ? number | string | symbol : T extends number ? number : T extends bigint ? bigint : T extends boolean ? boolean : T;

so that all object keys are allowed when an array contains strings. I usually work with somewhat narrow object types like Record<string, ...> and it's not needed for those.

Playground link

Edit: I'm also not sure searching an array for a full PropertyKey really comes up in practice, as you usually get the keys through a method like Object.keys which only returns string keys. But maybe I'm missing something there.

[karlismelderis-mckinsey]

would it be possible treat searchElement as unknown? I can imagine that before .includes call you don't really know what that value is or should not even care

it's totally valid case to do this and build, e.g. a type guard from it

const nums = [1,2,3] as const
nums.includes(4)

[KisaragiEffective]

I tacked this and couldn't get to a perfectly working implementation. 😇 As mentioned above, we should only narrow in then-branch of include checks but it doesn't seem currently possible. Concretely the below testcase cannot pass.

  function checkNarrowing(val: "pika" | "chu" | "pikachu") {
    const strarr: ("pika" | "chu")[] = [];
    if (strarr.includes(val)) {
      expectType<"pika" | "chu">(val);
    } else {
      expectType<"pika" | "chu" | "pikachu">(val);
    }
  }

Not sure if this is best thing that we can have, but somehow works:

declare const __tag: unique symbol;

interface Array<T> {
  includes(searchElement: unknown, fromIndex?: number): searchElement is T & {[__tag]: never};
}

function checkNarrowing(val: "pika" | "chu" | "pikachu") {
    const strarr: ("pika" | "chu")[] = [];
    if (strarr.includes(val)) {
            val;
        // ^?
    } else {
            val;
        // ^?
    }
}

playground.

[KisaragiEffective]

I think the above is one of the best thing that we can have, as same "properties" are shown in following minimized case: