Repository navigation
Array method definition revamp: Use case collection #36554
Description
We've gotten numerous issue reports and PRs to change the methods of Array, particularly reduce, map, and filter. The built-in test suite doesn't cover these very well, and these methods interact with each other and the surrounding contextual type in fairly subtle ways.
Jack Bates (@jablko) has done a great job at #33645 collecting a variety of issues into a single PR; we need to augment this PR (or something like this) with a proper test suite so we can be sure about what's being changed.
I'd like to create a clearinghouse issue here to collect self-contained (I CANNOT POSSIBLY STRESS THIS ENOUGH, SELF-CONTAINED, DO NOT IMPORT FROM RXJS OR WHAT HAVE YOU) code samples that make use of the array methods.
Please include with your snippet:
- Compiler settings
- Compiler version you were testing with
- The expected behavior (examples that should and should not produce errors are both useful)
- No imports or exports; snippets need to be self-contained so that we can put them into our test suite without extra complications
Once we've established a critical mass of code snippets, we can start combining the existing PRs into an all-up revamp and assess its impact to real-world code suites to figure out which changes don't result in unacceptable breaking changes.
self-contained
Activity
RyanCavanaugh commentedon Jan 31, 2020
From #19535:
concatshould reflect the flattening of its input argument. Tested on 3.8-beta with target: ESNext, strict onRyanCavanaugh commentedon Jan 31, 2020
From #24579:
flatshould, well, flatten. Tested on 3.8-beta with target: ESNext, strict onRyanCavanaugh commentedon Jan 31, 2020
From #26976:
concatshould at least allow an empty array as a target. Arguably it should allow heterogenous operations? Tested on 3.8-beta with target: ESNext, strict onRyanCavanaugh commentedon Jan 31, 2020
From #29604:
flatshouldn't produceany[](?!),Array.prototype.concatshould be better if possible. Tested on 3.8-beta with target: ESNext, strict onRyanCavanaugh commentedon Jan 31, 2020
From a real-world code suite broken by #33645:This should error (previously misidentified as "should not error"):
RyanCavanaugh commentedon Jan 31, 2020
From a real-world code suite broken by #33645:
This one should be simpler to demonstrate exactly what aspect of things got broken
falsandtru commentedon Feb 1, 2020
falsandtru commentedon Feb 1, 2020
Ryan Cavanaugh (@RyanCavanaugh) This is caused by a mistake of the order of overloads. I made #36570 and looks like it makes no regression.
jablko commentedon Feb 3, 2020
Ryan Cavanaugh (@RyanCavanaugh) Shouldn't it? If
Aisstring[],arris[["a"]]andelis["b"]thenadd(arr, el)will return[["a"], "b"], which doesn't match the return typeArray<A>?Should that implementation be updated ->
return arr.concat([el])?RyanCavanaugh commentedon Feb 3, 2020
Jack Bates (@jablko) good point. The actual code was in
fp-tsand indeed they've removed it; the implementation now uses a loop instead https://github.com/gcanti/fp-ts/blob/master/src/Array.ts#L373jablko commentedon Feb 3, 2020
Ryan Cavanaugh (@RyanCavanaugh) I think this should be an error? The reason it currently doesn't error is because
option: anyis assignable toConcatArray<never>(atconst path = ancestor.concat(option)). However the only way the return typenever[]is accurate is ifoptionis[].I think the
flattenTree()signature needs to beancestor: any[] = [], to be callable with theconst path = ancestor.concat(option), whereoptioncan be something other than[]? That, or the signature must beoption: never[].45 remaining items
ackvf commentedon Jul 27, 2022
lib.es2016.array.include.d.tsshould not produce error when testing existence of an element that comes from a wider set of values that intersects withArray<T>.playground link
In this example, the result of
test2andtest4is undesired as the test potentially can yieldtruevalue, much like the following snippet.Disputably, some people may even argue that even
test3should not produce an error. ¯\(ツ)/¯whzx5byb commentedon Aug 14, 2022
Qwerty (Vítězslav Ackermann Ferko) (@ackvf) See discussion in #14520.
nevnein commentedon Aug 29, 2023
I hope this is not out of scope, but when one of the mentioned array methods is called on a tuple of length
n, the index parameter could be inferred as0 | 1 | … | n -1instead of justnumber. TS already knows the tuple length, and this could be really useful when accessing other tuples of the same length andnoUncheckedIndexedAccessis set totrue.Compiler settings: default, but with
noUncheckedIndexedAccessset totrueCompiler version: 5.2.2
Example:
Here is the playground
Clindbergh commentedon Apr 9, 2024
Filterdoes not work correctly with an array of unions:Expected
The error is
Actual
Typescript Version: 5.2.2
Related: #38380.
karlismelderis-mckinsey commentedon Jun 20, 2024
I would appreciate if Array.includes would behave like this:
icecream17 commentedon Jun 20, 2024
it depends, the most you can guarantee in general is
searchElement is T ? boolean : falsesince even if the type matches it's still possible for includes to return false:Retsam commentedon Jun 21, 2024
Karlis Melderis (@karlismelderis-mckinsey) Yeah this has been suggested before e.g. #31018 and there's some issues. One is, like Steven Nguyen (@icecream17) said it can return false even when
searchElementactually isT- which would require #15048: a false result on theincludescheck can't be used to prove thatsearchElementisn'tT.But also, I think widening
searchElementfromTtounknownis problematic for other use cases. There's two reasons to useincludes:if(colorArray.includes("red"))if(["red", "blue", "green"].includes(maybeColor))The type-guard signature you suggest is useful for the second case, but makes the first case worse: if
type Color = "red" | "blue" | "green"andcolorArrayisColor[], it's not great ifcolorArrray.includes("read")is accepted: that's a typo that's currently caught but wouldn't be ifincludestookunknownas its argument.Personally, I suggest making a utility function and using it in place of
arr.includes(val):dsongdani commentedon Nov 28, 2024
Given the conversation about
includesabove, I'll drop this here, but maybe I should be filing something new (happy to do that if we think it's useful). I haven't been able to find a similar complaint, but the terms I've tried searching for are so generic that they probably aren't terribly useful on their own.I was surprised that array methods, most notably
includes, don't widen when you annotate the array definition withsatisfies.TS Playground
It seems reasonable to expand the argument of
includesto whatever has been claimed by thesatisfiespredicate. Maybe this won't be true for all array methods.Retsam commentedon Nov 28, 2024
(For convenience
type Sister = "Louisa" | "Isabella" | "Mirabel")@dsongman Are you saying that the
sistersarray should be typed asSister[](which is the current type), but somehow remember that it was defined assatisfies Characters[]and change the signature ofincludes?I don't think there's any plausible mechanism for that - ultimately the methods of arrays are defined by normal type system definitions. In this case:
In this case
TisSisterand so trying to passCharactersassearchElementdoesn't work, for the reasons specified above, andsatisfiescan't change that. EitherTisSister(as in the above code) or you can doconst sisters: Characters[] = /*...*/to have TbeCharacters`, but I don't think there's a way to have both.dsongdani commentedon Nov 28, 2024
Yeah, that's right. Backing up to make sure I'm stating the problem and not necessarily the solution…
I find that in the application codebases I've worked in, a very frequent issue that comes up is wanting to define a list of values via a union and then iterating over those values in the UI somewhere. If you're okay with duplication, you can make a type that ensures an array contains all values of a union. If you want a single source of truth, though, the general consensus seems to be to define an array of things, add
as constto it, and then you can derive the union viatypeof myThing[number].If, however, you want to define an iteratable subset of a union, there isn't a great solution. Using
satisfies SuperType[]does a lot:SuperTypeas you type them.SuperType[]orSuperType. This seems to imply that thesatisfiespredicate holds, since the same is not true for theas constversion of the same array (see this playground).But while you can pass the subtype to functions that accept
SuperType, you can't useincludesto see if a member ofSuperTypeis in the subtype, which seems odd.Retsam commentedon Nov 28, 2024
@dsongman Yeah, like I said, I just don't think there's a plausible mechanism for making that happen - it seems like a good use of
satisfies, but there isn't any mechanism by whichsatisfiescould attach extra information to the type that could be used as a supertype.The difference between the
constversion and thesatisfiesversion is that while thesatisfiesversion infers asSister[], theconstversion infers asreadonly ['Louisa', 'Isabella', 'Mirabel']. Theas constis doing two things here:On the other hand,
satisfies Characters[]only does #1 - you can get the same result withas constif you apply it directly to the literals:And you get an error with
printCharacters(constSisters);only becauseprintCharactersrequires a mutable array, if you change it to(characters: readonly Characters[]) =>bothconstSistersandsatisfactorySisterswork. (Saying that a function takesreadonly T[]doesn't prevent it from accepting mutable arrays as well, it just says that it doesn't need to be able to mutate the array - in theory any function that doesn't mutate its input array probably should be typed withreadonly T[]orReadonlyArray<T>)The point is that there isn't any lingering 'metadata' where the type system 'remembers' the
satisfiesconstraint later: it just changes the type of the array when defined. And if your array is typed asSister[], thenincludesis going to require aSister.(I recommend using the
includesutility described in my earlier comment #36554 (comment) )It's not impossible that
includesmight change someday to accept supertypes, but I don't thinksatisfiescan be used to implement that sort of logic.luukvhoudt commentedon Jun 6, 2025
Array.shift()andArray.pop()always return an union of all array item types, even when each array item has its own type. I this case I rather expect to get the type specific to the array item being returned. So forshiftthe first item type, forpopthe last:TS Playground
icecream17 commentedon Jun 6, 2025
Luuk van Houdt (@luukvhoudt)