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

Support ReadonlyArray.includes as a type guard #31018

Closed
5 tasks done
ExE-Boss opened this issue Apr 18, 2019 · 30 comments
Closed
5 tasks done

Support ReadonlyArray.includes as a type guard #31018

ExE-Boss opened this issue Apr 18, 2019 · 30 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Apr 18, 2019

Search Terms

  • ReadonlyArray type guard

Suggestion

For cases where ReadonlyArray's type argument is a subtype of string or some other type handled by as const, it’s useful to be able to do:

const VALID_VALUES = ['a' | 'b'] as const;
function doStuffIfValid(x: string) {
	if (VALID_VALUES.includes(x)) {
		// $ExpectType 'a' | 'b'
		x
	}
}

Note that the above produces an error on line 3 about string not being assignable to 'a' | 'b' in the VALID_VALUES.includes(x) call.

Use Cases

I want to use this to improve type guards within code that interacts with untyped third‑party JavaScript.

Examples

some-module/index.ts

export type ValidValues = 'a' | 'b';
const VALID_VALUES = ['a' | 'b'] as const;

export function doStuffIfValid(thing: Record<string, string>, x: ValidValues): void;
export function doStuffIfValid(thing: Record<ValidValues, Record<string, string>>): void;

export function doStuffIfValid(thing: unknown, x?: unknown) {
	if (typeof x === 'string') {
		if (!VALID_VALUES.includes(x)) {
			throw new TypeError(`x must be one of ${VALID_VALUES} or undefined, got ${x}`);
		}
		// Do something where x is 'a' | 'b'
	} else if (typeof x !== 'undefined') {
		throw new TypeError(`x must be one of ${VALID_VALUES} or undefined, got ${x}`);
	}
}

Right now, VALID_VALUES has to be expressed as:

const VALID_VALUES = ['a' | 'b'] as unknown as TypeGuardReadonlyArray<ValidValues>;
interface TypeGuardReadonlyArray<T> extends ReadonlyArray<T> {
	includes(searchElement: unknown, fromIndex?: number): searchElement is T;
}

for the above to not cause compilation errors and the type guard to work properly.

Note that this only works correctly when T is a literal union type.

untyped-third-party-code.js

const stuff = require('some-module');

// Doesn't throw
stuff.doStuffIfValid({}, 'a');
stuff.doStuffIfValid({}, 'b');
stuff.doStuffIfValid({a: {}, b: {}});

// Throws during runtime, as this file is untyped third-party code.
stuff.doStuffIfValid({}, 'c');

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Apr 18, 2019
@RyanCavanaugh
Copy link
Member

Without #15048, this is a problematic change. Consider:

const someStrings: ReadonlyArray<string> = ["a", "b", "c"];

function fn(x: number | string) {
  if (someStrings.includes(x)) {
    //
  } else {
    // here, x is assumed to be 'number', but that's wrong!
  }
}
fn("d");

@ExE-Boss
Copy link
Contributor Author

That’s why I only want this to apply when T is a literal union type (i.e. the result of as const).

@RyanCavanaugh
Copy link
Member

That has the same problem though. A type is always a bound on a value, not an exact specification

const someStrings: ReadonlyArray<"a" | "b" | "c"> = ["a"];

function fn(x: "b" | "x") {
  if (someStrings.includes(x)) {
    //
  } else {
    // here, x is wrongly assumed to be "x"
  }
}
fn("b");

@ExE-Boss
Copy link
Contributor Author

Yeah, but you probably shouldn't be doing const someStrings: ReadonlyArray<"a" | "b" | "c"> = ["a"];, which is why I suggested this.

My use case is const someStrings = ["a"] as const.

@typescript-bot
Copy link
Collaborator

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

@dacapo1142
Copy link

I encountered the same case.
Here is my workaround:

const OS_TYPE = ['Linux', 'macOS', 'Windows'] as const
type OSType = typeof OS_TYPE[number]

//user defined type guard
function isOSType(s: string):s is OSType{
    if ((OS_TYPE as any as string[]).includes(s))
        return true;
    return false;
}

as any as string[] seems like a poor idea? Is there a better way?

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Nov 7, 2019

Try as readonly string[], or if that fails, .includes(s as any).

@finnmerlett
Copy link

Here is a generic version of the typeguard:

const isInArray = <T, A extends T>(
  item: T,
  array: ReadonlyArray<A>
): item is A => array.includes(item as A);

@artem-popov
Copy link

artem-popov commented Aug 18, 2021

I'm using workaround:

type Status = 'idle' | 'loading' | 'ready' | 'error';
const statuses: Status[] = ['idle', 'loading', 'ready', 'error'];

const setStatus = status => {
          const domStatus = script.getAttribute('data-status');
          const domStatusGuarded = statuses.filter(item => item === domStatus).find(Boolean);

          return domStatusGuarded || status;
 };

@f0rmat1k
Copy link

f0rmat1k commented Oct 8, 2021

@RyanCavanaugh i would like to make that:

const someStrings: = ["a", "b", "c"] as const;

function fn(x: number | string) {
  if (someStrings.includes(x)) {
    // 'a' | 'b' | 'c'
  } else {
    // ok, still string | number, why not?
  }
}
fn("d");

@frux
Copy link

frux commented Oct 8, 2021

I faced the same issue and found a workaround which is very simillar to the one @artem-popov suggested. But I use one .find method instead of .filter + .find.

const myList = ['1', '2'] as const;

function doSomething(value: string) {
    // dirty hack to check if the value is one of entries of the list
    const _value = myList.find(listValue => listValue === value);

    if (_value) {
        // _value is "1" | "2"
    } else {
        // _value is any other string
    }
}

@frux
Copy link

frux commented Oct 8, 2021

BTW I don't understand why this issue was closed. Is there any sensible reasons why Array.prototype.includes can not be a typeguard?

@devuxer
Copy link

devuxer commented Oct 23, 2021

I agree this should reopened for the simple reason that the item passed to includes does not need to be the type of the array, it just needs to be possibly the same type as the array. So, if you know you have an array of strings and you want to test if a number or string belongs the array, you shouldn't have to do any casting, it should just work. If you want to test a Date, however, that's indicative of incorrect code and should be an error.

So, TypeScript strictness here doesn't actually improve the type safety of the code, it just makes it inconvenient to write the code.

Also, I completely don't understand @RyanCavanaugh's example where he says, "here, x is assumed to be 'number', but that's wrong!" I don't know who is doing the assuming here, but the compiler does not think x is a number, it thinks it's a number | string.


Meanwhile, here is my current workaround.

function belongsToArray<TValue>(value: unknown, allowedValues: ReadonlyArray<TValue>): value is TValue {
    return (allowedValues as ReadonlyArray<unknown>).includes(value);
}

I prefer casting the array to ReadOnlyArray<unknown> because I believe this is guaranteed to be a legitimate cast, and it avoids any.

@Sleepful
Copy link

Sleepful commented May 13, 2022

Facing this right now, instead I have to do this work-around:

	const notSureIfValid = 'some value'
    const valid = ARRAY_OF_VALID_VALUES.find(
      (c): c is typeof ARRAY_OF_VALID_VALUES[number] => c ===  notSureIfValid
    )
    // now you can check valid for `undefined` and you will get what you want
    if (!valid) { return }
	// typescript properly discriminates the 'undefined' value and the rest are left in the type

so verbose...

@f0rmat1k
Copy link

@Sleepful is is not a cool in my opinion. Almost any

@fcostarodrigo
Copy link

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: T, fromIndex?: number): boolean;
}

The type of searchElement in the includes function shouldn't be T.

This type locks you on only passing values that belong to the array and the return of includes becomes only true instead of boolean. But you want to do things like [1, 2].includes(x), you know that the array only has 1 and 2 and maybe the type of x is number and you just want to check if x, besides being a number, is actually 1 or 2.

So maybe a better type is

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 ReadonlyArray<T>[number];
}

@doronhorwitz
Copy link

This article explains the issue nicely and offers a pretty good helper as a workaround: https://fettblog.eu/typescript-array-includes/ (see under the "Option 2: A helper with type assertions" header)

@f0rmat1k
Copy link

f0rmat1k commented Jun 21, 2023

This article explains the issue nicely and offers a pretty good helper as a workaround: https://fettblog.eu/typescript-array-includes/ (see under the "Option 2: A helper with type assertions" header)

it uses casting el as T so it's not fair check

@doronhorwitz
Copy link

This article explains the issue nicely and offers a pretty good helper as a workaround: https://fettblog.eu/typescript-array-includes/ (see under the "Option 2: A helper with type assertions" header)

it uses casting el as T so it's not fair check

It doesn't need to be a fair check since it's a workaround. What's important is that the article explains well this particular shortcoming of Typescript and offers a cute workaround that's helpful until this shortcoming is fixed (or not).

@f0rmat1k
Copy link

f0rmat1k commented Jun 27, 2023

@doronhorwitz my point is any workaround is bad in typescript. Since you use as Something you can just cheat cast like:

const VALID_VALUES = ['a', 'b'];

function doStuffIfValid(x: string) {
	if (VALID_VALUES.includes(x)) {
		(x as unknown as typeof VALID_VALUES) // now x is typeof VALID_VALUES
	}
}

If you hid the casting somewhere deep, it's not better then any casting 'in place'. And this issue is from people, who don't like casting, because it's turning off fair checks and you can get runtime error. So pls stop offering cheats. We like typescript because it offers fair check (is most situations :-) )

@doronhorwitz
Copy link

@doronhorwitz my point is any workaround is bad in typescript. Since you use as Something you can just cheat cast like:

const VALID_VALUES = ['a', 'b'];

function doStuffIfValid(x: string) {
	if (VALID_VALUES.includes(x)) {
		(x as unknown as typeof VALID_VALUES) // now x is typeof VALID_VALUES
	}
}

If you hid the casting somewhere deep, it's not better then any casting 'in place'. And this issue is from people, who don't like casting, because it's turning off fair checks and you can get runtime error. So pls stop offering cheats. We like typescript because it offers fair check (is most situations :-) )

"Cheats"... well that escalated quickly.

@julian-kingman-lark
Copy link

@RyanCavanaugh i would like to make that:

const someStrings: = ["a", "b", "c"] as const;

function fn(x: number | string) {
  if (someStrings.includes(x)) {
    // 'a' | 'b' | 'c'
  } else {
    // ok, still string | number, why not?
  }
}
fn("d");

Love this solution, IMO this makes intuitive sense, any chance this could be opened?

@loberton
Copy link

@RyanCavanaugh i would like to make that:

const someStrings: = ["a", "b", "c"] as const;

function fn(x: number | string) {
  if (someStrings.includes(x)) {
    // 'a' | 'b' | 'c'
  } else {
    // ok, still string | number, why not?
  }
}
fn("d");

Love this solution, IMO this makes intuitive sense, any chance this could be opened?

Bumping this. Still aggravating that we have to cast around a very legitimate use case for includes. Closing this with the reasoning given so far is not sufficient, please do better for the TS community.

@RyanCavanaugh
Copy link
Member

The reasoning is correct, not sure why people are mad at me. The feature described in that comment is #15048. Without that functionality, there's no way to do what's being proposed -- includes is just a method same as any that you might write in your code; there is no special sauce in the type system that causes includes to be/not be a type guard, and we don't intend to start adding weird special-cases like that which aren't reproducible in user types.

@loberton
Copy link

loberton commented Feb 21, 2024

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: T, fromIndex?: number): boolean;
}

The type of searchElement in the includes function shouldn't be T.

This type locks you on only passing values that belong to the array and the return of includes becomes only true instead of boolean. But you want to do things like [1, 2].includes(x), you know that the array only has 1 and 2 and maybe the type of x is number and you just want to check if x, besides being a number, is actually 1 or 2.

So maybe a better type is

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 ReadonlyArray<T>[number];
}

The issue I and I believe several others have in this discussion is that searchElement is not inherently a type that is defined by the const array that includes is being called on. There's a legitimate use case where the parameter is of the same primitive type (string, number, etc) but is otherwise unknown.

In other words, I do not believe that this should result in a compiler error (playground link)

const myArr = ["one", "two", "three"] as const;
const someString: string = "three";

myArr.includes(someString);

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Feb 21, 2024

That's a different problem than whether or not includes is a type guard. See #26255 and #26255 (comment)

I do not believe that this should result in a compiler error

This is a fine belief to have, and you are free to write

interface Array<T> {
  includes(arg: any): boolean;
}

in a global .d.ts in your project to get that behavior.

@spoike
Copy link

spoike commented Mar 13, 2024

To be more precise/dynamic in this case the argument may be of type unknown (as includes pretty much don't need to know what the type is) and type predicate so TS may infer the argument to the type you want:

interface ReadonlyArray<T> {
    includes(arg: unknown, fromIndex?: number): arg is T
}

Playground sample

@loberton
Copy link

Oh duh. Yeah, unknown appears to be what I needed. I refactored your sample a touch to use constants and a little stricter typing, but the includes definition is unchanged and what I intend to use. Thank you @spoike!! 😊

Playground sample

@karlismelderis-mckinsey

I was burned by same issue

would it make sense to update public node ReadonlyArray & Array include types to accept unknown?

@Oblarg
Copy link

Oblarg commented Apr 11, 2024

This should be reopened; this is a feature gap and there's absolutely no reason other than "we don't feel like prioritizing this" not to address it.

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