-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
markerikson
merged 8 commits into
reduxjs:master
from
EskiMojo14:action-creator-middleware
May 5, 2023
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
67eef52
create action creator middleware
EskiMojo14 d8024b7
add CurriedGetDefaultMiddleware default
EskiMojo14 c87f959
fix gDM test
EskiMojo14 dc11155
Adds docs and fix test.
EskiMojo14 ed3ba43
fix order
EskiMojo14 b69e9df
unshift, not prepend
EskiMojo14 6189cc3
call the creator, not the action
EskiMojo14 fc53a5a
infer action name from type
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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], | ||
}) | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) { | ||
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) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
64 changes: 64 additions & 0 deletions
64
packages/toolkit/src/tests/actionCreatorInvariantMiddleware.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, looks good: