Skip to content

Conversation

@knpwrs
Copy link
Contributor

@knpwrsknpwrs commented Jun 9, 2018

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
Copy link

coveralls commented Jun 9, 2018

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
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
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
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

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

@knpwrs
Copy link
ContributorAuthor

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

@mweststrate
Copy link
Collaborator

mweststrate commented Jun 23, 2018 via email

@ghost
Copy link

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

@mweststrate
Copy link
Collaborator

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

@mweststratemweststrate merged commit edc8b7c into immerjs:masterJul 21, 2018
@knpwrsknpwrs 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