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

Set.has() and Array.includes() should narrow the type of their argument to the type of this when they return true #51678

Closed
jyasskin opened this issue Nov 29, 2022 · 8 comments
Labels
Duplicate An existing issue was already created

Comments

@jyasskin
Copy link

lib Update Request

Configuration Check

My compilation target is ES2022 and my lib is the default.

Missing / Incorrect Definition

Set.has() and Array.includes() should narrow the type of their argument to the type of this when they return true (which implies they should accept wider types as arguments).

This is probably impossible until #14520 is solved, but I wanted to add a more concrete use case than just making it convenient to pass wide arguments.

Sample Code

type Ingredient = {
  amount?: string;
  unit?: string;
  name?: string;
}
const ingredientKeys = new Set(["amount", "unit", "name"] as const);

let key: string = "...";
let value: string = "...";
let result: Ingredient = {};
// Doesn't work but should:
if (ingredientKeys.has(key)) {
  result[key] = value;
}

// Works, but can't do it as a member function:
function has<T extends U, U>(set: ReadonlySet<T>, elem: U): elem is T {
  const wider: ReadonlySet<U> = set;
  return wider.has(elem);
}
if (has(ingredientKeys, key)) {
  result[key] = value;
}

Playground Link

Documentation Link

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/has

@jyasskin
Copy link
Author

Yikes, this also requires #15048 even though that behavior isn't documented in https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates.

@fatcerberus
Copy link

If you mean the requirement that type predicates be exhaustive, that is documented, albeit admittedly somewhat offhandedly:

Notice that TypeScript not only knows that pet is a Fish in the if branch; it also knows that in the else branch, you don’t have a Fish, so you must have a Bird.

which implies the type predicate must be exhaustive.

@jcalz
Copy link
Contributor

jcalz commented Nov 29, 2022

Essentially a duplicate of #36275

@MoritzKn
Copy link

MoritzKn commented Nov 29, 2022

Is this that complicated? This seems to fulfill the requirement already:

/** Shouldn't this be the type signature of Array.prototype.includes? */
function arrayIncludes<T>(arr: T[], v: unknown): v is T {
  return arr.includes(v as T);
}

I assume I am missing something but couldn't the type signature simply be changed to this?

@jcalz
Copy link
Contributor

jcalz commented Nov 29, 2022

^ #36275 (comment)

There's a code snippet in this comment that shows why you can't do this, but explicitly:

Type predicates also narrow in the false case. If Array.includes(val) returns true then sure, the type of val could be narrowed to the type of the array elements. But if it returns false, then it is rarely correct to narrow val to exclude the type of the array elements. The only time this is even remotely acceptable would be if the array element type is a union of unit/literal types, and if the array is somehow known to contain an element for each and every member of that union.

Without direct support for #14520 and #15048, writing a version of this that doesn't have weird side effects is ugly, and not anything the TS team wants to merge into the standard library. This is mostly all covered in #36275.

@fatcerberus
Copy link

fatcerberus commented Nov 29, 2022

I think this Playground illustrates the issue with that signature nicely.

Or even better, this one: Playground

@fatcerberus
Copy link

To be clear, #15048 is required for the reasons explained by @jcalz. #14520 is optional (the alternative is to type the search term as unknown), but most people still want it to be error to check for an unrelated type in an array, so strArray.includes(num) should be an error, but strArray.includes(strOrNum) shouldn't. Without #14520 you only have the choice of both being an error, or neither.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Dec 1, 2022
@typescript-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants