Does not seem to detect direct manipulation of a value

[omnidan]

I tried adding your library to my redux-undo-boilerplate and was testing it out by adding state++ in my counter reducer. Unfortunately, this library does not seem to detect this state manipulation.

You can test it out by cloning the enforce-immutable branch and running it with npm run dev: https://github.com/omnidan/redux-undo-boilerplate/tree/enforce-immutable

[leoasis]

You're not mutating the state there, since a number is immutable in Javascript natively. What you are mutating there is the local variable reference state that holds the state.

That is perfectly valid stuff to do inside a reducer, and it won't cause any problems with redux at all.

To give an example so you can see the difference:

// This sets the local state value to something new, but it's not a state mutation
state = {...state, foo: 1}

// This is a mutation, and this _will_ be detected by the library:
state.foo = 1

The example you point out in your repo, is the equivalent of doing this, which is not a mutation:

state = state + 1
// or, to make it even clearer:
const prevState = state
state = prevState++
// here, state now points to the previous value + 1, but prevState still holds the previous value.

Hope this helps you understand what is a state mutation and what is a variable reference mutation

[omnidan]

@leoasis interesting. I did not know that. Well that explains it then, so I'll close it. Thanks! 😁

[omnidan]

@leoasis okay, I must be doing something really wrong. I tried what you told me to do, but it doesn't seem to detect that either: omnidan/redux-undo-boilerplate@4203de9

[leoasis]

Well, that seems to actually be a bug here. I'm taking a look and will push a fix. Thanks for reporting!

[omnidan]

@leoasis no problem 😁 let me know when it's fixed and I'll test it for you (or you can just use omnidan/redux-undo-boilerplate@4203de9)

[leoasis]

@omnidan Alright, pushed some changes to master, would you mind checking if that now works for you? I realized that I was checking the mutations taking into account the returning state as well as the previous, but it turns out that I only needed to check the reference and the snapshot of the previous state to see if they are different.

Anyway, it should be working, added some new tests and they all pass, but if it doesn't (or it breaks with something else) let me know!

[omnidan]

@leoasis huh, I can't seem to be able to install the master branch with npm - I tried:

npm install leoasis/redux-immutable-state-invariant
npm install leoasis/redux-immutable-state-invariant#master
npm install git://github.com/leoasis/redux-immutable-state-invariant
npm install git://github.com/leoasis/redux-immutable-state-invariant#master

None of these seem to work, they all say [email protected] node_modules/redux-immutable-state-invariant but then don't install it at all. Would you mind testing it with my boilerplate and releasing a new version? 😁

[omnidan]

Actually they do install something, but it doesn't seem to have any source code, just an example folder, README.md, LICENSE.md and package.json 😱

[leoasis]

Ah yes, you're right, since it gets it from the repo but it doesn't have the compiled files. Will test your repo locally and publish a release

[omnidan]

Ahh that makes sense. I was getting kinda confused about this 😛 Thanks a lot!

[leoasis]

Tried it on your repo and seems to work now, released! https://github.com/leoasis/redux-immutable-state-invariant/releases. Let me know if you find something else!

Thanks again for filing the issue!

[omnidan]

@leoasis hm... I just installed 1.1.1 but it still does not seem to detect the mutation for me. Is there anything you changed in my code to make it work?

[leoasis]

Yes, it is working for me. Cloned your repo, went to the enforce-immutable branch, then did: npm install and npm run dev. Then I click the + button in the site and I see the mutation error.

[omnidan]

@leoasis Sorry - it was a stupid mistake on my side. I started it in production mode, it works fine in dev mode 😛 This can be closed then!

Time to get some ☕

[leoasis]

Glad it worked, and thanks again for finding (and filing) this bug :D

[omnidan]

You're welcome 😁 Thanks for fixing it, I'll add your project to my boilerplate now.

EDIT: https://github.com/omnidan/redux-undo-boilerplate/tree/master#what-happens-if-i-mutate-the-state-directly

[leoasis]

Thanks for using it/mentioning :D