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

State is not treated as immutable #4

Open
pennersr opened this issue Jul 27, 2016 · 4 comments
Open

State is not treated as immutable #4

pennersr opened this issue Jul 27, 2016 · 4 comments

Comments

@pennersr
Copy link
Contributor

The example setter and the current implementation mutate state in the versionSetter code:

(state, version) => state.app.version = version

and:

state[versionString] = state[versionString] || {};
state[versionString].version = version;
@jgkim
Copy link

jgkim commented Aug 17, 2016

Because redux and redux-persist themselves are not immutability aware, the default seems to be OK to me. If you use the immutable state, then you can use a custom version setter. I guess redux-persist-migrate do the job before transforms of redux-persist so that you can use a setter that handles the transformed and saved state.

@rt2zz
Copy link
Contributor

rt2zz commented Aug 17, 2016

ya that was my thinking. Immutability would be nice, but in this case it would complicate things. I am open to ideas however for how it can be simply and performantly implemented.

@ndbroadbent
Copy link
Contributor

It might be nice to support Immutabile.JS and seamless-immutable out of the box. I don't think performance really matters since it's just run one time when the app is started. Checking for these libraries would only add a couple of lines of code.

@ndbroadbent
Copy link
Contributor

Actually I think this should be implemented as a transform. That would make a lot more sense. Then you could just run it before the immutability transform.

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

No branches or pull requests

4 participants