reducer builder: track call order using Set, to allow differentiating between addAsyncThunk and addMatcher

packages/toolkit/src/mapBuilders.ts

@@ -38,6 +38,17 @@ export type TypedActionCreator<Type extends string> = {
   type: Type
 }
 
+interface InternalMapBuilder<State> {
+  addCase(
+    typeOrActionCreator: string | TypedActionCreator<any>,
+    reducer: CaseReducer<State, any>,
+  ): InternalMapBuilder<State>
+  addMatcher(
+    matcher: TypeGuard<any>,
+    reducer: CaseReducer<State, any>,
+  ): InternalMapBuilder<State>
+}
+
 /**
  * A builder for an action <-> reducer map.
  *
@@ -47,7 +58,7 @@ export interface ActionReducerMapBuilder<State> {
   /**
    * Adds a case reducer to handle a single exact action type.
    * @remarks
-   * All calls to `builder.addCase` must come before any calls to `builder.addMatcher` or `builder.addDefaultCase`.
+   * All calls to `builder.addCase` must come before any calls to `builder.addAsyncThunk`, `builder.addMatcher` or `builder.addDefaultCase`.
    * @param actionCreator - Either a plain action type string, or an action creator generated by [`createAction`](./createAction) that can be used to determine the action type.
    * @param reducer - The actual case reducer function.
    */
@@ -70,7 +81,7 @@ export interface ActionReducerMapBuilder<State> {
   /**
    * Adds case reducers to handle actions based on a `AsyncThunk` action creator.
    * @remarks
-   * All calls to `builder.addAsyncThunk` must come before after any calls to `builder.addCase` and before any calls to `builder.addMatcher` or `builder.addDefaultCase`.
+   * All calls to `builder.addAsyncThunk` must come after any calls to `builder.addCase` and before any calls to `builder.addMatcher` or `builder.addDefaultCase`.
    * @param asyncThunk - The async thunk action creator itself.
    * @param reducers - A mapping from each of the `AsyncThunk` action types to the case reducer that should handle those actions.
    * @example
@@ -191,6 +202,13 @@ const reducer = createReducer(initialState, builder => {
   addDefaultCase(reducer: CaseReducer<State, Action>): {}
 }
 
+const callOrder: Array<keyof ActionReducerMapBuilder<any>> = [
+  'addCase',
+  'addAsyncThunk',
+  'addMatcher',
+  'addDefaultCase',
+]
+
 export function executeReducerBuilderCallback<S>(
   builderCallback: (builder: ActionReducerMapBuilder<S>) => void,
 ): [
@@ -201,94 +219,90 @@ export function executeReducerBuilderCallback<S>(
   const actionsMap: CaseReducers<S, any> = {}
   const actionMatchers: ActionMatcherDescriptionCollection<S> = []
   let defaultCaseReducer: CaseReducer<S, Action> | undefined
-  const builder = {
+  const called = new Set<keyof ActionReducerMapBuilder<S>>()
+  function ensureCallOrder(
+    method: keyof ActionReducerMapBuilder<any>,
+    allowCallTwice = true,
+  ) {
+    if (called.has(method) && !allowCallTwice) {
+      throw new Error(`\`builder.${method}\` can only be called once`)
+    }
+    for (const otherMethod of callOrder.slice(callOrder.indexOf(method) + 1)) {
+      if (called.has(otherMethod)) {
+        throw new Error(
+          `\`builder.${method}\` should only be called before calling \`builder.${otherMethod}\``,
+        )
+      }
+    }
+    called.add(method)
+  }
+
+  // builder methods without dev checks
+  const rawBuilder: InternalMapBuilder<S> = {
     addCase(
       typeOrActionCreator: string | TypedActionCreator<any>,
-      reducer: CaseReducer<S>,
+      reducer: CaseReducer<S, any>,
     ) {
-      if (process.env.NODE_ENV !== 'production') {
-        /*
-         to keep the definition by the user in line with actual behavior,
-         we enforce `addCase` to always be called before calling `addMatcher`
-         as matching cases take precedence over matchers
-         */
-        if (actionMatchers.length > 0) {
-          throw new Error(
-            '`builder.addCase` should only be called before calling `builder.addMatcher`',
-          )
-        }
-        if (defaultCaseReducer) {
-          throw new Error(
-            '`builder.addCase` should only be called before calling `builder.addDefaultCase`',
-          )
-        }
-      }
       const type =
         typeof typeOrActionCreator === 'string'
           ? typeOrActionCreator
           : typeOrActionCreator.type
       if (!type) {
-        throw new Error(
-          '`builder.addCase` cannot be called with an empty action type',
-        )
+        throw new Error('A reducer cannot be defined for an empty action type')
       }
       if (type in actionsMap) {
         throw new Error(
-          '`builder.addCase` cannot be called with two reducers for the same action type ' +
-            `'${type}'`,
+          `A reducer already exists for the action type '${type}'`,
         )
       }
       actionsMap[type] = reducer
-      return builder
+      return rawBuilder
+    },
+    addMatcher(matcher: TypeGuard<any>, reducer) {
+      actionMatchers.push({ matcher, reducer })
+      return rawBuilder
     },
-    addAsyncThunk<
-      Returned,
-      ThunkArg,
-      ThunkApiConfig extends AsyncThunkConfig = {},
-    >(
-      asyncThunk: AsyncThunk<Returned, ThunkArg, ThunkApiConfig>,
-      reducers: AsyncThunkReducers<S, ThunkArg, Returned, ThunkApiConfig>,
+  }
+  const builder: ActionReducerMapBuilder<S> = {
+    addCase(
+      typeOrActionCreator: string | TypedActionCreator<any>,
+      reducer: CaseReducer<S>,
     ) {
       if (process.env.NODE_ENV !== 'production') {
-        // since this uses both action cases and matchers, we can't enforce the order in runtime other than checking for default case
-        if (defaultCaseReducer) {
-          throw new Error(
-            '`builder.addAsyncThunk` should only be called before calling `builder.addDefaultCase`',
-          )
-        }
+        /*
+         to keep the definition by the user in line with actual behavior,
+         we enforce `addCase` to always be called before calling `addMatcher`
+         as matching cases take precedence over matchers
+         */
+        ensureCallOrder('addCase')
+      }
+      rawBuilder.addCase(typeOrActionCreator, reducer)
+      return builder
+    },
+    addAsyncThunk(asyncThunk, reducers) {
+      if (process.env.NODE_ENV !== 'production') {
+        ensureCallOrder('addAsyncThunk')
       }
       if (reducers.pending)
-        actionsMap[asyncThunk.pending.type] = reducers.pending
+        rawBuilder.addCase(asyncThunk.pending, reducers.pending)
       if (reducers.rejected)
-        actionsMap[asyncThunk.rejected.type] = reducers.rejected
+        rawBuilder.addCase(asyncThunk.rejected, reducers.rejected)
       if (reducers.fulfilled)
-        actionsMap[asyncThunk.fulfilled.type] = reducers.fulfilled
+        rawBuilder.addCase(asyncThunk.fulfilled, reducers.fulfilled)
       if (reducers.settled)
-        actionMatchers.push({
-          matcher: asyncThunk.settled,
-          reducer: reducers.settled,
-        })
+        rawBuilder.addMatcher(asyncThunk.settled, reducers.settled)
       return builder
     },
-    addMatcher<A>(
-      matcher: TypeGuard<A>,
-      reducer: CaseReducer<S, A extends Action ? A : A & Action>,
-    ) {
+    addMatcher(matcher: TypeGuard<any>, reducer) {
       if (process.env.NODE_ENV !== 'production') {
-        if (defaultCaseReducer) {
-          throw new Error(
-            '`builder.addMatcher` should only be called before calling `builder.addDefaultCase`',
-          )
-        }
+        ensureCallOrder('addMatcher')
       }
-      actionMatchers.push({ matcher, reducer })
+      rawBuilder.addMatcher(matcher, reducer)
       return builder
     },
-    addDefaultCase(reducer: CaseReducer<S, Action>) {
+    addDefaultCase(reducer) {
       if (process.env.NODE_ENV !== 'production') {
-        if (defaultCaseReducer) {
-          throw new Error('`builder.addDefaultCase` can only be called once')
-        }
+        ensureCallOrder('addDefaultCase', false)
       }
       defaultCaseReducer = reducer
       return builder

packages/toolkit/src/tests/createReducer.test.ts

@@ -353,7 +353,7 @@ describe('createReducer', () => {
             .addCase(decrement, (state, action) => state - action.payload)
         })
       }).toThrowErrorMatchingInlineSnapshot(
-        `[Error: \`builder.addCase\` cannot be called with two reducers for the same action type 'increment']`,
+        `[Error: A reducer already exists for the action type 'increment']`,
       )
       expect(() => {
         createReducer(0, (builder) => {
@@ -363,7 +363,7 @@ describe('createReducer', () => {
             .addCase(decrement, (state, action) => state - action.payload)
         })
       }).toThrowErrorMatchingInlineSnapshot(
-        `[Error: \`builder.addCase\` cannot be called with two reducers for the same action type 'increment']`,
+        `[Error: A reducer already exists for the action type 'increment']`,
       )
     })
 
@@ -381,7 +381,7 @@ describe('createReducer', () => {
           )
         })
       }).toThrowErrorMatchingInlineSnapshot(
-        `[Error: \`builder.addCase\` cannot be called with an empty action type]`,
+        `[Error: A reducer cannot be defined for an empty action type]`,
       )
     })
   })
@@ -498,7 +498,25 @@ describe('createReducer', () => {
         stringActions: 0,
       })
     })
-    test('calling addCase, addMatcher and addDefaultCase in a nonsensical order should result in an error in development mode', () => {
+    test('calling addCase, addAsyncThunk, addMatcher and addDefaultCase in a nonsensical order should result in an error in development mode', () => {
+      expect(() =>
+        createReducer(initialState, (builder: any) =>
+          builder
+            .addAsyncThunk(addTodoThunk, {})
+            .addCase(incrementBy, () => {}),
+        ),
+      ).toThrowErrorMatchingInlineSnapshot(
+        `[Error: \`builder.addCase\` should only be called before calling \`builder.addAsyncThunk\`]`,
+      )
+      expect(() =>
+        createReducer(initialState, (builder: any) =>
+          builder
+            .addMatcher(numberActionMatcher, () => {})
+            .addAsyncThunk(addTodoThunk, {}),
+        ),
+      ).toThrowErrorMatchingInlineSnapshot(
+        `[Error: \`builder.addAsyncThunk\` should only be called before calling \`builder.addMatcher\`]`,
+      )
       expect(() =>
         createReducer(initialState, (builder: any) =>
           builder
@@ -582,6 +600,27 @@ describe('createReducer', () => {
         `[Error: \`builder.addAsyncThunk\` should only be called before calling \`builder.addDefaultCase\`]`,
       )
     })
+    test('calling addAsyncThunk after addMatcher should result in an error in development mode', () => {
+      expect(() =>
+        createReducer(initialState, (builder: any) =>
+          builder
+            .addMatcher(
+              () => true,
+              () => {},
+            )
+            .addAsyncThunk(addTodoThunk, {}),
+        ),
+      ).toThrowErrorMatchingInlineSnapshot(`[Error: \`builder.addAsyncThunk\` should only be called before calling \`builder.addMatcher\`]`)
+    })
+    test('calling addAsyncThunk for an action already covered by addCase should result in an error in development mode', () => {
+      expect(() =>
+        createReducer(initialState, (builder: any) =>
+          builder
+            .addCase(addTodoThunk.pending, () => {})
+            .addAsyncThunk(addTodoThunk, { pending() {} }),
+        ),
+      ).toThrowErrorMatchingInlineSnapshot(`[Error: A reducer already exists for the action type 'todos/add/pending']`)
+    })
   })
 })
 

packages/toolkit/src/tests/createSlice.test.ts

@@ -262,7 +262,7 @@ describe('createSlice', () => {
           })
           slice.reducer(undefined, { type: 'unrelated' })
         }).toThrowErrorMatchingInlineSnapshot(
-          `[Error: \`builder.addCase\` cannot be called with two reducers for the same action type 'increment']`,
+          `[Error: A reducer already exists for the action type 'increment']`,
         )
       })