RFC: configurable produce implementation

[phryneas]

With immer alternatives like structura.js and mutative popping up, it might make sense to allow the configuration of the produce implementation used by RTK.

This PR would, by default, still use immer, but expose buildCreateReducer and buildCreateSlice functions to create custom createReducer and createSlice functions. This should tree-shake well, and if only those other implementations are used, immer should not be bundled anymore (as soon as the polyfill in 2.0 is disabled, which is why I base this PR against the v2 version).

This would also theoretically allow produce to be swapped out for a stub, so people who really want to use RTK without any immutability helper could do so - even though I would highly recommend against that.

It would also prepare for future situations when Records and Tuples are widely available, and an immutability helper will not be necessary anymore.

There are still a few more places where we use produce, so additional changes would be required in

• createEntityAdapter
• createApi (here we need to configure with a version allowing for patches)

So far, I just wanted to put this out as a base for discussion - the code changes required for this are pretty small, so I think it might make sense at this point.

PS: we might need to do some docs updates, as, at the moment, the docs directly reference the types of createSlice and that has now moved into its own interface definition.

PS2: at this point, mutative does not allow for returning a new object, so that one might not work yet. Let's see if we can convince the author to add that functionality.

[codesandbox]

  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0888fde:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration
rsk-github-issues-example	Configuration
@examples-query-react/basic	Configuration
@examples-query-react/advanced	Configuration
@examples-action-listener/counter	Configuration
rtk-esm-cra	Configuration

[unadlib]

hi @phryneas,

I will add support for return values to mutative. But from the user's point of view, we should suggest always returning the draft itself or no value at all, not only does it maintain optimal performance (no extra performance wasted traversing unpredictable return values), but it also maintains the full mutation feature.

[phryneas]

Hi @unadlib! That's great news!
In writing reducers with immer (which we might need to rename to "writing mutable immutable reducers" or something like that), we generally recommend changing the existing state - but there are cases where the user might want to return something completely different.

Most common cases would be:

• replacing the full state with a completely new object (e.g. resetting state or taking a big chunk of data that came in, for example from an api call)
• the user wants to optimize things by hand
• they are just migrating over old Redux code that is not doing mutable updates yet.

From our perspective it would be fine if the rule were "if the user returns a value and that value is not the draft, just return that whole new value without traversing and touching it in any way", but I'm not sure if there are additional things that you have to do and cannot skip.

[unadlib]

Hi @unadlib! That's great news! In writing reducers with immer (which we might need to rename to "writing mutable immutable reducers" or something like that), we generally recommend changing the existing state - but there are cases where the user might want to return something completely different.

Most common cases would be:

• replacing the full state with a completely new object (e.g. resetting state or taking a big chunk of data that came in, for example from an api call)
• the user wants to optimize things by hand
• they are just migrating over old Redux code that is not doing mutable updates yet.

From our perspective it would be fine if the rule were "if the user returns a value and that value is not the draft, just return that whole new value without traversing and touching it in any way", but I'm not sure if there are additional things that you have to do and cannot skip.

Cool proposal. Since the user may refer to the draft in the return of a completely new object, I will use the option strict, which we will suggest the user enable strict in development mode and disable strict in production mode. This will ensure safe returns and also keep good performance in the production build.

If the return value is mixed drafts or undefined, then use safeReturn() wrapper.

[markerikson]

✋ This still relies on Immer imports over in ./utils

[unadlib]

@phryneas @markerikson Mutative v0.3.2 already supports returning values in the draft functions. It is already fully compliant with produce interface in redux-toolkit and maintains good performance.

[markerikson]

@unadlib : nice!

One other question that came up is that we use several of Immer's utilities like isDraft and isDraftable, in order to check whether it's safe to pass certain values into produce().

Does mutative have anything like those?

[unadlib]

@unadlib : nice!

One other question that came up is that we use several of Immer's utilities like isDraft and isDraftable, in order to check whether it's safe to pass certain values into produce().

Does mutative have anything like those?

@markerikson Yes, Mutative also provides isDraft() and isDraftable().

[markerikson]

I rebased this branch against v2.0-integration.

Also, for kicks, I tried swapping all our usages of Immer to use Mutative instead. Unfortunately, there's several critical test failures, including basic createReducer behavior. Looks like it's hitting a Cannot perform 'has' on a proxy that has been revoked error.

https://github.com/reduxjs/redux-toolkit/tree/feature/try-mutative

[unadlib]

I rebased this branch against v2.0-integration.

Also, for kicks, I tried swapping all our usages of Immer to use Mutative instead. Unfortunately, there's several critical test failures, including basic createReducer behavior. Looks like it's hitting a Cannot perform 'has' on a proxy that has been revoked error.

https://github.com/reduxjs/redux-toolkit/tree/feature/try-mutative

hi, @markerikson, I will debug it later. Overall, regarding the integration with Reducer, Immer and Mutative are not exactly equivalent, Mutative supports more options.

[unadlib]

I rebased this branch against v2.0-integration.

Also, for kicks, I tried swapping all our usages of Immer to use Mutative instead. Unfortunately, there's several critical test failures, including basic createReducer behavior. Looks like it's hitting a Cannot perform 'has' on a proxy that has been revoked error.

https://github.com/reduxjs/redux-toolkit/tree/feature/try-mutative

For good performance, Mutative does not traverse mixed draft return values by default. When the return value is mixed with drafts, you can use safeReturn().

Mutative docs:

We recommend to enable strict in development mode and disable strict in production mode. This will ensure safe returns and also keep good performance in the production build. If the return value is mixed drafts or undefined, then use safeReturn().

You can fix tests like this,

https://github.com/reduxjs/redux-toolkit/blob/feature/try-mutative/packages/toolkit/src/tests/createReducer.test.ts#L174

- return state.concat({ ...newTodo, completed: false })
+ return safeReturn(state.concat({ ...newTodo, completed: false }))

https://github.com/reduxjs/redux-toolkit/blob/feature/try-mutative/packages/toolkit/src/tests/createReducer.test.ts#L181

- return state.map((todo, i) => {
+ return safeReturn(state.map((todo, i) => {

[markerikson]

Hmm. That definitely makes it harder to use mutative as a drop-in replacement, because we don't know ahead of time what users are going to do in their reducers.

I think the right answer here is that we'd need to inject a wrapper that uses safeReturn, something like:

const createReducerWithMutative = buildCreateReducer({
  createNextState: (state, recipe) => {
    return create(state, (draft) => {
      return safeReturn(recipe(draft))
    })
  },
  isDraft,
  isDraftable
})

[unadlib]

Hmm. That definitely makes it harder to use mutative as a drop-in replacement, because we don't know ahead of time what users are going to do in their reducers.

I think the right answer here is that we'd need to inject a wrapper that uses safeReturn, something like:

const createReducerWithMutative = buildCreateReducer({
  createNextState: (state, recipe) => {
    return create(state, (draft) => {
      return safeReturn(recipe(draft))
    })
  },
  isDraft,
  isDraftable
})

When the user uses createReducerWithImmer, if the reducer wants to return the value undefined, the reducer will have to return return Immer's nothing, and this implicit rule is similar to the case.

I also agree with you. Whether createReducerWithMutative returns undefined explicitly or not is left up to the user, like this.

const createReducerWithMutative = buildCreateReducer({
  createNextState: (state, recipe) => {
    return create(state, (draft) => {
      const value = recipe(draft);
      return value === undefined ? value : safeReturn(value);
    })
  },
  isDraft,
  isDraftable
})

[unadlib]

@markerikson I have refactored the implementation of the return value, once you upgrade to mutative v0.5.0, you will migrate Immer to Mutative smoothly, the relevant tests have passed.

The details are at unadlib/mutative#9

BTW, I tested the Immer v10 branch. Mutative is still more than 15x faster than Immer.

[giusepperaso]

Hi, I'm the author of structurajs. Thanks for this pull request!

After some work I think I finally managed to make the library fully compatible with redux toolkit

As an example I did setup a try-structura branch which passes all of the tests contained into the packages/toolkit folder

Note that in this example branch I removed Immer completely, and I put all of my exports inside the exports.ts file

Also, I called enableAutoFreeze and enableStandardPatches to make all of the tests pass, but they are not always necessary and could be disabled to gain some performance:

• structura does not allow the produced state to be modified (at compile time via typescript), so relying on object freeze could be a bit redundant
• structura by default does not use standard json patches, but a proprietary format based on linked lists which is slightly faster; you probably don't need standard json patches unless you have to manipulate them with another library

If you see any problem or if you need help with something let me know! @phryneas @markerikson

[markerikson]

@giusepperaso : thanks for working on that!

FWIW we really do want freezing in place, because we want users to see errors at runtime if they attempt to mutate outside of a reducer. (Not everyone uses TS!)

[EskiMojo14]

is it worth making a configurable version of createDraftSafeSelector too? that currently uses immer's isDraft() and current()

[netlify]

✅ Deploy Preview for redux-starter-kit-docs ready!

Name	Link
🔨 Latest commit	0888fde
🔍 Latest deploy log	https://app.netlify.com/sites/redux-starter-kit-docs/deploys/651b2b31e7e2f100084383b4
😎 Deploy Preview	https://deploy-preview-3074--redux-starter-kit-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[markerikson]

Found another similar lib: https://github.com/tnfe/limu

[unadlib]

Hi, I'm not sure if there are any next steps planned for this proposal.

Currently, the performance test information related to Immer and Mutative is as follows:

• Use mutative instead of immer instantdb/instant#243
• Improve sorted entity adapter sorting performance #4361 (comment)
• https://mutative.js.org/docs/getting-started/performance/

Perhaps performance improvement is not a high priority for the redux-toolkit. So maybe I could provide an additional Mutative lib based on the redux-toolkit?

But, if there are further plans for this proposal, I would be very happy to participate.

[markerikson]

@unadlib it's still an idea I'm interested in conceptually, but it's not on our roadmap to work on this any time soon. (I haven't had time to work on much RTK stuff lately, and when I do my focus will be on RTK Query features.)