Repository navigation
Array method definition revamp: Use case collection #36554
Description
Activity
From #19535:
concatshould reflect the flattening of its input argument. Tested on 3.8-beta with target: ESNext, strict onconst foo: [number, string][] = [[1, 'one']]; const a = foo.concat([2, 'two']); // SHOULD ERROR, // the actual content of 'a' is [[1, 'one'], 1, 'two'] // so a[1] should be number | string | [number, string] a[1][0];
Reacted by satoren, ExE Boss, Maarten Zuidhoorn, dnalborczyk and Chris MarxFrom #24579:
flatshould, well, flatten. Tested on 3.8-beta with target: ESNext, strict ondeclare let arr: [[number, boolean], string]; let x0 = arr.flat(0); // Should be [[number, boolean], string] or (number | boolean | string)[] let x1 = arr.flat(1); // Should be [number, boolean, string] or (number | boolean | string)[] let x2 = arr.flat(2); // Should be [number, boolean, string] or (number | boolean | string)[]
Reacted by Shawn McKnight, Luke, ExE Boss, Maarten Zuidhoorn and dnalborczykFrom #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 on// Should be OK (currently an error) and produce string[] let a1 = [].concat(['a']); // Should be OK (is) and continue to produce string[] let a2 = ['a'].concat(['b']); // Should either error (current behavior) or maybe produce (string | number)[] let a3 = [1].concat(['a']);
Reacted by Jordan Harband, hui21109 and Dmytro ParzhytskyiFrom #29604:
flatshouldn't produceany[](?!),Array.prototype.concatshould be better if possible. Tested on 3.8-beta with target: ESNext, strict on// Should be an error const a: boolean[] = [[17], ["foo"]].flat(); // Should be an error (stretch goal) const b: boolean[] = Array.prototype.concat([17], [19], [21]);
From a real-world code suite broken by #33645:This should error (previously misidentified as "should not error"):
// Should error because add([["a"]], ["b"]) will not produce a string[][] function add<A>(arr: Array<A>, el: A): Array<A> { return arr.concat(el) }
From a real-world code suite broken by #33645:
// Did not error; probably shouldn't in the future either class A { flattenTree(option: any, changeOnSelect: any, ancestor = []) { let flattenOptions: any = []; const path = ancestor.concat(option); flattenOptions = flattenOptions.concat(this.flattenTree(option, changeOnSelect, path)); } }
This one should be simpler to demonstrate exactly what aspect of things got broken
([] as any[]).reduce(() => 0, 0); // Expected: number, Actual: any ([] as unknown[]).reduce(() => 0, 0); // Expected: number, Actual: unknown ([] as never[]).reduce(() => 0, 0); // Expected: number, Actual: number
Ryan Cavanaugh (@RyanCavanaugh) This is caused by a mistake of the order of overloads. I made #36570 and looks like it makes no regression.
From a real-world code suite broken by #33645:
// Should not error function add<A>(arr: Array<A>, el: A): Array<A> { return arr.concat(el) }
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])?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#L373From a real-world code suite broken by #33645:
// Did not error; probably shouldn't in the future either class A { flattenTree(option: any, changeOnSelect: any, ancestor = []) { let flattenOptions: any = []; const path = ancestor.concat(option); flattenOptions = flattenOptions.concat(this.flattenTree(option, changeOnSelect, path)); } }
This one should be simpler to demonstrate exactly what aspect of things got broken
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
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>.declare const test1: 1 declare const test2: 1 | 2 declare const test3: 2 | 3 declare const test4: number declare const array: (0 | 1)[] array.includes(test1) // OK array.includes(test2) // Argument of type '1 | 2' is not assignable to parameter of type '0 | 1'. Type '2' is not assignable to type '0 | 1'.(2345) array.includes(test3) // Argument of type '2 | 3' is not assignable to parameter of type '0 | 1'. Type '2' is not assignable to type '0 | 1'.(2345) array.includes(test4) // Argument of type 'number' is not assignable to parameter of type '0 | 1'.(2345)
In this example, the result of
test2andtest4is undesired as the test potentially can yieldtruevalue, much like the following snippet.array.findIndex((v) => v === test1) // OK array.findIndex((v) => v === test2) // OK array.findIndex((v) => v === test3) // This condition will always return 'false' since the types '0 | 1' and '2 | 3' have no overlap.(2367) array.findIndex((v) => v === test4) // OK
Disputably, some people may even argue that even
test3should not produce an error. ¯\(ツ)/¯Reacted by Gabriel Vergnaud, Dmitry Zhavoronkov and Karlis MelderisQwerty (Vítězslav Ackermann Ferko) (@ackvf) See discussion in #14520.
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 totrue
Compiler version: 5.2.2
Example:const aTuple = [ "a", "b", "c"] as const const bTuple = [ "x", "y", "z"] as const // These are both correctly inferred as exactly 3 const aLength = aTuple.length const bLength = bTuple.length type range = 0 | 1 | 2 const possibleIndex = 2 as range // This could be something like getRandomNumber(0,2) // This is inferred as x | y | z and doesn't include undefined, as expected const possibleAccess = bTuple[possibleIndex] // This causes an error, because the return array could contain undefined const test: string[] = aTuple.map((_, i) => { // With noUncheckedIndexedAccess=true access is inferred as possibily undefined, // but TS could/should know that the the index will always be 0 | 1 | 2, as above const access = bTuple[i] return access })
Here is the playground
Reacted by Luke Deen Taylor and Ethan ResnickFilterdoes not work correctly with an array of unions:Expected
const ab: number[] | string[] = ([] as string[] | number[]).filter( (value) => false, );
The error is
TS2322: Type (string | number)[] is not assignable to type string[] | number[] Type (string | number)[] is not assignable to type string[] Type string | number is not assignable to type string Type number is not assignable to type stringActual
const ab: (string | number)[] = ([] as string[] | number[]).filter( (value) => false, );
Typescript Version: 5.2.2
Related: #38380.
I would appreciate if Array.includes would behave like this:
interface Array<T> { includes(searchElement: unknown, fromIndex?: number): searchElement is T; } interface ReadonlyArray<T> { includes(searchElement: unknown, fromIndex?: number): searchElement is T; }
Reacted by Christian Svensson, Toni Villena, Ghabriel Nunes, nakagawa-james-ppcd and Faisal Hakimit 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:const abcs: String[] = "abcs".split('') abcs.includes("d") // > false, even though "d" matches type String
Reacted by RetsamKarlis 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:- You're using the string to check something about the array. e.g.
if(colorArray.includes("red")) - You're using the array to check something about the string. e.g.
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):function includes<const S>(haystack: readonly S[], needle: unknown): needle is S { const _haystack: readonly unknown[] = haystack; return _haystack.includes(needle) }
Reacted by Faisal Hakim- You're using the string to check something about the array. e.g.
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.type Characters = 'Abuela' | 'Julieta' | 'Agustín' | 'Mirabel' | 'Isabella' | 'Louisa'; const sisters = [ 'Louisa', 'Isabella', 'Mirabel', ] satisfies Characters[]; const listCharacters = (characters: Characters[]) => characters.forEach((character) => console.log(character)); // satisfies FTW! listCharacters(sisters); // character cannot be passed into includes because the param is typed as 'Lousia' | 'Isabella' | 'Mirabel' const isSister = (character: Characters):character is (typeof sisters)[number] => sisters.includes(character);
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.(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:
interface Array<T> { includes(searchElement: T, fromIndex?: number): boolean }
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.Are you saying that the sisters array should be typed as Sister[] (which is the current type), but somehow remember that it was defined as satisfies Characters[] and change the signature of includes?
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:- It ensures that your values conform to
SuperTypeas you type them. - The subtype array or one of its values can be passed to functions that want
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.- It ensures that your values conform to
@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.This seems to imply that the satisfies predicate holds, since the same is not true for the as const version of the same array (see this playground).
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:- Inferring the strings as literal values
- Inferring the array as a readonly tuple
On the other hand,
satisfies Characters[]only does #1 - you can get the same result withas constif you apply it directly to the literals:// The resulting type of `constSisters2` is identical to `satisfactorySisters` const constSisters2 = [ 'Louisa' as const, 'Isabella' as const, 'Mirabel' as const, ];
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.Reacted by dsongdaniArray.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:declare const subject: [string, null, number]; function test_shift_returns_first_type_in_the_array(): void { const result = subject.shift() ?? null; const expected: string | null = result; // ^ Type 'string | number | null' is not assignable to type 'string | null'. // Type 'number' is not assignable to type 'string'. const actual: string | number | null = result; } function test_pop_returns_last_type_in_the_array(): void { const result = subject.pop() ?? null; const expected: number | null = result; // ^ Type 'string | number | null' is not assignable to type 'number | null'. // Type 'string' is not assignable to type 'number'. const actual: string | number | null = result; }
test_pop_returns_last_type_in_the_array() test_pop_returns_last_type_in_the_array() test_pop_returns_last_type_in_the_array() // > results in a string
We've gotten numerous issue reports and PRs to change the methods of
Array, particularlyreduce,map, andfilter. 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:
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.
ｓｅｌｆ－ｃｏｎｔａｉｎｅｄ