Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create action creator middleware #3414

Merged
merged 8 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions docs/api/actionCreatorMiddleware.mdx
Original file line number Diff line number Diff line change
@@ -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],
})
```
17 changes: 15 additions & 2 deletions docs/api/getDefaultMiddleware.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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>(
Expand Down
34 changes: 34 additions & 0 deletions packages/toolkit/src/actionCreatorInvariantMiddleware.ts
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ My only question here is whether this is going to get properly tree-shaken. We switched from Terser to ESBuild for minification, and I was seeing that the other dev check middleware needed to have message functions like this inside the middleware, and inside an else in order to shake properly.

Let me do a prod build of a test app and see what happens.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, looks good:

image

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)
}
}
15 changes: 15 additions & 0 deletions packages/toolkit/src/createAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
IfVoid,
IsAny,
} from './tsHelpers'
import { hasMatchFunction } from './tsHelpers'
import isPlainObject from './isPlainObject'

/**
Expand Down Expand Up @@ -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.
*/
Expand Down
17 changes: 17 additions & 0 deletions packages/toolkit/src/getDefaultMiddleware.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -23,6 +25,7 @@ interface GetDefaultMiddlewareOptions {
thunk?: boolean | ThunkOptions
immutableCheck?: boolean | ImmutableStateInvariantMiddlewareOptions
serializableCheck?: boolean | SerializableStateInvariantMiddlewareOptions
actionCreatorCheck?: boolean | ActionCreatorInvariantMiddlewareOptions
}

export type ThunkMiddlewareFor<
Expand All @@ -41,6 +44,7 @@ export type CurriedGetDefaultMiddleware<S = any> = <
thunk: true
immutableCheck: true
serializableCheck: true
actionCreatorCheck: true
}
>(
options?: O
Expand Down Expand Up @@ -72,6 +76,7 @@ export function getDefaultMiddleware<
thunk: true
immutableCheck: true
serializableCheck: true
actionCreatorCheck: true
}
>(
options: O = {} as O
Expand All @@ -80,6 +85,7 @@ export function getDefaultMiddleware<
thunk = true,
immutableCheck = true,
serializableCheck = true,
actionCreatorCheck = true,
} = options

let middlewareArray = new MiddlewareArray<Middleware[]>()
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions packages/toolkit/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export {
createAction,
getType,
isAction,
isActionCreator,
isFSA as isFluxStandardAction,
} from './createAction'
export type {
Expand Down Expand Up @@ -78,6 +79,8 @@ export type {
CaseReducerWithPrepare,
SliceActionCreator,
} from './createSlice'
export type { ActionCreatorInvariantMiddlewareOptions } from './actionCreatorInvariantMiddleware'
export { createActionCreatorInvariantMiddleware } from './actionCreatorInvariantMiddleware'
export {
// js
createImmutableStateInvariantMiddleware,
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
})
})
5 changes: 3 additions & 2 deletions packages/toolkit/src/tests/configureStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading