Clarify Immer behavior in createReducer docs

[markerikson]

The createReducer docs page calls out the error of using Immer to mutate the draft while also returning a new value.

One thing we should specifically highlight is that this can happen with arrow functions that use implicit returns, like (state, action) => state.field = action.value).

We should reference a couple sections of the Immer README docs on this topic, like https://github.com/mweststrate/immer#inline-shortcuts-using-void , and possibly also link to immerjs/immer#246 (comment) .

I'd also like to add a couple sub-headers for that section, like:

Simplifying Immutable Updates using Immer

Immer API Behavior Notes

Or something along those lines

[AhmedAbdulrahman]

Hi @markerikson I would like to start on this :)

[markerikson]

@AhmedAbdulrahman cool! Go ahead and fork the repo, make a branch, and make the changes. Submit a PR when you've got something ready and I'll look at it.

[AhmedAbdulrahman]

So far I did add:

• Right sub headers
• Add your note regarding returning new value

I hope I didn't misunderstood you :) @markerikson

[markerikson]

Ah... not quite what I had in mind.

I'm specifically looking to split up the preceding chunk of text and examples into two parts.

Basically, replace the heading "Direct State Mutation" with "Simplifying Immutable Updates using Immer". Then add the heading "Immer API Behavior Notes" right before the paragraph that starts "If you choose...", and add a couple more examples to that section showing what can happen when you use arrow functions, based on the description in the Immer docs.

[dcp12345678]

Looks like this change has made its way to the latest docs:
"If you do use this, it may help to add some comments to your code that explain your reducers are using Redux Starter Kit and Immer"

Just curious, did you consider just calling the createReducer function something like createReducerWithMutability or something like that which would have made your intent clearer? Seems a bit cumbersome to have to include comments in your code (as suggested above) every time you use this library.

Great work on the starter kit though, much appreciated!

[lukejoliat]

@markerikson is this issue still up in the air? Happy to work on it, if so. Unless it has been covered in the changes @dcp12345678 is referring to.

[qn895]

Hi! To add on this, perhaps we should add in the documentation on cases you need to set new state based not only on the action, but also the current state itself.

In my case, I ran into something like this:

const slice = createSlice({
 initialState,
 reducers,
 extraReducers: {
   [otherModule.somethingChanged]: (state, action) => {
      // need to assign new state.blah based on state.blah and action.payload (kinda like a merge)
      // but state and console.log(state) is an immer proxy
   }
 }
}

So I need to know what this current state.blah is before I can set it. I tried multiple ways until I found immer's exposed original(state.blah) function, which worked for my case.

[markerikson]

@qn895 : yes, state is the original state wrapped in an Immer proxy, but that means you can just read it directly, like:

someReducer(state, action) {
    const {counter} = state;
    const newCounter = counter + 1;
    state.counter = newCounter;
}

You shouldn't need to unwrap the proxy at all.

[krewllobster]

@markerikson This is a related question to the last few comments:

The docs say (or maybe it was your comments in github) that in development, all state objects are frozen. I'm using react-map-gl which performs some internal style diffing and is throwing an error when I try to use the redux state to hold the style which is just a POJO.

It looks like immer exposes a "setAutoFreeze" function to which you can pass false, but I'm unsure as to how to turn this off in development using RSK.

Thanks!!

[markerikson]

I believe that's some kind of standalone function exposed by Immer. You'd likely import that yourself in your index.js file and call it directly.

However, I wouldn't expect "diffing" to involve mutating the object, and if there's mutations going on, then the mutation check middleware is going to throw errors.

[krewllobster]

React-mapbox-gl does have a disclaimer saying that their mapDiffing function is buggy unfortunately -- it also doesn't fix the issue. Perhaps there is a way to expose whether or not freezing will be used as an option on the createReducer or createSlice functions rather than having it be environment dependent?

[markerikson]

I see that Immer's setAutoFreeze function is effectively global. However, that's because they create a global instance of an Immer() class and export its methods as the default functions like produce and setAutoFreeze.

Hypothetically, I could imagine a way to configure and supply a unique Immer instance or something to createSlice, but I would really rather not add additional options like that.

[krewllobster]

Thats a bummer. Right now if I want to store my map style using RSK I actually have to create a new instance on each render cycle which gets rather repetetive. I'll totally grant that this seems like a bug on the mapbox side, not yours, but you're much more likely to respond than someone at uber!

Currently getting style within component function and using as such:

const component = () => { const mapStyle = useSelector(state => state.map.style) return ( <MapboxGl mapStyle={{...mapStyle}} {...otherProps} /> ) }

[markerikson]

Yeah, I know, but unfortunately this is really bad behavior on their part :(

[krewllobster]

I just started a new issue with react-map-gl, so hopefully they'll get back to me. I mentioned using RSK in the issue, so if you have any input feel free.

Thank you for being responsive!!

[krewllobster]

@markerikson they have now patched this in react-mapbox-gl v5.1! Thanks for your input over there.