Make anyArray.filter(Boolean) return any[], not unknown[] #31515
Conversation
Allows anys.filter(Boolean) to once again return any[], not unknown[].
|
I'm also going to keep looking at the original failure to see whether there's a bug in assignability that causes. |
|
@typescript-bot run dt |
|
@typescript-bot run rwc |
|
@typescript-bot test this |
| declare var Bullean: BulleanConstructor; | ||
| declare let anys: Ari<any>; | ||
| var xs: Ari<any>; | ||
| var xs = anys.filter(Bullean) |
There was a problem hiding this comment.
this should fail since I didn't fix the small repro example.
| ~~ | ||
| !!! error TS2403: Subsequent variable declarations must have the same type. Variable 'xs' must be of type 'Ari<any>', but here has type 'Ari<unknown>'. | ||
|
|
||
| declare let realanys: any[]; |
There was a problem hiding this comment.
this should pass, and does, after the workaround.
|
RWC, DT and user tests are all clean. |
|
Hold the phone. I changed ReadonlyArray.filter by mistake, not Array.filter. Both need to be changed. I'll re-run tests and re-report how much breaks. |
I want to test how well this works.
|
Well, that didn't work. I'm changing the Boolean factory function for now. The user tests show no changes except fixes for the errors introduced by the original PR. |
|
@typescript-bot test this |
|
@typescript-bot run dt |
|
@typescript-bot user test this |
src/lib/es5.d.ts
Outdated
| @@ -513,7 +513,7 @@ interface Boolean { | |||
|
|
|||
| interface BooleanConstructor { | |||
| new(value?: any): Boolean; | |||
| <T>(value?: T): value is Exclude<T, false | null | undefined | '' | 0>; | |||
| <T extends any>(value?: T): value is Exclude<T, false | null | undefined | '' | 0>; | |||
There was a problem hiding this comment.
#29571 will 100% break this (and we only held off merging it because we got spooked with the number of breaks already in 3.5 IIRC), since this is a terrible hack that makes T "look like any" even though it's a type parameter and extends any should be identical to extends unknown or simply no constraint.
|
I'm proposing we just revert #29955. The cure here seems worse than the disease we were trying to address |
sandersn
changed the title
|
@RyanCavanaugh I agree. I didn't even notice that the Boolean factory was not a type guard until 3 weeks ago. I switched it back to boolean and wrote up our options in the description. |
|
@typescript-bot run dt User tests look good on my local machine; the |
| @@ -513,7 +513,7 @@ interface Boolean { | |||
|
|
|||
| interface BooleanConstructor { | |||
| new(value?: any): Boolean; | |||
| <T extends any>(value?: T): value is Exclude<T, false | null | undefined | '' | 0>; | |||
| <T>(value?: T): boolean; | |||
There was a problem hiding this comment.
but what about
(someArray as SomeType[]).map(some => {
if (!some.test) return; // after this line the result will be (SomeType || undefined)[]
return some;
}).filter(Boolean); // here undefined was filtered with Exclude in definitionsit breaks #29955 :{
Allows anys.filter(Boolean) to once again return any[], not unknown[].
Fixes #31189
Doesn't break any of our test suite, but I'm running it on user tests and I need to request a DT and RWC run.
Edit: I've found three solutions:
TtoT extends any. This will break if Reinterpret a type parameter constrained to any as an upper bound constraint聽#29571 goes in [1].string | undefinedunion by callingBoolean(but you can define your own type guard of course).filtertoI like (2) the best since you can always write your own type guard and I have never seen
if (Boolean(x))in JS or TS before, and we haven't shipped it so nobody relies on it yet. (3) is a bad example in a 馃 馃懡 馃 "why not overloads + conditional types" way.I'll switch to (2) and make sure the test results look good.
[1] I'm not sure that #29571 is a good change, but it is a safer change. My intent with
T extends anyis basically "disable type checking for this type parameter", but I think it's also common to use it to meanT extends unknown.