Skip to content

Add TypeScript Mapped Draft Type - #161

Merged
mweststrate merged 9 commits into
immerjs:masterfrom
knpwrs:mapped-draft-type
Jul 21, 2018
Merged

Add TypeScript Mapped Draft Type#161
mweststrate merged 9 commits into
immerjs:masterfrom
knpwrs:mapped-draft-type

Conversation

@knpwrs

@knpwrsknpwrs commented Jun 9, 2018

Copy link
Copy Markdown
Contributor

These changes enable the following:

importproducefrom'immer';interfaceState{readonlyx: number;}// `x` cannot be modified hereconststate: State={x: 0;};constnewState=produce<State>((draft)=>{// `x` can be modified heredraft.x++;});// `newState.x` cannot be modified here

This helps TypeScript users ensure immutability of their state interfaces. I had to upgrade TypeScript for the index.d.ts file to parse properly. This feature was merged into TypeScript back in February: microsoft/TypeScript#21919

It also appears that the flow/ts.ts file doesn't run and never did. I tested this externally and it works, but I'd like to see a more robust test suite in this repo.

EDIT: I've added some TypeScript tests in Jest.

EDIT:Fixes#97

@coveralls

coveralls commented Jun 9, 2018

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 93.137% when pulling 1978d62 on knpwrs:mapped-draft-type into 537aa92 on mweststrate:master.

@knpwrs

Copy link
Copy Markdown
ContributorAuthor

I just pushed a commit to make the Draft type recursive so users can have nested readonly interfaces:

interfaceState{readonlyfoo: number;readonlybaz: {readonlyx: number;readonlyy: number;};}

I'm still trying to figure out how to make arrays work properly.

@knpwrs

Copy link
Copy Markdown
ContributorAuthor

Looks like everything is working now. All of the following compiles and works as expected:

importproducefrom'immer';interfaceState{readonlynum: number;readonlyfoo?: string;bar: string;readonlybaz: {readonlyx: number;readonlyy: number;};readonlyarr: ReadonlyArray<{readonlyvalue: string}>;readonlyarr2: {readonlyvalue: string}[];}conststate: State={num: 0,bar: 'foo',baz: {x: 1,y: 2,},arr: [{value: 'asdf'}],arr2: [{value: 'asdf'}],}constnewState=produce<State>(draft=>{draft.num++;draft.foo='bar';draft.bar='foo';draft.baz.x++;draft.baz.y++;draft.arr[0].value='foo';draft.arr.push({value: 'asf'});draft.arr2[0].value='foo';draft.arr2.push({value: 'asf'});})(state);

@knpwrs

Copy link
Copy Markdown
ContributorAuthor

I only just now noticed the previous discussions in #97 and #109. I believe what I have here is a solution that fixes #97 and addresses the problems in #109.

@ghost

Copy link
Copy Markdown

This would be really useful to us, thank you very much @knpwrs

@knpwrs

Copy link
Copy Markdown
ContributorAuthor

@mweststrate is there any interest from the project owners to add these typings?

@mweststrate

mweststrate commented Jun 23, 2018 via email

Copy link
Copy Markdown
Collaborator

@ghost

Copy link
Copy Markdown

Hello @mweststrate
+1 for this PR from my side 😀 👍

@mweststrate

Copy link
Copy Markdown
Collaborator

Reviewed! No comments, looks awesome :). Will release next week

@mweststrate
mweststrate merged commit edc8b7c into immerjs:masterJul 21, 2018
@knpwrs
knpwrs deleted the mapped-draft-type branch July 21, 2018 16:32
@knpwrsknpwrs mentioned this pull request Jul 22, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@knpwrs@coveralls@mweststrate