Type predicate in ReadonlyArray.prototype.includes may be incorrect

[mefechoel]

Hey there and thanks for this great library, I love the idea of a TS-reset. I think I've encountered some incorrect behaviour though.

This is also mentioned in #49 I believe, but since that issue is mainly about something else, I'm opening this new one.

The problem with the type predicate in ReadonlyArray.prototype.includes is that if it returns true, you know for sure, that the argument is of the same type as the array items. This does not work the other way round though. Just because the argument is not in the array, it does not necessarily mean that it is not of the same type as the array items.

The simplest (very contrieved) example I could think of is the following:

There are two union types. One of which is a subset of the other. If we have an empty ReadonlyArray of the subset type, and we check if it includes an element of the superset type, we know that the check will be come back as false. If we have an if/else branch for that case, TS will narrow the item we checked with .includes to the difference of the superset type and the subset type, which may not be true, as the item could well be of the subset type as well, it would just not be in the list.

In code it could something like this:

// Our superset type
type Code = 0 | 1 | 2;
// Our subset type
type SpecificCode = 0 | 1;
// Get a superset item that is also in the subset
const getCode = (): Code => 0;
const currentCode = getCode();
// Create an empty list of subset type
const specificCodeList: ReadonlyArray<SpecificCode> = [];

if (specificCodeList.includes(currentCode)) { // This will be false, since 0 is not in []
	currentCode; // -> SpecificCode
} else { // This branch will be entered, and ts will think z is 2, when it is actually 0
	currentCode; // -> 2
}

I've also created a more real-worldy example in the ts playground using language codes and http request semantics to get closer to an actual use case.

I share your opinion from #49 (comment) about ReadonlyArrays mostly being used in an exhaustive way, including all members of a union type, but that is not gererally true. I guess that if you'd encounter such an edge case we're this problem occurres it would be very hard to understand what's going on. That's why I would be more comfortable with a few more manual checks and ReadonlyArray.prototype.includes not being a type predicate unless there actually is a way of having it only be a type predicate if the result is truthy (that would be amazing!). Another option would be to remove this version from the recommended set of resets and having it as an opt in improvement. The base version without the type predicate could still be in the recommended resets and be overridden with the current implementation with a second import of this more altered version.

[mattpocock]

Yes, I think I agree with you. This was initially raised on Twitter in relation to normal arrays, but I think the same behaviour applies here.

I think we should probably revert the type predicate behaviour here.