RFC: Make `immer` optional for `createReducer`

[neurosnap]

I think end-developers should be able to opt-out of immer behavior for createReducer and createSlice. There have been some questions raised recently after the 1.0 announcement that people have experienced performance issues when using createSlice because of immer.

For most use-cases, I think immer should be enabled. However, there are specific cases where I have needed to tune the performance of a set of reducers and needed to disable immer.

immer definitely provides a ton of value out-of-the-box and I think should be enabled by default. However, I also think this is one area where we can be a little more flexible and allow the option to disable it. createReducer is still extremely valuable even without immer and I think that is the main reason why I think it should be opt-out.

For createReducer we could provide an additional parameter that determines if immer is used:

 export function createReducer<
   S,
   CR extends CaseReducers<S, any> = CaseReducers<S, any>
- >(initialState: S, actionsMap: CR): Reducer<S> {
+ >(initialState: S, actionsMap: CR, useImmer: boolean = true): Reducer<S> {
   return function(state = initialState, action): S {
+    const caseReducer = actionsMap[action.type];
+    if (!useImmer) {
+      return caseReducer ? caseReducer(state, action) : state;
+    }
     // @ts-ignore createNextState() produces an Immutable<Draft<S>> rather
     // than an Immutable<S>, and TypeScript cannot find out how to reconcile
     // these two types.
     return createNextState(state, (draft: Draft<S>) => {
-     const caseReducer = actionsMap[action.type];
       return caseReducer ? caseReducer(draft, action) : undefined;
     });
   };
 }

Or we could create a separate function

export function createSimpleReducer<
  S,
  CR extends CaseReducers<S, any> = CaseReducers<S, any>
>(initialState: S, actionsMap: CR, useImmer: boolean = true): Reducer<S> {
  return function(state = initialState, action): S {
    const caseReducer = actionsMap[action.type];
    return caseReducer ? caseReducer(state, action) : state;
  };
}

I'm curious of everyone's thoughts. Can anyone else think of other ways to implement this feature?

I also think that createSlice should provide a property immer which gets passed to createReducer.

 const slice = createSlice({
   name: 'slice',
+  immer: false,
   // ...
 });

I could see this kind of change requiring some typings work before merging.

I'm willing to do the work to get a PR up for this, but I wanted to make sure it was something that the maintainers agree would be useful.

[markerikson]

Already asked in #183 , and no, I'm not planning to do this.

The point of RSK is to be opinionated. There's a million createReducer implementations out there that don't use Immer, including a copy-pasteable version from the Redux core docs. I don't see a need to make this configurable.

There's related discussion at https://github.com/reduxjs/redux-starter-kit/issues/90#issuecomment-547154051 . Immer has a global setAutoFreeze() function that allows you to disable freezing if desired.

I can hypothetically imagine wanting to set that on a per-slice/reducer basis, and it would be technically possible if we accepted an option and then created a unique Immer() instance. But, again, I don't want to add that level of configuration at this point.

In general, my recommendation is to either not use RSK's functions here, or disable Immer's auto-freezing.

[ghost]

I'm really on the fence about using RSK because of this whole Immer thing. We don't use it or Immutable.js at my company and we have a ton of junior devs writing Redux code. They don't have any problem just using spread and destructure to write immutable state reducers.

The things being done in this library are very similar to what we have for our own in-house library but I wanted to switch to RSK because it's an official, well documented and maintained kit. If we use RSK and continue to write reducers that always return a whole new state object, Immer will be doing nothing for us but adding to our overhead and bundle size. Looking at Immer releases, I see their still making some major changes and fixing bugs every release so it's a little scary to me.

I guess I just wanted to add my voice to support for this option or something like it - maybe a separate non-Immer implementation of createReducer and createSlice.

[markerikson]

As I said earlier, the point of RTK is to be opinionated.

Sure, you can write nested immutable update logic by hand, the way you always have. In fact, you still can while using RTK, because you can return new state values from reducers.

But the point is you don't have to do it the hard way, and you shouldn't because it's error prone.

RTK currently uses Immer 4.x, as the 5.x releases don't add any features that we care about (specifically Map and Set support). That said, the fact that Immer is continuing to get worked on is a feature, not a bug - it means the lib is being well maintained.

If you skip the Immer aspect, RTK's createReducer is API-identical to the 6-line recipe we've shown in the docs for years:

function createReducer(initialState, handlers) {
  return function reducer(state = initialState, action) {
    if (handlers.hasOwnProperty(action.type)) {
      return handlers[action.type](state, action)
    } else {
      return state
    }
  }
}

But seriously, you should be using Immer, via createSlice. That will end up saving you a lot of hand-written code in the long run, and assuming the app continues to grow, odds are you'll have saved more lines of code than the actual size of Immer itself.

[ghost]

But the point is you don't have to do it the hard way, and you shouldn't because it's error prone.

I don't think that using spread/destructure is hard or error prone. As a matter of fact, picking out errors is easy because the pattern is very clear and when you don't follow it - it sticks out like a sore thumb in code reviews. We don't put a lot of logic in our reducers.

I honestly can't remember the last time we had an issue with reducers and we have dozens of teams using React/Redux.

createSlice...will end up saving you a lot of hand-written code in the long run...

Yep. I hear you - that's why our in-house kit already has something like createSlice without Immer and it works great. I like to keep things as simple as possible and not add the complexity of what amounts to training wheels in my opinion.

[markerikson]

I don't think that using spread/destructure is hard or error prone

Well, I'm genuinely glad your teams are doing well :) But having seen hundreds of Redux apps and helped thousands of users, I can confirm that accidental mutation is by far the most common mistake that users make, and I can also confirm just from my own experience that writing manual immutable update logic is a royal pain.

I don't ever want to have to do that again, I don't want our users to keep having to deal with that, and the size and abstraction Immer adds are totally worth it for the way it makes the rest of my code simpler.

We don't put a lot of logic in our reducers.

Note that this doesn't actually make things better or easier. If you're doing something like generating a new array outside of Redux in a component, and your reducer is just return action.payload, you still have to follow the rules of immutable updates. It's just not as obvious that you are supposed to be doing things immutably.

That's one of the reasons why the new Style Guide page specifically recommends that you should put as much logic as possible in reducers. It helps remind you that you should be following the rules of reducers like immutable updates, and if you're using RTK, you get the benefit of Immer for doing those updates.

[phryneas]

To add to this:

I don't think that using spread/destructure is hard or error prone.

immer does not only protect you from accidental modification in the reducer, but also from accidental mutation during render (and everywhere else). If you had to search once for a bug where someone accidentally modified a value during render, you know that this can take many hours to find.

And yes, you might catch those in code reviews, but that code review is bound to happen when code is submitted. Most likely someone already spent hours chasing down some bug somewhere before that review. That a bug is caught doesn't mean that it didn't waste quite a few developer hours beforehand.

Also, as Mark already said: The bundle size of immer is smaller than the amount of code that immer saves you in almost every project that is large enough to worry about bundle size.

[ghost]

Hmmm, well you guys make some good points and even though we don't generally have problems with the things you're describing, it might not be bad to protect against them anyway.

Maybe there are problems that haven't even caught yet that can be fixed.

Most likely someone already spent hours chasing down some bug somewhere before that review.

In 2 years of using Redux I think this happened to us once or twice. They would tell me. I'm not saying it's not a problem ever and it's certainly not a fun bug to deal with, but the main thing I'm afraid of is that we trade that relative safety for new problems. I'd hate to recommend a pattern switch and then have it cause more problems than it solves. So, I'm just reading through the issues here and on Immer to see what people are going through and as far as I can see, it's not all risk free. (I'm a little nervous that there's no redux-persist guidance because that is one thing we have spent hours perfecting in our current setup. I saw the issue where the one commenter said they solved it so maybe it will just work out.)

At this point I guess I'll setup a trial project using RTK and see what happens, discuss with my peers and so forth...

Thank you so much for your detailed and thoughtful replies!

[phryneas]

If you notice any bumps on the road, make sure to open a ticket. We are committed to make this as good as possible!

[markerikson]

@waynebloss : yeah, while I haven't ever used redux-persist myself, the only real point of concern here should be that apparently it tries to pass some non-serializable values in a couple actions, and it looks like that can be handled by telling the serializability check middleware to ignore those.

There's no "guidance" on using that in the docs, simply because it's a purely optional addon that isn't related to the Redux core or RTK at all. I could see us possibly adding a new core docs page on "Store Persistence" at some point, but that's about as far as I'd go.

[neurosnap]

I wouldn't add immer just to protect against unintentional mutation. There are other libraries that do that better without the performance overhead in production: https://github.com/buunguyen/redux-freeze

There is performance overhead with immer but for most applications it won't be noticeable.

I do think making immer not an option is a weird way to be opinionated, considering createSlice and the other aspects of this library are still useful without immer. The typing alone on createSlice is non-trivial to re-implement successfully. But this is the maintainers' prerogative and I support their decision.

[ghost]

@markerikson Well since we're here - what do you personally do for persisting a slice of state if you've never used redux-persist? We use it pretty heavily. Almost every app we create has some sort of frontend-only preferences/cache/etc that we want to store on the client...

(I actually checked how widely used redux-persist is just now and I was surprised to see that it's not that big - only about 360k weekly downloads compared to about 3 and 4 million for react-redux and redux. Maybe we need an update on how we do certain things.)

[markerikson]

Another reason to not bother making Immer optional atm is that we still don't have tree-shaking working for RTK, due to other dependencies: #78 .

So, if you use RTK, right now you're getting Immer whether you use createSlice or not, and would still do so even if createSlice had some option to not call Immer internally. For that matter, even if tree-shaking worked right, we've already had to do some ridiculous build shenanigans just to get redux-immutable-state-invariant to get left out of prod bundles. I don't even want to know how complicated it would get to try getting Immer to drop out conditionally.

(I'd really like to improve the tree-shaking behavior, but my priority atm is rewriting the Redux core docs, and no one has jumped in and tried to fix the build behavior.)

Folks are always welcome to fork the lib and do whatever they want with it, but given that I'm the maintainer, I'm the one setting the vision and what I want in the lib.

@waynebloss : I've never had to bother with store persistence, or routing for that matter. The apps I've worked on have been true "single-page" apps - ie, desktop-like apps that just happen to live in a browser.

[phryneas]

I wouldn't add immer just to protect against unintentional mutation. There are other libraries that do that better without the performance overhead in production: https://github.com/buunguyen/redux-freeze

This is a little bit of a rant, and even though it was triggered by you, it's more of a "we really have to have this written here" thing, so please don't take it as an attack :smile:

If anyone has performance problems, immer is probably the last place to look

First about the comparison with deep-freeze: Deep-freeze is about multitudes slower than immer to prevent unintentional modification (give the performance tests a try). But as you already noticed, redux-freeze doesn't do that in production. immer doesn't as well.

So, aside from the performance impact of preventing mutation, there's one other point where immer can have a performance impact: In the reducer. And only there. And sorry, but I don't believe that it plays a role if your reducer takes 0.1ms or 0.3ms. The stuff that comes after that takes so much more time and has so much more space for optimization.

• subscribing to the store from more components: if react rerenders too much of your application
• subscribing to less: if too many selectors are called on every store modification (or memoizing these)
• rendering less components: by using virtualized lists
• cleaning up your render functions: memoizing a single slow loop will have more impact on your application than swapping five libraries
• dispatching less actions and moving stuff out of global store: redux should not be used for forms, generally a lot of state should be moved into local state

If you have done all that and still suspect immer of "slowing down" your application, you might be one of those rare cases that has deeply-nested lists with tens of thousands of object in the store. Then removing immer isn't going to save you, because even vanilla js will be slow for you - you're gonna have to go the immutable.js route then, I'm sorry.

There is performance overhead with immer but for most applications it won't be noticeable.

Yup, just had to say the same thing with a lot more words for everyone to read :rofl:

I do think making immer not an option is a weird way to be opinionated, considering createSlice and the other aspects of this library are still useful without immer. The typing alone on createSlice is non-trivial to re-implement successfully. But this is the maintainers' prerogative and I support their decision.

It would duplicate a lot of the code and the types of RTK (which would be a chore to maintain) and it would signal that we think that using Redux without immer is a good idea - which I would not ever recommend if the option of immer were in the room. Sure, there may be some extreme edge cases (where you would need something like immutable due to very big data structures) but the people that need these know it themselves. In the end everything we put in this library is a recommendation to the ecosystem, and just writing in the docs "we put this here, but we really don't recommend that" is not an option, as too many people are just not reading the docs of anything.

[theogravity]

I would like to disable immer temporarily as I'm working with a large project that does not use redux and intermixes serializable and non-serializable elements in its data. What I'd like to do is to be able to shove the data in as-is without concern about immer, work on a strategy to separate the non-serializable items out, and re-enable immer.

I'll definitely check out setAutoFreeze() to see if it fits my use-case.

[phryneas]

@theogravity serializability is a redux concern, not an immer concern. Non-serializable objects do not work with many middlewares and the devtools, which is why we warn about them. But you can always disable those warnings manually. https://redux-toolkit.js.org/api/serializabilityMiddleware

[markerikson]

@theogravity I'm kinda confused - if you're not using Redux, why is RTK even involved here?

[theogravity]

@phryneas Thanks for the reference!

@markerikson The project was not built on redux and has its own state management scattered around and I am migrating it to use redux (RTK specifically) as we have complex data flows that are difficult to debug and to reduce future tech debt.

[trappar]

Big fan of RTK, but I just wanted to throw in another use case where the forced use of immer is causing issues.

I have a reducer where all the actions/handlers use immer, except for one. That one needs to accept incremental patches incoming from the server. I can build these patches in a way that’s compatible with immer’s applyPatches functionality, but because RTK wraps everything in produce I can’t easily apply the patches. Right now I’m using fast-json-patch from inside a reducer, which seems ridiculous, and it’s very difficult to debug due to the proxies. I’m thinking I will either have to rewrite all the affected reducers and use produce manually, or make use of immer’s new “original” function and return a new value just to break out of the box RTK has put me in.

Just because something is opinionated doesn’t mean it shouldn’t have escape hatches.

[markerikson]

@trappar I've already said, this thread and others, why we're not going to make Immer optional for createReducer and createSlice.

That use of "patches" sounds like a very rare edge case to me. RTK is built for the 80%+ use case. If you have cases where you need to do something different, you can always write a separate reducer by hand.

[trappar]

For others who find themselves in a position like this - here's a 12 line reimplementation of my usage of this library. This offers an API almost identical to RTK but also supports reducers that aren't wrapped in immer's "produce". Think of it as a jumping off point for your own implementation:

import {produce} from 'immer';

export const createReducer = ({initialState = {}, immerReducers = {}, reducers = {}}) =>
    (state = initialState, action) => {
        const type = action.type;
        if (immerReducers.hasOwnProperty(type)) {
            return produce(immerReducers[type])(state, action);
        } else if (reducers.hasOwnProperty(type)) {
            return reducers[type](state, action);
        }
        return state;
    }

[phryneas]

Just to state this once more as people generally seem to forget about it:

Using RTK doesn't mean you can't write your own reducers. RTK offers shortcuts, you don't have to use each and every one of them at all times and all costs.

A simple example for a one-off reducer combining one non-immer reducer with a RTK slice:

const reducer = (state = initialState, action) => {
  if (oneOffAction.match(action)) {
    return ...// custom logic here;
  }
  return mySlice.reducer(state, action)
}

Also, from RTK 1.4 forward we are checking with immer's isDraftable function if an object is passed into produce or not (as required by immer). That method is documented as such:

Returns true if Immer is capable of turning this object into a draft. Which is true for: arrays, objects without prototype, objects with Object as their prototype, objects that have the immerable symbol on their constructor or prototype

So if you have something non-serializable, there is a great chance that it isn't wrapped in immer anyways.

[jboler]

I'm bypassing immer in individual reducers this way. original() returns the original state object and any return value in produce() is used as the new state as long as the draft has not been modified.

import { original } from 'immer';
import { merge } from 'lodash';

const API = createSlice({
  name: 'API',
  initialState: initialState(),
  reducers: {
    fetchResources: (draft, { payload }: FetchAction) => {
      const state = original(draft) as ApiState;
      return {
        ...state,
        objects: merge(
          {},
          state.objects,
          normalize(payload.response, normalizeOpts)
        ),
      };
    },
  }
};)

[markerikson]

That's not "bypassing" Immer. produce() is still being called internally to createSlice(), as shown by the fact that you had to call original(). Immer has always allowed returning a new object for the result if you wanted to, same as with normal Redux reducers.

The complaints here have been centered around Immer's produce() being called at all.

[Izhaki]

I think the authors of RTK have consistently made very solid decisions, and I would rank using immer as one of the best of these decisions. I love RTK; I love createSlice.

A specific use case

I am here, to share a specific use case that might be of interest.

I'm working on a Webpack plugin that hooks on many compiler and compilation events. These events are dispatched as actions to the store.

For the app produced by yarn create next-app, there are about 500 events per second being fired.

0.2% immer penalty

Is this something to cause an immer performance concern?

Immer's performance shows that updating 5000 items (out of 50,000) takes roughly 19ms.

So in my case, we are looking at about 2ms per second immer "penalty" (not science, just crude approximation).

But still...

Still, all of this happens in node, with no requirement whatsoever for immutability. Hence I landed on this issue.

(The browser part of the plugin uses the same store and DOES require immutability into React.)

So whilst I understand the rationale behind not providing the option to opt out of immer, and I think a very strong case was made for that above, I just feel this specific case should be known.

[markerikson]

Heh. Using RTK in a Webpack plugin is certainly not one of the primary use cases we're targeting :) Obviously it can work, because a Redux store and reducer functions are just plain JS and can be used anywhere. But in terms of "use cases RTK is specifically designed for", it's pretty far off our radar :)

[Izhaki]

Indeed, it's really not a typical RTK/redux case.

I do want to note the following though:

Redux as a Reactive Framework

Redux (and hence RTK) are pitched and universally seen as a state management solution.

But personally I've found it to be a very powerful reactive framework. I've harnessed its reactive potential in a variety of projects and submodules (to give one example, a game-server simulator to drive integration tests), and I cannot think of a design/technology that would lend itself better to the problem at hand.

It is a sort of 'just-right' reactive framework. Compared to, say, RxJS or cycleJS, redux is more mainstream, scoped, simple - and it does what you want it to do.

RTK has nothing to do with this directly (all the reactive part is due to middlewares), but there's always an element of a store involved.

RTK to Rule

Redux was designed as a high-level abstraction. As such it offers flexibility over usability.

RTK targets the concrete, so we have usability over flexibility. RTK is well-thought-out, well-scoped, and a direct offspring of redux.

Although I'm sure I haven't been exposed to every use-case out there, I'm struggling to come up with an example where redux would be more sensible than RTK.

The Prophesy

I'm just saying this because there is some chance RTK will be another case of 'a victim of its own success' - it may get such a phenomenal traction (if it isn't already), that use-cases and requests are just going to grow.

RTK has a very well defined scope and use cases in mind - for a good reason. Just a prophesy of the difficulties that may lay ahead.

Anyhow, this is all well off topic, so let's not continue this discussion here.

[danielo515]

It's sad to see how many people doesn't understand the point on writing immutable code. Many people only do it because react and redux require it, and that's all. Libraries like immer keep people in the ignorance at the same time they support old bad habits of coding at the same time it adds bundle size, performance penalty and impossible to polifyl requirements . Immutable code is better because it is easier to test, easier to follow and easier to reason about. JS has enough primitives to write immutable code easily, I don't understand why experienced programmers keep wanting to use let and +=. I really wanted to use RTK for the boilerplate reduction but I wanted it specially because of the typings. However, not using create slice takes away half of that, and at that point I don't think the magic and the extra bundle still worth the cost.

[markerikson]

@danielo515 I really don't understand what point you're making here.

The important thing is that the data gets updated immutably. The syntax for that and how it is accomplished is not what matters.

Now, yes, I get the point of needing to know what immutability is and how to do it by hand, so that you understand what Immer is actually doing for you. That's why I've repeatedly explained what immutability is, what Immer does, why this matters, in many many places throughout our docs:

• https://redux.js.org/tutorials/essentials/part-1-overview-concepts#immutability
• https://redux.js.org/tutorials/essentials/part-2-app-structure#reducers-and-immutable-updates
• https://redux.js.org/tutorials/fundamentals/part-3-state-actions-reducers#reducers-and-immutable-updates
• https://redux.js.org/tutorials/fundamentals/part-8-modern-redux#immutable-updates-with-immer
• https://redux-toolkit.js.org/usage/immer-reducers
• https://redux-toolkit.js.org/usage/usage-guide#simplifying-reducers-with-createreducer

JS has enough primitives to write immutable code easily

Yes, and those primitives are hard to use and very error-prone.

Accidental mutations have been the number one cause of bugs in Redux apps since day 1. Immer not only lets you write shorter code, Immer effectively eliminates that source of bugs. That right there is enough reason to use Immer as a default.

If you still want to write immutable update logic by hand, you can, even in createSlice. Nothing's stopping you from writing a triply-nested spread operator to return a result. But hand-written immutable updates should not be the default way to use Redux any more.

impossible to polyfill requirements

This is incorrect. Immer has an ES5 fallback that works if there are no proxies available in the environment, and RTK already turns that on by default.

I don't think the magic and the extra bundle still worth the cost.

This is simply wrong, and all the other feedback I've gotten has proven that the current RTK design is the right way to go.

[markerikson]

At this point I don't see any reason to keep this thread open, because we're not changing the design.