Allow `undefined` as the first argument of `array.includes`?

[Mathias-S]

Shouldn't it be possible to use undefined as the first argument of array.includes and array.indexOf?

Consider the following example:

function example(obj: { name?: string }) {
  // Valid & safe JS, but fails type checking:
  ["a", "b", "c"].includes(obj.name);
}

This happens quite a few times in a codebase I'm working on. Some workarounds include:

function workarounds(obj: { name?: string }) {
  // Extra JavaScript for the sake of unnecessarily satisfying TypeScript
  obj.name && ["a", "b", "c"].includes(obj.name);

  // No extra JS, but violates ESLint rule @typescript-eslint/no-non-null-assertion
  ["a", "b", "c"].includes(obj.name!);
}

The last workaround using TypeScript's ! non-null assertion works, but violates @typescript-eslint/no-non-null-assertion which is part of typescript-eslint's recommended config. Also, ! wouldn't work in the following case:

function example2(obj?: { name: string }) {
  // Readable, valid & safe JS, but fails type checking
  ["a", "b", "c"].includes(obj?.name);

  // In this case, extra JS is needed to (unnecessarily) satisfy TypeScript
  obj?.name && ["a", "b", "c"].includes(obj.name);
}

Is there any benefit to disallowing undefined as the searchElement in array.includes?

I don't see any drawbacks to allowing it, but there are some clear benefits that I believe are in line with ts-reset's philosophy.

[ArthurKa]

It's string[]. There is definitely no undefined in it.

[mattpocock]

Agree with @ArthurKa, I don't see this widening as being useful.

[twittwer]

I would agree with @Mathias-S that it would be really helpful, as it prevents additional runtime code only "caused" by the use of typescript:

function doStuff(ids: string[], activeId?: string) {
  // required TS code
  const activeIndex: number = activeId === undefined ? -1 : ids.indexOf(activeId);
  // valid JS code (with the same result)
  const activeIndex = ids.indexOf(activeId);
}

[ArthurKa]

I would agree with @Mathias-S that it would be really helpful, as it prevents additional runtime code only "caused" by the use of typescript:

function doStuff(ids: string[], activeId?: string) {
  // required TS code
  const activeIndex: number = activeId === undefined ? -1 : ids.indexOf(activeId);
  // valid JS code (with the same result)
  const activeIndex = ids.indexOf(activeId);
}

And what about

function doStuff(ids: string[], activeId?: string | number) {

Allow number also? No. It doesn't seem to be right for me. You need some point to stop widening. Just string looks good to me. Lots of JS valid code is not valid for TS type system. It's the main idea of TS. To restrict a lot of permissiveness.

[twittwer]

IMO there is a difference between allowing undefined and allowing another type.
The widening for undefined handles somehow the "optional" behaviour of the indexOf argument.