Immer 7

[mweststrate]

Planned changes:

1. produce will no longer accept primitives as base state (first arg) to produce. CC @markerikson do you see any potential problems here?
2. add utility to get a current state copy of a draft, for logging purposes etc?
3. more consistent handling of what attributes will be drafted (see [WIP] Improved support for getters #593):
   1. all own properties will be read upon cloning, like object.assign, except that non-enumerables will be included as well
4. more consistent freezing of properties
   1. only own, enumerable, non-symbolic, non-accessor properties will be frozen automatically
5. fix Iterating over a Set can cause mutations to be ignored #586
6. Investigate Different behavior with nested drafts depending on setUseProxies #588
7. add fix for Immer does not work with a polyfilled Symbol implementation #577
8. add fix for Should original return plain values as-is? #605
9. run perf tests

[markerikson]

Hmm. We do show some examples kinda like this in our current "basic tutorial", to introduce the idea:

const counterSlice = createSlice({
  name: 'counter',
  initialState: 0,
  reducers: {
    increment: state => state + 1,
    decrement: state => state - 1
  }
})

Obviously that doesn't take any actual advantage of Immer, but it's entirely valid, and it's getting the action creators generated for free as usual.

I don't expect that most real code does that, but having legal reducers like that stop working seems problematic.

What's the impetus for the change in behavior?

[mweststrate]

@markerikson there has been quite some confusion in the past where things were passed into produce and Immer didn't seem to work, because the thing passed wasn't draftable, e.g. a class that holds state, rather than a plain object (in which case it should have been marked immerable).

I suspect that you wrap all the reducer functions in RKJS with some other function? Making that conditional should make sure existing patterns keep working, a bit like return isDraftable(state) ? produce(state, draft => reducer(draft, action)) : reducer(state, action)

[markerikson]

Yeah, here's our current implementation as of v1.3.x:

export function createReducer<S>(
  initialState: S,
  mapOrBuilderCallback:
    | CaseReducers<S, any>
    | ((builder: ActionReducerMapBuilder<S>) => void)
): Reducer<S> {
  let actionsMap =
    typeof mapOrBuilderCallback === 'function'
      ? executeReducerBuilderCallback(mapOrBuilderCallback)
      : mapOrBuilderCallback

  return function(state = initialState, action): S {
    const caseReducer = actionsMap[action.type]
    if (caseReducer) {
      if (isDraft(state)) {
        // we must already be inside a `createNextState` call, likely because
        // this is being wrapped in `createReducer`, `createSlice`, or nested
        // inside an existing draft. It's safe to just pass the draft to the mutator.
        const draft = state as Draft<S> // We can assume this is already a draft
        return caseReducer(draft, action) || state
      } else {
        // @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>) => {
          return caseReducer(draft, action)
        })
      }
    }

    return state
  }
}

So we could presumably add another condition in there or something. In fact, maybe we add a second condition to the isDraft() clause?

[mweststrate]

yeah correct, that should do the trick :) (that would even be a forward & backward compatible change)

…

On Sat, May 23, 2020 at 6:00 PM Mark Erikson ***@***.***> wrote: Yeah, here's our current implementation as of v1.3.x: export function createReducer<S>( initialState: S, mapOrBuilderCallback: | CaseReducers<S, any> | ((builder: ActionReducerMapBuilder<S>) => void)): Reducer<S> { let actionsMap = typeof mapOrBuilderCallback === 'function' ? executeReducerBuilderCallback(mapOrBuilderCallback) : mapOrBuilderCallback return function(state = initialState, action): S { const caseReducer = actionsMap[action.type] if (caseReducer) { if (isDraft(state)) { // we must already be inside a `createNextState` call, likely because // this is being wrapped in `createReducer`, `createSlice`, or nested // inside an existing draft. It's safe to just pass the draft to the mutator. const draft = state as Draft<S> // We can assume this is already a draft return caseReducer(draft, action) || state } else { // @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>) => { return caseReducer(draft, action) }) } } return state }} So we could presumably add another condition in there or something. In fact, maybe we add a second condition to the isDraft() clause? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#603 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBAH6L2BI6HUCKELPSLRS76LDANCNFSM4NIBGG5A> .

[mweststrate]

actually, we can preserve the current behavior for primitives I guess, it is mostly about passing some undraftable object into produce which is a pitfall.

[corysimmons]

+10000 for being able to log Proxy's and see actual data. Some situations can be really hard to debug without this. If it's a perf thing, just show Proxy data with NODE_ENV=development

[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 7bb29d5:

Sandbox	Source
Immer sandbox	Configuration

[aleclarson]

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