Uh oh!
There was an error while loading. Please reload this page.
Make anyArray.filter(Boolean) return any[], not unknown[] - #31515
Make anyArray.filter(Boolean) return any[], not unknown[]#31515Nathan Shively-Sanders (sandersn) merged 5 commits into
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. |
Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized Definitely Typed test suite on this PR at ccf38f6. You can monitor the build here. It should now contribute to this PR's status checks. |
TypeScript Bot (@typescript-bot) run rwc |
TypeScript Bot (@typescript-bot) test this |
Heya Nathan Shively-Sanders (@sandersn), I've started to run the extended test suite on this PR at f80dd7e. You can monitor the build here. It should now contribute to this PR's status checks. |
| 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 (@typescript-bot) test this |
Heya Nathan Shively-Sanders (@sandersn), I've started to run the extended test suite on this PR at e118188. You can monitor the build here. It should now contribute to this PR's status checks. |
Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized Definitely Typed test suite on this PR at e118188. You can monitor the build here. It should now contribute to this PR's status checks. |
TypeScript Bot (@typescript-bot) user test this |
Heya Nathan Shively-Sanders (@sandersn), I've started to run the community code test suite on this PR at e118188. You can monitor the build here. It should now contribute to this PR's status checks. |
| 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.
Ryan Cavanaugh (RyanCavanaugh)
commented
May 22, 2019
I'm proposing we just revert #29955. The cure here seems worse than the disease we were trying to address |
Ryan Cavanaugh (@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 (@typescript-bot) run dt User tests look good on my local machine; the |
Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized Definitely Typed test suite on this PR at ab9d935. You can monitor the build here. It should now contribute to this PR's status checks. |
Heya Nathan Shively-Sanders (@sandersn), I've started to run the extended test suite on this PR at ab9d935. You can monitor the build here. It should now contribute to this PR's status checks. |
| 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
(someArrayasSomeType[]).map(some=>{if(!some.test)return;// after this line the result will be (SomeType || undefined)[] returnsome;}).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.