-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Add BooleanConstructor as an overload to .filter to allow for easy type predicate filtering #50387
Comments
Related: #16655 “Boolean() cannot be used to perform a null check” |
A bit convoluted to understand for most people, isn't it? |
see #50377 |
@ghoullier Aha, didn't see that @orta had already tried it. LMK how I can help. |
@lf-novelt Could you clarify what you mean? Which bits are convoluted? The point of intermediate-advanced TS is to try to make the beginner's TS life easier while staying out of the way. I'd argue that this overload does that very well. |
Indeed this syntax is a cool trick but not obvious to understand at first glance. I don't mind TS supporting it but would not push for this example as a "go to" to filter nulls or undefined from an array. And But I still think |
Duplicate of #16655. |
@MartinJohns I don't think it is, it's a separate suggestion which is of much smaller impact. |
In fact, you could consider this issue just a moving of this great suggestion into its own issue. |
True, it's only a partial duplicate, but your use case is the main one mentioned in #16655 and most of its duplicates. But I said my bit and I'll drop out of this conversation. Considering the unnatural amount of upvotes this issue already has it's clear that it has been promoted somewhere else. But just FYI, this suggestion is already being tried out: #50377 |
This fails and it shouldn't. const a = [] as InputsDataType[] | string[] | AppButtonType[] | CustomButtonT[];
const testFilter = (data: InputsDataType): data is InputsDataType => {
return true;
}
a.filter(testFilter); |
After #57465 there's less need for this. You can write That being said, there was a lot of confusion on Twitter about whether Approach 1: add an overload for filter(BooleanConstructor) aka the ts-reset approachHere's one version of this that specializes on diff --git a/src/lib/es5.d.ts b/src/lib/es5.d.ts
index e404df509a..3bbf59bbba 100644
--- a/src/lib/es5.d.ts
+++ b/src/lib/es5.d.ts
@@ -1252,6 +1252,12 @@ interface ReadonlyArray<T> {
* @param thisArg An object to which the this keyword can refer in the callbackfn function. If thisArg is omitted, undefined is used as the this value.
*/
map<U>(callbackfn: (value: T, index: number, array: readonly T[]) => U, thisArg?: any): U[];
+ filter(this: ReadonlyArray<object | null | undefined>, predicate: BooleanConstructor, thisArg?: any): (T & {})[];
/**
* Returns the elements of an array that meet the condition specified in a callback function.
* @param predicate A function that accepts up to three arguments. The filter method calls the predicate function one time for each element in the array.
@@ -1443,6 +1449,12 @@ interface Array<T> {
* @param thisArg An object to which the this keyword can refer in the callbackfn function. If thisArg is omitted, undefined is used as the this value.
*/
map<U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any): U[];
+ filter(this: Array<object | null | undefined>, predicate: BooleanConstructor, thisArg?: any): (T & {})[];
/**
* Returns the elements of an array that meet the condition specified in a callback function.
* @param predicate A function that accepts up to three arguments. The filter method calls the predicate function one time for each element in the array. Here's another that uses a conditional type to similar ends: diff --git a/src/lib/es5.d.ts b/src/lib/es5.d.ts
index e404df509a..eaabc04b1b 100644
--- a/src/lib/es5.d.ts
+++ b/src/lib/es5.d.ts
@@ -1258,6 +1258,12 @@ interface ReadonlyArray<T> {
* @param thisArg An object to which the this keyword can refer in the predicate function. If thisArg is omitted, undefined is used as the this value.
*/
filter<S extends T>(predicate: (value: T, index: number, array: readonly T[]) => value is S, thisArg?: any): S[];
+ filter(predicate: BooleanConstructor, thisArg?: any): [T] extends ([object|null|undefined]) ? (T & {})[] : T[];
/**
* Returns the elements of an array that meet the condition specified in a callback function.
* @param predicate A function that accepts up to three arguments. The filter method calls the predicate function one time for each element in the array.
@@ -1449,6 +1455,12 @@ interface Array<T> {
* @param thisArg An object to which the this keyword can refer in the predicate function. If thisArg is omitted, undefined is used as the this value.
*/
filter<S extends T>(predicate: (value: T, index: number, array: T[]) => value is S, thisArg?: any): S[];
+ filter(predicate: BooleanConstructor, thisArg?: any): [T] extends ([object|null|undefined]) ? (T & {})[] : T[];
/**
* Returns the elements of an array that meet the condition specified in a callback function.
* @param predicate A function that accepts up to three arguments. The filter method calls the predicate function one time for each element in the array. The problem with both of these is that they fundamentally clash with the fallback logic that was introduced for ([] as Fizz[] | Buzz[]).filter(item => item.id < 5);
// ~~~~~~~~~~~~~~~~~~~
// Argument of type '(item: any) => boolean' is not assignable to parameter of type 'BooleanConstructor'. I don't see how you'd fix this issue. Since ts-reset uses a similar approach, it also breaks this sort of call: mattpocock/ts-reset#168 Approach 2: Make BooleanConstructor a type predicateWe can change the call signature of diff --git a/src/lib/es5.d.ts b/src/lib/es5.d.ts
index e404df509a..f8e8df6522 100644
--- a/src/lib/es5.d.ts
+++ b/src/lib/es5.d.ts
@@ -532,7 +532,8 @@ interface Boolean {
interface BooleanConstructor {
new (value?: any): Boolean;
- <T>(value?: T): boolean;
+ <T>(value?: T): value is ([T] extends [object | null | undefined] ? T & {} : T);
readonly prototype: Boolean;
} This is conceptually simpler but it also runs into a few problems. First, TS actually made this change back in 2019 with #29955. But it was quickly reverted in #31515 because it had an undesirable effect on Second, it doesn't work! Even with this change, TS doesn't select the right overload of declare let dates: (Date|null)[];
interface BCWithNew {
// Comment out the next line to get "nonNullDates: Date[]" below
new (value?: any): Boolean;
<T>(value?: T): value is T & {};
readonly prototype: Boolean;
}
declare let bcN: BCWithNew;
const nonNullDates = dates.filter(bcN);
// ^? const nonNullDates: (Date | null)[]
type T = BCWithNew extends (value: Date | null, index: number, array: (Date|null)[]) => value is Date ? true : false;
// ^? type T = true I think this is a TS bug. @Andarist dug into why this happens, see this thread. My understanding is that it is a correctness / expedience tradeoff in finding the right overload. The issue around So in conclusion, it's harder to improve |
Despite |
Another use case with boolean, something like; // Cast to any to simulate incoming API value
const array: string[] | undefined | null = ['aa'] as any;
const isValid = Boolean(array && array.length > 0);
// Error on `array.map` for possible null or undefined, while we already verified the existence & length before
if (isValid) array.map(e => console.log(isValid)); It ignores the fact the existence & length was already checked before |
lib Update Request
Configuration Check
My compilation target is
ES2015
and my lib isthe default
.Missing / Incorrect Definition
I would love to add an overload to Array.filter which takes in a common use case of passing a
Boolean
directly to.filter
. This would offer a simple, intuitive workaround for a common pain point with.filter
.This could be achieved via an overload adding
BooleanConstructor
, and a type predicate.https://www.typescriptlang.org/play?#code/PTAEHUEsBcAtTgU1AewG6IE4BsUEMATAWACgBjFAOwGdpQ9NNQBeUAbQCIBGDgGlACulAogBmkSogIBdANykQoJaAB6AflKkJ0LKLxlkAQUZ4AngB4AKgD5QAb1JLx2HZgAUAB0xTIZPDoAuUAAhFBRsRDxKAGEqWkwBMmgUTABKIIA5KgAxPGxqCxs2OVIAX00SaFMPZCzKXPzC21ZLUEQADx1halAABlAAH1AAcmHB0EoBbGxxoRFxSQJQNQnEDCYgy3kSUgoaOmdXKWMmVgZMADpDrDdQ8MjKVO3FZXUgA
Sample Code
filteredArr
should bestring[]
, not(string | undefined)[]
.The text was updated successfully, but these errors were encountered: