Immer + withSlices discussion

[Ebubekir-Tas]

Background

When using immer with zustand-slice's withSlices middleware an error arises:

Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.

I cloned the zustand-slices library and changed some of the package's code locally to make Immer "work" using withSlices in a small repo. I put "work" in quotations because it's more of a workaround than an absolute fix.

Issue Explanation

The set function in with-slices.ts shows that nextState is always returned, which explains the issue because when passing Immer middleware the state is converted into a draft state for you to operate with mutable syntax on immutable data structures. However, Immer requires that you either modify the draft directly or return a new state, not both.

Proposal

To fix this, I suggest adding a withImmer (or immerEnabled) boolean flag to the slice configuration, which withSlices can use to decide how to handle state updates. Like so:

export type SliceConfig<
  Name extends string,
  Value,
  Actions extends SliceActions<Value>,
> = {
  name: Name;
  value: Value;
  actions: Actions;
  withImmer?: boolean;
};

Usage:

const userSlice = createSlice<'user', UserState, UserActions>({
  name: 'user',
  withImmer  : true,
  value: {
    name: 'John Doe',
  }, 
  actions: {
    setName: (newName) => (prev) => {
      prev.name = newName;
    }
  },
});

const useUserStore = create()(immer(withSlices(userSlice)))

This proposal ensures that withSlices can handle state updates correctly, whether or not Immer is used. One thing thing not mentioned is potential error handling, for example if withImmer property is used but immer middleware is not passed. This can be a potential drawback that may need to be addressed.

proof of concept:

Conclusion

This proposal is a hypothetical solution, and its feasibility would need further evaluation. I investigated this issue because the conflict between Zustand's Immer middleware and the zustand-slices library was a dealbreaker for me. I dislike using Immer's produce directly because the middleware's advantage is that it allows you to reap the benefits of Immer without having to explicitly manage it, similar to Redux Toolkit.

Immer's produce needs to be called in every single action, which is very tedious and gives me PTSD, lol. If I could just add a withImmer flag or some equivalent to apply Immer middleware to every single action, that would be awesome.

[dai-shi]

Thanks for opening up the discussion.

The set function in with-slices.ts shows that nextState is always returned

So, is this TypeScript only issue? Does it work just fine with JS without types?

I suggest adding a withImmer (or immerEnabled) boolean flag

That's not acceptable for some preferences (of mine 😄 ):

• It should be used without immer and it shouldn't increase even a byte in the production code.
• immer middleware has a "mutator" type, so technically we could support create()(...) syntax, but avoiding the syntax is one of my motivation of developing this library.
• Basically, I'd prefer and recommend bare produce which is explicit and type safe.

That said, what would be acceptable is a type-only solution or even a separate util.

I dislike using Immer's produce directly because the middleware's advantage is that it allows you to reap the benefits of Immer without having to explicitly manage it, similar to Redux Toolkit.

That's a fair point and I understand such an opinion.

I think we should export a new util createSliceWithImmer or something like that. There are two design choices: a) use immer from zustand/middleware/immer or b) add immer as an optional peer dependency in this lib.

My personal preference is b), because it allows to mix a slice with immer and one without it. I'm not sure what people feel such an experience though. (In general, my personal preference is to avoid Immer, so my preference on this point can be ignored.)

[Ebubekir-Tas]

That's not acceptable for some preferences (of mine 😄 ):

I figured that was the case, but sometimes like in chess you have to make a move even if it's not the best one to move forward

Just to clarify the immer error of both returning a new value and modifying its draft is not a TypeScript issue, it's a common error that exists in Redux Toolkit as well. For example: https://stackoverflow.com/questions/60806105/error-an-immer-producer-returned-a-new-value-and-modified-its-draft-either-r

this happens when you have something like this example that both returns a new draft and modifies it:

// bad immerAction: (state) => { return draftState.count = 10 }

as opposed to only modifying the draft:

// good immerAction: (draftState) => { draftState.count = 10 }

which is common in libraries using Immer under the hood as shown in the RTK thread

In with-slices we can see:

    for (const [actionName, actionsBySlice] of sliceMapsByAction) {
      state[actionName] = (...args: unknown[]) => {
        set(((prevState: Record<string, unknown>) => {
          const nextState: Record<string, unknown> = {};
          for (const [sliceName, actionFn] of actionsBySlice) {
            const prevSlice = prevState[sliceName];
            const nextSlice = actionFn(...args)(prevSlice);
            nextState[sliceName] = nextSlice;
          }
          return nextState;
        }) as never);
      };
    }

nextState is always returned, in my modification with a withImmer flag I thought to handle cases of modifying the draft state like so:

  for (const [actionName, actionsBySlice] of sliceMapsByAction) {
        state[actionName] = (...args: unknown[]) => {

          const useImmer = Array.from(actionsBySlice.keys()).some(sliceName =>
            configs.find(config => config.name === sliceName && config.withImmer === true)
          );

          // handle case of immer
          if (useImmer) {
            set((prevState) => {
              for (const [sliceName, actionFn] of actionsBySlice) {
                actionFn(...args)(prevState[sliceName]);
              }
            });
            // doesn't return draft state because it modifies it directly
            return;
          } else {
            // doesn't use immer
            set(((prevState: Record<string, unknown>) => {
              const nextState: Record<string, unknown> = {};
              for (const [sliceName, actionFn] of actionsBySlice) {
                const prevSlice = prevState[sliceName];
                const nextSlice = actionFn(...args)(prevSlice);
                nextState[sliceName] = nextSlice;
              }
              return nextState;
            }) as never);
        }

and in this case I was able to handle stores using either immer or without, although requiring a withImmer flag, as shown:

const countSlice = createSlice({
  name: 'count',
  value: 0,
  actions: {
    inc: () => (prev) => prev + 1,
    reset: () => () => 0,
  },
});

const textSlice = createSlice({
  name: 'text',
  value: 'Hello',
  actions: {
    updateText: (newText: string) => () => newText,
    reset: () => () => 'Hello',
  },
});

const userSlice = createSlice<'user', UserState, UserActions>({
  name: 'user',
  withImmer  : true,
  value: {
    name: 'John Doe',
  }, 
  actions: {
    setName: (newName) => (prev) => {
      prev.name = newName;
    }
  },
});

const useUserStore = create(immer(withSlices(userSlice)))
const useCountStore = create(withSlices(countSlice, textSlice));

// create()() or create() doesn't effect the outcome, it's compatible with either syntax

I understood this is almost certainly not the best approach but it is theoretical

I think we should export a new util createSliceWithImmer or something like that. There are two design choices: a) use immer from zustand/middleware/immer or b) add immer as an optional peer dependency in this lib.

I think this is a good idea, I want to research this when I get home later.

[dai-shi]

b) add immer as an optional peer dependency in this lib.

In that case it's probably with a different entry point:

import { createSlice } from 'zustand-slices/immer';

I'm not 100% sure yet.