fix(pluck): pluck now can return object #71
Conversation
|
Thank you so much for this update. Looking at #61, I'm pretty sure I merged that by mistake. That MR was not ready. |
types/pluck.d.ts
Outdated
| export function pluck<K extends keyof U, U>(prop: K, list: readonly U[]): Array<U[K]>; | ||
| export function pluck<U extends Record<any, any>, IK extends keyof any>(__: Placeholder, record: Record<IK, U>): <K extends keyof U>(prop: K) => { [KK in keyof typeof record]: U[K] }; | ||
| export function pluck<U>(__: Placeholder, list: readonly U[]): <K extends keyof U>(prop: K) => Array<U[K]>; |
There was a problem hiding this comment.
Typescript always selects the last defined overload when you pass an overloaded function like a an argument to another function. That means if you called flip(pluck), flip would get the function pluck<U>(__: Placeholder, list: readonly U[]): <K extends keyof U>(prop: K) => Array<U[K]> type definition and return <U>(list: readonly U[], __: Placeholder) => <K extends keyof U>(prop: K) => Array<U[K]>, and that's not what we want.
The order was already correct, please re-order your changes to match the original order
There was a problem hiding this comment.
I don't know what I was thinking about when I was rearranging them. Thank you for being vigilant. I fixed that one and made the last overload as wide as possible.
types/pluck.d.ts
Outdated
| @@ -1,10 +1,10 @@ | |||
| import { Placeholder } from 'ramda'; | |||
|
|
|||
| export function pluck<K extends PropertyKey>(prop: K extends Placeholder ? never : K): { | |||
| <U extends readonly unknown[] | Record<K, any>>(obj: Record<PropertyKey, U>): U extends readonly (infer T)[] ? T[] : U extends Record<K, infer T> ? T[] : never; | |||
| <U extends Record<K, any>, IK extends string>(obj: Record<IK, U>): { [KK in keyof typeof obj]: U[K] }; | |||
| <U extends readonly unknown[] | Record<K, any>>(list: U[]): U extends readonly (infer T)[] ? T[] : U extends Record<K, infer T> ? T[] : never; | |||
There was a problem hiding this comment.
I believe you will also need to apply U extends readonly (infer T)[] ? T[] : U extends Record<K, infer T> ? T[] that is here to your updated versions for when obj: Record<>. This would cover the case of { a: string[]; b: string[]; } or in other works, Record<string, string[]>
There was a problem hiding this comment.
You are right. Done that
|
@Nemo108 I'm working on getting the next release out end of this week. Please fix remaining comments |
|
Hi! As far as I can see, I fixed every comment you have left to me. This PR is awaiting for your review |
|
@Nemo108 sorry I had left my comments in "Pending" state. I made them after your last update. You should be able to see them now |
* restore the order of function overloads; * make the last defined overload cover wider cases; * add more tests to cover most of the cases.
Nemo108
force-pushed
the
@nemo108/pluck-record
branch
from
235ac61 to
43f4027
Compare
|
@Harris-Miller I'm sorry for the wait. I think I've done everithing you've asked me to do, can you check it one more time? |
The current version of pluck typing has a big mistake: it always returns an array, when it should return an object if the object is passed as a parameter.
This commit can fix this.