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

Fix TypeScript declarations to support currying and immutable state #98

Closed
wants to merge 4 commits into from

Conversation

olavim
Copy link

@olavim olavim commented Feb 15, 2018

The TypeScript declarations prevent modying the produced draft if it was produced from an object that has a readonly interface:

interface IState {
  readonly foo: boolean;
}

const state: IState = {
  foo: false
};

produce(state, draft => {
  draft.foo = true; // Error: Cannot assign to 'foo' because it is a constant or a read-only property.
});

Also calling produce with only one argument is not allowed, so currying is impossible:

const mapper = produce((draft, index) => { // Error: Invalid number of arguments, expected 2.
  ...
});

This PR solves both of these cases.


Fixes #97

@coveralls
Copy link

coveralls commented Feb 15, 2018

Coverage Status

Coverage decreased (-2.4%) to 96.111% when pulling a01a0ff on tilastokeskus:master into 2b6cf4f on mweststrate:master.

@mweststrate
Copy link
Collaborator

thanks for the PR @tilastokeskus! Not sure if this PR really solves the issue. If I understand the typings correctly, I think this will just make the top level type mutable, but not things that are marked as readonly in any subtypes?

@olavim
Copy link
Author

olavim commented Feb 21, 2018

@mweststrate You're right, it didn't work properly after all. And apparently there are some other problems too with what I did: microsoft/TypeScript#13723

From what I gather, recursively removing the readonly-ness from a type properly is not possible at the moment. I pushed a hacky change that makes the state mutable, and at least on my IDE (WebStorm 2018) still gives me correct suggestions when accessing the draft's data. See if you can find glaring issues with it.

@ghost
Copy link

ghost commented Feb 22, 2018

I have a simpler fix than this one that also fixes some bugs in the typings. I'm about to create a pull request.

Edit: see #109

@mweststrate
Copy link
Collaborator

@tilastokeskus thanks for the effort! I think #109 is a cleaner solution though, so closing this one in favor of that one. N.b. note that you can always use produce(state, (draft: any) => {}), so there is no need to actually make that part of the built-in typings.

@mweststrate mweststrate closed this Mar 6, 2018
Sign up for free to 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.

None yet

3 participants