Merge pull request #3414 from EskiMojo14/action-creator-middleware

fixes #3413

Author: markerikson

docs/api/actionCreatorMiddleware.mdx

@@ -0,0 +1,67 @@
+---
+id: actionCreatorMiddleware
+title: Action Creator Middleware
+sidebar_label: Action Creator Middleware
+hide_title: true
+---
+
+ 
+
+# Action Creator Middleware
+
+A custom middleware that detects if an action creator has been mistakenly dispatched, instead of being called before dispatching.
+
+A common mistake is to call `dispatch(actionCreator)` instead of `dispatch(actionCreator())`.
+This tends to "work" as the action creator has the static `type` property, but can lead to unexpected behaviour.
+
+## Options
+
+```ts no-transpile
+export interface ActionCreatorInvariantMiddlewareOptions {
+  /**
+   * The function to identify whether a value is an action creator.
+   * The default checks for a function with a static type property and match method.
+   */
+  isActionCreator?: (action: unknown) => action is Function & { type?: unknown }
+}
+```
+
+## Exports
+
+### `createActionCreatorInvariantMiddleware`
+
+Creates an instance of the action creator check middleware, with the given options.
+
+You will most likely not need to call this yourself, as `getDefaultMiddleware` already does so.
+Example:
+
+```ts
+// file: reducer.ts noEmit
+
+export default function (state = {}, action: any) {
+  return state
+}
+
+// file: store.ts
+
+import {
+  configureStore,
+  createActionCreatorInvariantMiddleware,
+} from '@reduxjs/toolkit'
+import reducer from './reducer'
+
+// Augment middleware to consider all functions with a static type property to be action creators
+const isActionCreator = (
+  action: unknown
+): action is Function & { type: unknown } =>
+  typeof action === 'function' && 'type' in action
+
+const actionCreatorMiddleware = createActionCreatorInvariantMiddleware({
+  isActionCreator,
+})
+
+const store = configureStore({
+  reducer,
+  middleware: [actionCreatorMiddleware],
+})
+```

docs/api/getDefaultMiddleware.mdx

@@ -70,7 +70,7 @@ It is preferable to use the chainable `.concat(...)` and `.prepend(...)` methods
 
 One of the goals of Redux Toolkit is to provide opinionated defaults and prevent common mistakes. As part of that,
 `getDefaultMiddleware` includes some middleware that are added **in development builds of your app only** to
-provide runtime checks for two common issues:
+provide runtime checks for three common issues:
 
 - [Immutability check middleware](./immutabilityMiddleware.mdx): deeply compares
   state values for mutations. It can detect mutations in reducers during a dispatch, and also mutations that occur between
@@ -82,13 +82,21 @@ provide runtime checks for two common issues:
   such as functions, Promises, Symbols, and other non-plain-JS-data values. When a non-serializable value is detected, a
   console error will be printed with the key path for where the non-serializable value was detected.
 
