Skip to content
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

Confusing description of Pick mapped type #25976

Closed
LAITONEN opened this issue Jul 26, 2018 · 4 comments · Fixed by #27515
Closed

Confusing description of Pick mapped type #25976

LAITONEN opened this issue Jul 26, 2018 · 4 comments · Fixed by #27515
Assignees
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Good First Issue Well scoped, documented and has the green light Help Wanted You can do this

Comments

@LAITONEN
Copy link

Search Terms:
confusing, pick, description

Problem:

Typescript has so much great functionality, but it gets hard to comprehend them for a not very experienced developer if the descriptions are confusing. Example is the description of Pick mapped property.

Actual behavior:
The description of type Pick<T, K extends keyof T> = { [P in K]: T[P]; }says:

From T pick a set of properties K.

However, the way I have been assuming the description at first, before I discovered the truth, was: we need to supply a defined type / interface as a first parameter and another defined type / interface as a second parameter, i.e.:

type ParamOne = {
  one: string;
  two: string;
  three: number;
}

type ParamTwo = {
  two: string;
  three: number;
}

type Picked = Pick<ParamOne, ParamTwo> // error

This does not work and would lead to the error, because the second parameter should contain the keys of ParamOne delineated with |, even though the description says a set of !properties K, not a set of !keys K. I understand that the word propertiesprobably refers to T, but hear me out.

So, the correct way to get type { two: string, three: number }would be this way:

type Picked = Pick<ParamOne, 'two' | 'three'>

Expected behavior:
Therefore, instead of having:

From T pick a set of properties K.

as a description, would not it be more clearer, especially for the not very experienced developers, if the description would change to:

From T pick a set of properties whose keys are K.

?

I understand that there is also the definition of the type available for us which looks this way:

type Pick<T, K extends keyof T> = { [P in K]: T[P]; }

But for me as a person who has not been working with typescript for a lot of time, this looked like very complicated and unfamiliar syntax when perceived altogether in a single line, even though some parts, like the one to the right, where the object property is being assigned, were more or less readable.

I hope that future generations of typescript users won't be struggling with mapped types the way I have been and this correction might improve the situation. Thank you!

@mhegazy mhegazy added the Suggestion An idea for TypeScript label Jul 26, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Jul 26, 2018

@DanielRosenwasser thoughts on the comment change?

@bterlson
Copy link
Member

Another option: From T, pick a set of properties whose keys are in the union K (since K will usually be a union of string literal types?)

@LAITONEN
Copy link
Author

@bterlson I like it even more! With an addition of a more professional term it can also teach a novice to use this word :)

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Help Wanted You can do this Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Good First Issue Well scoped, documented and has the green light and removed Suggestion An idea for TypeScript labels Aug 14, 2018
@DanielRosenwasser DanielRosenwasser self-assigned this Aug 14, 2018
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Aug 24, 2018
basarat added a commit to basarat/TypeScript that referenced this issue Oct 3, 2018
@matt2323
Copy link

matt2323 commented Oct 3, 2018

Looks like I was a bit too slow :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants