Repository: reduxjs/redux-toolkit
Pull request #631 (2020-06-22 19:26:11)

allow to return undefined for null-states in caseReducers
This was just brought up in the chat. `null` is non-draftable, but we should accept the reducer not returning anything in that specific case & not modify the state (keeping it `null`).
This might be the case where a reducer has the state `S | null` and just skips state modification while `draft` is `null`.

netlify[bot]: Deploy preview for *redux-starter-kit-docs* ready!

Built with commit 4702c8c719dcb269655cd3959516f25b14ecac72

https://deploy-preview-631--redux-starter-kit-docs.netlify.app
codesandbox[bot]: This pull request is automatically built and testable in [CodeSandbox](https://codesandbox.io).

  To see build info of the built libraries, click [here](https://ci.codesandbox.io/status/reduxjs/redux-toolkit/pr/631/builds/34151) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4702c8c719dcb269655cd3959516f25b14ecac72:

|Sandbox| Source |
|--|--|
|[serene-monad-klgfd](https://codesandbox.io/s/serene-monad-klgfd)| Configuration |
|[heuristic-forest-s9i2m](https://codesandbox.io/s/heuristic-forest-s9i2m)| Configuration |
|[blissful-brahmagupta-90sio](https://codesandbox.io/s/blissful-brahmagupta-90sio)| Configuration |

markerikson: For whatever reason I'm not entirely grokking the bug here, even after having read through the discussion in the channel.

Why is `return state` an issue when the state is null?
phryneas: `state` is `null` => `state` is not "draftable", so we don't run the reducer with `immer`.
In that case we have to prevent the reducer from returning `undefined`, because that would set the state to `undefined`. Which is why usually, we'd throw an error to tell the user that they *really*  should return something.

The problem in the chat was that the type of the state was *sometimes* an object (draftable, takes the `immer` path) and *sometimes* `null` (non-draftable, does not take the `immer` path).


So if you have a caseReducer like this, for a state of `S|null`:
```
caseReducer(state, action) {
  if (state && action.payload.something == "foo") {
    state.something = action.payload.otherThing
  }
}
```
That would make sense from the point of someone writing that reducer - but sometimes it would take the `produce` path and sometimes not, causing an error.

So we'd have to handle the `undefined` return value extra for the case that the `state` was `null` to begin with - and assume that it just would imply "no not modify the state" instead of "reset it to `undefined`" or "throw an error".
markerikson: Awright, if you say this works right, I believe you :)

Patch:
diff --git a/src/createReducer.ts b/src/createReducer.ts
index 5fe7784b..2fb07bcc 100644
--- a/src/createReducer.ts
+++ b/src/createReducer.ts
@@ -160,6 +160,9 @@ export function createReducer<S>(
           const result = caseReducer(previousState as any, action)
 
           if (typeof result === 'undefined') {
+            if (previousState === null) {
+              return previousState
+            }
             throw Error(
               'A case reducer on a non-draftable value must not return undefined'
             )


Test patch:
diff --git a/src/createReducer.test.ts b/src/createReducer.test.ts
index a33fff8d..780a2004 100644
--- a/src/createReducer.test.ts
+++ b/src/createReducer.test.ts
@@ -274,6 +274,20 @@ describe('createReducer', () => {
         `"A case reducer on a non-draftable value must not return undefined"`
       )
     })
+    test('allows you to return undefined if the state was null, thus skipping an update', () => {
+      const reducer = createReducer(null as number | null, builder =>
+        builder.addCase(
+          'decrement',
+          (state, action: { type: 'decrement'; payload: number }) => {
+            if (typeof state === 'number') {
+              return state - action.payload
+            }
+          }
+        )
+      )
+      expect(reducer(0, decrement(5))).toBe(-5)
+      expect(reducer(null, decrement(5))).toBe(null)
+    })
     test('allows you to return null', () => {
       const reducer = createReducer(0 as number | null, builder =>
         builder.addCase(