+- [Action creator check middleware](./actionCreatorMiddleware.mdx): another custom middleware created specifically for use in Redux Toolkit.
+  Identifies when an action creator was mistakenly dispatched without being called, and warns to console with the action type.
+
 In addition to these development tool middleware, it also adds [`redux-thunk`](https://github.com/reduxjs/redux-thunk)
 by default, since thunks are the basic recommended side effects middleware for Redux.
 
 Currently, the return value is:
 
 ```js
-const middleware = [thunk, immutableStateInvariant, serializableStateInvariant]
+const middleware = [
+  actionCreatorInvariant,
+  immutableStateInvariant,
+  thunk,
+  serializableStateInvariant,
+]
 ```
 
 ### Production
@@ -153,10 +161,15 @@ interface SerializableStateInvariantMiddlewareOptions {
   // See "Serializability Middleware" page for definition
 }
 
+interface ActionCreatorInvariantMiddlewareOptions {
+  // See "Action Creator Middleware" page for definition
+}
+
 interface GetDefaultMiddlewareOptions {
   thunk?: boolean | ThunkOptions
   immutableCheck?: boolean | ImmutableStateInvariantMiddlewareOptions
   serializableCheck?: boolean | SerializableStateInvariantMiddlewareOptions
+  actionCreatorCheck?: boolean | ActionCreatorInvariantMiddlewareOptions
 }
 
 function getDefaultMiddleware<S = any>(

packages/toolkit/src/actionCreatorInvariantMiddleware.ts

@@ -0,0 +1,34 @@
+import type { Middleware } from 'redux'
+import { isActionCreator as isRTKAction } from './createAction'
+
+export interface ActionCreatorInvariantMiddlewareOptions {
+  /**
+   * The function to identify whether a value is an action creator.
+   * The default checks for a function with a static type property and match method.
+   */
+  isActionCreator?: (action: unknown) => action is Function & { type?: unknown }
+}
+
+export function getMessage(type?: unknown) {
+  const splitType = type ? `${type}`.split('/') : []
+  const actionName = splitType[splitType.length - 1] || 'actionCreator'
+  return `Detected an action creator with type "${
+    type || 'unknown'
+  }" being dispatched. 
+Make sure you're calling the action creator before dispatching, i.e. \`dispatch(${actionName}())\` instead of \`dispatch(${actionName})\`. This is necessary even if the action has no payload.`
+}
+
+export function createActionCreatorInvariantMiddleware(
+  options: ActionCreatorInvariantMiddlewareOptions = {}
+): Middleware {
+  if (process.env.NODE_ENV === 'production') {
+    return () => (next) => (action) => next(action)
+  }
+  const { isActionCreator = isRTKAction } = options
+  return () => (next) => (action) => {
+    if (isActionCreator(action)) {
+      console.warn(getMessage(action.type))
+    }
+    return next(action)
+  }
+}

packages/toolkit/src/createAction.ts

@@ -5,6 +5,7 @@ import type {
   IfVoid,
   IsAny,
 } from './tsHelpers'
+import { hasMatchFunction } from './tsHelpers'
 import isPlainObject from './isPlainObject'
 
 /**
@@ -293,6 +294,20 @@ export function isAction(action: unknown): action is Action<unknown> {
   return isPlainObject(action) && 'type' in action
 }
 
+/**
+ * Returns true if value is an RTK-like action creator, with a static type property and match method.
+ */
+export function isActionCreator(
+  action: unknown
+): action is BaseActionCreator<unknown, string> & Function {
+  return (
+    typeof action === 'function' &&
+    'type' in action &&
+    // hasMatchFunction only wants Matchers but I don't see the point in rewriting it
+    hasMatchFunction(action as any)
+  )
+}
+
 /**
  * Returns true if value is an action with a string type and valid Flux Standard Action keys.
  */

packages/toolkit/src/getDefaultMiddleware.ts

@@ -1,6 +1,8 @@
 import type { Middleware, AnyAction } from 'redux'
 import type { ThunkMiddleware } from 'redux-thunk'
 import thunkMiddleware from 'redux-thunk'
+import type { ActionCreatorInvariantMiddlewareOptions } from './actionCreatorInvariantMiddleware'
+import { createActionCreatorInvariantMiddleware } from './actionCreatorInvariantMiddleware'
 import type { ImmutableStateInvariantMiddlewareOptions } from './immutableStateInvariantMiddleware'
 /* PROD_START_REMOVE_UMD */
 import { createImmutableStateInvariantMiddleware } from './immutableStateInvariantMiddleware'
@@ -23,6 +25,7 @@ interface GetDefaultMiddlewareOptions {
   thunk?: boolean | ThunkOptions
   immutableCheck?: boolean | ImmutableStateInvariantMiddlewareOptions
   serializableCheck?: boolean | SerializableStateInvariantMiddlewareOptions
+  actionCreatorCheck?: boolean | ActionCreatorInvariantMiddlewareOptions
 }
 
 export type ThunkMiddlewareFor<
@@ -41,6 +44,7 @@ export type CurriedGetDefaultMiddleware<S = any> = <
     thunk: true
     immutableCheck: true
     serializableCheck: true
+    actionCreatorCheck: true
   }
 >(
   options?: O
@@ -72,6 +76,7 @@ export function getDefaultMiddleware<
     thunk: true
     immutableCheck: true
     serializableCheck: true
+    actionCreatorCheck: true
   }
 >(
   options: O = {} as O
@@ -80,6 +85,7 @@ export function getDefaultMiddleware<
     thunk = true,
     immutableCheck = true,
     serializableCheck = true,
+    actionCreatorCheck = true,
   } = options
 
   let middlewareArray = new MiddlewareArray<Middleware[]>()
@@ -120,6 +126,17 @@ export function getDefaultMiddleware<
         createSerializableStateInvariantMiddleware(serializableOptions)
       )
     }
+    if (actionCreatorCheck) {
+      let actionCreatorOptions: ActionCreatorInvariantMiddlewareOptions = {}
+
+      if (!isBoolean(actionCreatorCheck)) {
+        actionCreatorOptions = actionCreatorCheck
+      }
+
+      middlewareArray.unshift(
+        createActionCreatorInvariantMiddleware(actionCreatorOptions)
+      )
+    }
   }
 
   return middlewareArray as any

packages/toolkit/src/index.ts

@@ -40,6 +40,7 @@ export {
   createAction,
   getType,
   isAction,
+  isActionCreator,
   isFSA as isFluxStandardAction,
 } from './createAction'
 export type {
@@ -78,6 +79,8 @@ export type {
   CaseReducerWithPrepare,
   SliceActionCreator,
 } from './createSlice'
+export type { ActionCreatorInvariantMiddlewareOptions } from './actionCreatorInvariantMiddleware'
+export { createActionCreatorInvariantMiddleware } from './actionCreatorInvariantMiddleware'
 export {
   // js
   createImmutableStateInvariantMiddleware,

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

@@ -0,0 +1,64 @@
+import type { ActionCreatorInvariantMiddlewareOptions } from '@internal/actionCreatorInvariantMiddleware'
+import { getMessage } from '@internal/actionCreatorInvariantMiddleware'
+import { createActionCreatorInvariantMiddleware } from '@internal/actionCreatorInvariantMiddleware'
+import type { Dispatch, MiddlewareAPI } from '@reduxjs/toolkit'
+import { createAction } from '@reduxjs/toolkit'
+
+describe('createActionCreatorInvariantMiddleware', () => {
+  const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(() => {})
+
+  afterEach(() => {
+    consoleSpy.mockClear()
+  })
+  afterAll(() => {
+    consoleSpy.mockRestore()
+  })
+
+  const dummyAction = createAction('aSlice/anAction')
+
+  it('sends the action through the middleware chain', () => {
+    const next: Dispatch = (action) => ({
+      ...action,
+      returned: true,
+    })
+    const dispatch = createActionCreatorInvariantMiddleware()(
+      {} as MiddlewareAPI
+    )(next)
+
+    expect(dispatch(dummyAction())).toEqual({
+      ...dummyAction(),
+      returned: true,
+    })
+  })
+
+  const makeActionTester = (
+    options?: ActionCreatorInvariantMiddlewareOptions
+  ) =>
+    createActionCreatorInvariantMiddleware(options)({} as MiddlewareAPI)(
+      (action) => action
+    )
+
+  it('logs a warning to console if an action creator is mistakenly dispatched', () => {
+    const testAction = makeActionTester()
+
+    testAction(dummyAction())
+
+    expect(consoleSpy).not.toHaveBeenCalled()
+
+    testAction(dummyAction)
+
+    expect(consoleSpy).toHaveBeenLastCalledWith(getMessage(dummyAction.type))
+  })
+
+  it('allows passing a custom predicate', () => {
+    let predicateCalled = false
+    const testAction = makeActionTester({
+      isActionCreator(action): action is Function {
+        predicateCalled = true
+        return false
+      },
+    })
+    testAction(dummyAction())
+    expect(predicateCalled).toBe(true)
+  })
+})

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

@@ -75,9 +75,10 @@ describe('configureStore', () => {
         Object
       )
       expect(redux.applyMiddleware).toHaveBeenCalledWith(
-        expect.any(Function), // thunk
         expect.any(Function), // immutableCheck
-        expect.any(Function) // serializableCheck
+        expect.any(Function), // thunk
+        expect.any(Function), // serializableCheck
+        expect.any(Function) // actionCreatorCheck
       )
       expect(devtools.composeWithDevTools).toHaveBeenCalled() // @remap-prod-remove-line-line
       expect(redux.createStore).toHaveBeenCalledWith(