From cf44723155b218510f745634afe1a178a08219fa Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sat, 9 May 2020 21:15:05 +0200 Subject: [PATCH 01/11] attempt at actionListenerMiddleware --- src/createAction.ts | 2 +- .../createActionListenerMiddleware.test.ts | 236 +++++++++++++++ .../createActionListenerMiddleware.ts | 268 ++++++++++++++++++ 3 files changed, 505 insertions(+), 1 deletion(-) create mode 100644 src/entities/createActionListenerMiddleware.test.ts create mode 100644 src/entities/createActionListenerMiddleware.ts diff --git a/src/createAction.ts b/src/createAction.ts index 93558d7be6..ca3ffbac7a 100644 --- a/src/createAction.ts +++ b/src/createAction.ts @@ -81,7 +81,7 @@ export type _ActionCreatorWithPreparedPayload< * * @inheritdoc {redux#ActionCreator} */ -interface BaseActionCreator { +export interface BaseActionCreator { type: T match(action: Action): action is PayloadAction } diff --git a/src/entities/createActionListenerMiddleware.test.ts b/src/entities/createActionListenerMiddleware.test.ts new file mode 100644 index 0000000000..b94a5ec58b --- /dev/null +++ b/src/entities/createActionListenerMiddleware.test.ts @@ -0,0 +1,236 @@ +import { configureStore } from '../configureStore' +import { + createActionListenerMiddleware, + addListenerAction, + removeListenerAction +} from './createActionListenerMiddleware' +import { createAction } from '../createAction' + +const middlewareApi = { + getState: expect.any(Function), + dispatch: expect.any(Function) +} + +describe('createActionListenerMiddleware', () => { + let store = configureStore({ + reducer: () => ({}), + middleware: [createActionListenerMiddleware()] as const + }) + let reducer: jest.Mock + let middleware: ReturnType + + const testAction1 = createAction('testAction1') + type TestAction1 = ReturnType + const testAction2 = createAction('testAction2') + + beforeEach(() => { + middleware = createActionListenerMiddleware() + reducer = jest.fn(() => ({})) + store = configureStore({ + reducer, + middleware: [middleware] as const + }) + }) + + test('directly subscribing', () => { + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener) + + store.dispatch(testAction1('a')) + store.dispatch(testAction2('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([ + [testAction1('a'), middlewareApi], + [testAction1('c'), middlewareApi] + ]) + }) + + test('subscribing with the same listener will not make it trigger twice (like EventTarget.addEventListener())', () => { + /** + * thoughts: allow to use this to override the options for a listener? + * right now it's just exiting if the listener is already registered + */ + + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener) + middleware.addListener(testAction1, listener) + + store.dispatch(testAction1('a')) + store.dispatch(testAction2('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([ + [testAction1('a'), middlewareApi], + [testAction1('c'), middlewareApi] + ]) + }) + + test('unsubscribing via callback', () => { + const listener = jest.fn((_: TestAction1) => {}) + + const unsubscribe = middleware.addListener(testAction1, listener) + + store.dispatch(testAction1('a')) + unsubscribe() + store.dispatch(testAction2('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]]) + }) + + test('directly unsubscribing', () => { + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener) + + store.dispatch(testAction1('a')) + + middleware.removeListener(testAction1, listener) + store.dispatch(testAction2('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]]) + }) + + test('subscribing via action', () => { + const listener = jest.fn((_: TestAction1) => {}) + + store.dispatch(addListenerAction(testAction1, listener)) + + store.dispatch(testAction1('a')) + store.dispatch(testAction2('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([ + [testAction1('a'), middlewareApi], + [testAction1('c'), middlewareApi] + ]) + }) + + test('unsubscribing via callback from dispatch', () => { + const listener = jest.fn((_: TestAction1) => {}) + + const unsubscribe = store.dispatch(addListenerAction(testAction1, listener)) + + store.dispatch(testAction1('a')) + // @ts-ignore TODO types + unsubscribe() + store.dispatch(testAction2('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]]) + }) + + test('unsubscribing via action', () => { + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener) + + store.dispatch(testAction1('a')) + + store.dispatch(removeListenerAction(testAction1, listener)) + store.dispatch(testAction2('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]]) + }) + + test('"condition" allows to skip the listener', () => { + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener, { + condition(action) { + return action.payload !== 'b' + } + }) + + store.dispatch(testAction1('a')) + store.dispatch(testAction1('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([ + [testAction1('a'), middlewareApi], + [testAction1('c'), middlewareApi] + ]) + }) + + test('"once" unsubscribes the listener automatically after one use', () => { + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener, { + once: true + }) + + store.dispatch(testAction1('a')) + store.dispatch(testAction1('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]]) + }) + + test('combining "once" with "condition', () => { + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener, { + once: true, + condition(action) { + return action.payload === 'b' + } + }) + + store.dispatch(testAction1('a')) + store.dispatch(testAction1('b')) + store.dispatch(testAction1('c')) + + expect(listener.mock.calls).toEqual([[testAction1('b'), middlewareApi]]) + }) + + test('by default, actions are forwarded to the store', () => { + reducer.mockClear() + + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener) + + store.dispatch(testAction1('a')) + + expect(reducer.mock.calls).toEqual([[{}, testAction1('a')]]) + }) + + test('"preventPropagation" prevents actions from being forwarded to the store', () => { + reducer.mockClear() + + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener, { preventPropagation: true }) + + store.dispatch(testAction1('a')) + + expect(reducer.mock.calls).toEqual([]) + }) + + test('combining "preventPropagation" and "condition', () => { + reducer.mockClear() + + const listener = jest.fn((_: TestAction1) => {}) + + middleware.addListener(testAction1, listener, { + preventPropagation: true, + condition(action) { + return action.payload === 'b' + } + }) + + store.dispatch(testAction1('a')) + store.dispatch(testAction1('b')) + store.dispatch(testAction1('c')) + + expect(reducer.mock.calls).toEqual([ + [{}, testAction1('a')], + [{}, testAction1('c')] + ]) + }) +}) diff --git a/src/entities/createActionListenerMiddleware.ts b/src/entities/createActionListenerMiddleware.ts new file mode 100644 index 0000000000..ee80e17a87 --- /dev/null +++ b/src/entities/createActionListenerMiddleware.ts @@ -0,0 +1,268 @@ +import { Middleware, Dispatch, AnyAction, MiddlewareAPI, Action } from 'redux' +import { TypedActionCreator } from '../mapBuilders' +import { createAction, BaseActionCreator } from '../createAction' + +type ActionListener> = ( + action: A, + api: MiddlewareAPI +) => void +interface ActionListenerOptions< + A extends AnyAction, + S, + _ extends Dispatch +> { + /** + * Indicates that the listener should be removed after if has run once. + */ + once?: boolean + /** + * If set to true, the action will not be forwarded to + * * listeners that were registered after this listener + * * middlewares later in the middleware chain + * * reducers + * If this listener is skipped due to `options.condition`, this has no effect. + */ + preventPropagation?: boolean + /** + * A function that determines if the listener should run, depending on the action and probably the state. + */ + condition?(action: A, getState: () => S): boolean +} + +interface AddListenerAction< + A extends AnyAction, + S, + D extends Dispatch +> { + type: 'actionListenerMiddleware/add' + payload: { + type: string + } + meta: { + listener: ActionListener + options?: ActionListenerOptions + } +} + +export const addListenerAction = createAction( + 'actionListenerMiddleware/add', + function prepare( + typeOrActionCreator: string | TypedActionCreator, + listener: ActionListener, + options?: ActionListenerOptions + ) { + const type = + typeof typeOrActionCreator === 'string' + ? typeOrActionCreator + : (typeOrActionCreator as TypedActionCreator).type + + return { + payload: { + type + }, + meta: { + listener, + options + } + } + } +) as BaseActionCreator< + { type: string }, + 'actionListenerMiddleware/add', + { + listener: ActionListener + options: ActionListenerOptions + } +> & { + , S, D extends Dispatch>( + actionCreator: C, + listener: ActionListener, S, D>, + options?: ActionListenerOptions, S, D> + ): AddListenerAction, S, D> + + ( + type: string, + listener: ActionListener, + options?: ActionListenerOptions + ): AddListenerAction +} + +interface RemoveListenerAction< + A extends AnyAction, + S, + D extends Dispatch +> { + type: 'actionListenerMiddleware/remove' + payload: { + type: string + } + meta: { + listener: ActionListener + } +} + +export const removeListenerAction = createAction( + 'actionListenerMiddleware/remove', + function prepare( + typeOrActionCreator: string | TypedActionCreator, + listener: ActionListener + ) { + const type = + typeof typeOrActionCreator === 'string' + ? typeOrActionCreator + : (typeOrActionCreator as TypedActionCreator).type + + return { + payload: { + type + }, + meta: { + listener + } + } + } +) as BaseActionCreator< + { type: string }, + 'actionListenerMiddleware/remove', + { + listener: ActionListener + } +> & { + , S, D extends Dispatch>( + actionCreator: C, + listener: ActionListener, S, D> + ): RemoveListenerAction, S, D> + + ( + type: string, + listener: ActionListener + ): RemoveListenerAction +} + +export function createActionListenerMiddleware< + S, + D extends Dispatch = Dispatch +>() { + type ListenerEntry = ActionListenerOptions & { + listener: ActionListener + } + + const listenerMap: Record> = {} + const middleware: Middleware< + { + (action: Action<'actionListenerMiddleware/add'>): Unsubscribe + }, + S, + D + > = api => next => action => { + if (addListenerAction.match(action)) { + const unsubscribe = addListener( + action.payload.type, + action.meta.listener, + action.meta.options + ) + delete action.meta + next(action) + return unsubscribe + } + if (removeListenerAction.match(action)) { + removeListener(action.payload.type, action.meta.listener) + } + + if (listenerMap[action.type]) { + const listeners = listenerMap[action.type] + for (const entry of listeners) { + if (!entry.condition || entry.condition(action, api.getState)) { + entry.listener(action, api) + if (entry.once) { + listeners.delete(entry) + } + if (entry.preventPropagation) { + return action + } + } + } + } + return next(action) + } + + type Unsubscribe = () => void + + function addListener>( + actionCreator: C, + listener: ActionListener, S, D>, + options?: ActionListenerOptions, S, D> + ): Unsubscribe + function addListener( + type: string, + listener: ActionListener, + options?: ActionListenerOptions + ): Unsubscribe + function addListener( + typeOrActionCreator: string | TypedActionCreator, + listener: ActionListener, + options?: ActionListenerOptions + ): Unsubscribe { + const type = + typeof typeOrActionCreator === 'string' + ? typeOrActionCreator + : typeOrActionCreator.type + + if (!listenerMap[type]) { + listenerMap[type] = new Set() + } + + let entry = findListenerEntry(listenerMap[type], listener) + + if (!entry) { + entry = { + ...options, + listener + } + + listenerMap[type].add(entry) + } + + return () => listenerMap[type].delete(entry!) + } + + function removeListener>( + actionCreator: C, + listener: ActionListener, S, D> + ): boolean + function removeListener( + type: string, + listener: ActionListener + ): boolean + function removeListener( + typeOrActionCreator: string | TypedActionCreator, + listener: ActionListener + ): boolean { + const type = + typeof typeOrActionCreator === 'string' + ? typeOrActionCreator + : typeOrActionCreator.type + + let entry = findListenerEntry(listenerMap[type], listener) + + if (!entry) { + return false + } + + listenerMap[type].delete(entry!) + return true + } + + function findListenerEntry( + entries: Set, + listener: Function + ): ListenerEntry | undefined { + for (const entry of entries) { + if (entry.listener === listener) { + return entry + } + } + } + + return Object.assign(middleware, { addListener, removeListener }) +} From 4bed5f0518c17fb036733eda9e68dfd4a38c5966 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 9 May 2020 20:10:25 -0400 Subject: [PATCH 02/11] Move listener middleware into src --- src/{entities => }/createActionListenerMiddleware.test.ts | 4 ++-- src/{entities => }/createActionListenerMiddleware.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename src/{entities => }/createActionListenerMiddleware.test.ts (98%) rename src/{entities => }/createActionListenerMiddleware.ts (98%) diff --git a/src/entities/createActionListenerMiddleware.test.ts b/src/createActionListenerMiddleware.test.ts similarity index 98% rename from src/entities/createActionListenerMiddleware.test.ts rename to src/createActionListenerMiddleware.test.ts index b94a5ec58b..f90f9dcf02 100644 --- a/src/entities/createActionListenerMiddleware.test.ts +++ b/src/createActionListenerMiddleware.test.ts @@ -1,10 +1,10 @@ -import { configureStore } from '../configureStore' +import { configureStore } from './configureStore' import { createActionListenerMiddleware, addListenerAction, removeListenerAction } from './createActionListenerMiddleware' -import { createAction } from '../createAction' +import { createAction } from './createAction' const middlewareApi = { getState: expect.any(Function), diff --git a/src/entities/createActionListenerMiddleware.ts b/src/createActionListenerMiddleware.ts similarity index 98% rename from src/entities/createActionListenerMiddleware.ts rename to src/createActionListenerMiddleware.ts index ee80e17a87..d4f6e2f232 100644 --- a/src/entities/createActionListenerMiddleware.ts +++ b/src/createActionListenerMiddleware.ts @@ -1,6 +1,6 @@ import { Middleware, Dispatch, AnyAction, MiddlewareAPI, Action } from 'redux' -import { TypedActionCreator } from '../mapBuilders' -import { createAction, BaseActionCreator } from '../createAction' +import { TypedActionCreator } from './mapBuilders' +import { createAction, BaseActionCreator } from './createAction' type ActionListener> = ( action: A, From 463c395917188182c417613e3c009d6ec79c9d2d Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 9 May 2020 20:16:09 -0400 Subject: [PATCH 03/11] Export action listener middleware --- etc/redux-toolkit.api.md | 34 ++++++++++++++++++++++++++++++++++ src/index.ts | 6 ++++++ 2 files changed, 40 insertions(+) diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md index 401db6562e..83ece3fe80 100644 --- a/etc/redux-toolkit.api.md +++ b/etc/redux-toolkit.api.md @@ -13,6 +13,7 @@ import { DeepPartial } from 'redux'; import { Dispatch } from 'redux'; import { Draft } from 'immer'; import { Middleware } from 'redux'; +import { MiddlewareAPI } from 'redux'; import { OutputParametricSelector } from 'reselect'; import { OutputSelector } from 'reselect'; import { ParametricSelector } from 'reselect'; @@ -59,6 +60,17 @@ export interface ActionReducerMapBuilder { // @public @deprecated export type Actions = Record; +// @public (undocumented) +export const addListenerAction: BaseActionCreator<{ + type: string; +}, "actionListenerMiddleware/add", { + listener: ActionListener; + options: ActionListenerOptions; +}, never> & { + , S, D extends Dispatch>(actionCreator: C, listener: ActionListener, S, D>, options?: ActionListenerOptions, S, D> | undefined): AddListenerAction, S, D>; + >(type: string, listener: ActionListener, options?: ActionListenerOptions | undefined): AddListenerAction; +}; + // @public export type AsyncThunkAction = (dispatch: GetDispatch, getState: () => GetState, extra: GetExtra) => Promise(type: T): Payl // @public export function createAction, T extends string = string>(type: T, prepareAction: PA): PayloadActionCreator['payload'], T, PA>; +// @public (undocumented) +export function createActionListenerMiddleware = Dispatch>(): Middleware<(action: Action<"actionListenerMiddleware/add">) => () => void, S, D> & { + addListener: { + >(actionCreator: C, listener: ActionListener, S, D>, options?: ActionListenerOptions, S, D> | undefined): () => void; + (type: string, listener: ActionListener, options?: ActionListenerOptions | undefined): () => void; + }; + removeListener: { + >(actionCreator: C_1, listener: ActionListener, S, D>): boolean; + (type: string, listener: ActionListener): boolean; + }; +}; + // @public (undocumented) export function createAsyncThunk(typePrefix: string, payloadCreator: (arg: ThunkArg, thunkAPI: GetThunkAPI) => Promise>> | Returned | RejectWithValue>, options?: AsyncThunkOptions): IsAny AsyncThunkAction, unknown extends ThunkArg ? (arg: ThunkArg) => AsyncThunkAction : [ThunkArg] extends [void] | [undefined] ? () => AsyncThunkAction : [void] extends [ThunkArg] ? (arg?: ThunkArg | undefined) => AsyncThunkAction : [undefined] extends [ThunkArg] ? (arg?: ThunkArg | undefined) => AsyncThunkAction : (arg: ThunkArg) => AsyncThunkAction> & { pending: ActionCreatorWithPreparedPayload<[string, ThunkArg], undefined, string, never, { @@ -342,6 +366,16 @@ export type PrepareAction

= ((...args: any[]) => { error: any; }); +// @public (undocumented) +export const removeListenerAction: BaseActionCreator<{ + type: string; +}, "actionListenerMiddleware/remove", { + listener: ActionListener; +}, never> & { + , S, D extends Dispatch>(actionCreator: C, listener: ActionListener, S, D>): RemoveListenerAction, S, D>; + >(type: string, listener: ActionListener): RemoveListenerAction; +}; + export { Selector } // @public diff --git a/src/index.ts b/src/index.ts index f4a1843d92..3ebe6c6785 100644 --- a/src/index.ts +++ b/src/index.ts @@ -104,4 +104,10 @@ export { SerializedError } from './createAsyncThunk' +export { + createActionListenerMiddleware, + addListenerAction, + removeListenerAction +} from './createActionListenerMiddleware' + export { nanoid } from './nanoid' From 60a176927e480f3e860fc8f2e4fa2bc75c8bd444 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 9 May 2020 20:24:02 -0400 Subject: [PATCH 04/11] Mark middleware types as alpha --- etc/redux-toolkit.api.md | 6 +++--- src/createActionListenerMiddleware.ts | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md index 83ece3fe80..810454c25b 100644 --- a/etc/redux-toolkit.api.md +++ b/etc/redux-toolkit.api.md @@ -60,7 +60,7 @@ export interface ActionReducerMapBuilder { // @public @deprecated export type Actions = Record; -// @public (undocumented) +// @alpha (undocumented) export const addListenerAction: BaseActionCreator<{ type: string; }, "actionListenerMiddleware/add", { @@ -135,7 +135,7 @@ export function createAction

(type: T): Payl // @public export function createAction, T extends string = string>(type: T, prepareAction: PA): PayloadActionCreator['payload'], T, PA>; -// @public (undocumented) +// @alpha (undocumented) export function createActionListenerMiddleware = Dispatch>(): Middleware<(action: Action<"actionListenerMiddleware/add">) => () => void, S, D> & { addListener: { >(actionCreator: C, listener: ActionListener, S, D>, options?: ActionListenerOptions, S, D> | undefined): () => void; @@ -366,7 +366,7 @@ export type PrepareAction

= ((...args: any[]) => { error: any; }); -// @public (undocumented) +// @alpha (undocumented) export const removeListenerAction: BaseActionCreator<{ type: string; }, "actionListenerMiddleware/remove", { diff --git a/src/createActionListenerMiddleware.ts b/src/createActionListenerMiddleware.ts index d4f6e2f232..54ffab6a6b 100644 --- a/src/createActionListenerMiddleware.ts +++ b/src/createActionListenerMiddleware.ts @@ -44,6 +44,9 @@ interface AddListenerAction< } } +/** + * @alpha + */ export const addListenerAction = createAction( 'actionListenerMiddleware/add', function prepare( @@ -101,6 +104,9 @@ interface RemoveListenerAction< } } +/** + * @alpha + */ export const removeListenerAction = createAction( 'actionListenerMiddleware/remove', function prepare( @@ -139,6 +145,9 @@ export const removeListenerAction = createAction( ): RemoveListenerAction } +/** + * @alpha + */ export function createActionListenerMiddleware< S, D extends Dispatch = Dispatch From 0b2c3d4193d5004a0c74939cd0a3c645a1652ece Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sun, 10 May 2020 18:17:51 +0200 Subject: [PATCH 05/11] some suggestions & fixes: * move meta properties into payload * do not propagate addListenerAction and removeListenerAction * fix a bug when unsubscribing from a type that was never subscribed before --- etc/redux-toolkit.api.md | 6 +- src/createActionListenerMiddleware.test.ts | 27 +++++++++ src/createActionListenerMiddleware.ts | 69 +++++++++++----------- 3 files changed, 64 insertions(+), 38 deletions(-) diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md index 810454c25b..6abf7051b2 100644 --- a/etc/redux-toolkit.api.md +++ b/etc/redux-toolkit.api.md @@ -63,10 +63,9 @@ export type Actions = Record; // @alpha (undocumented) export const addListenerAction: BaseActionCreator<{ type: string; -}, "actionListenerMiddleware/add", { listener: ActionListener; options: ActionListenerOptions; -}, never> & { +}, "actionListenerMiddleware/add", never, never> & { , S, D extends Dispatch>(actionCreator: C, listener: ActionListener, S, D>, options?: ActionListenerOptions, S, D> | undefined): AddListenerAction, S, D>; >(type: string, listener: ActionListener, options?: ActionListenerOptions | undefined): AddListenerAction; }; @@ -369,9 +368,8 @@ export type PrepareAction

= ((...args: any[]) => { // @alpha (undocumented) export const removeListenerAction: BaseActionCreator<{ type: string; -}, "actionListenerMiddleware/remove", { listener: ActionListener; -}, never> & { +}, "actionListenerMiddleware/remove", never, never> & { , S, D extends Dispatch>(actionCreator: C, listener: ActionListener, S, D>): RemoveListenerAction, S, D>; >(type: string, listener: ActionListener): RemoveListenerAction; }; diff --git a/src/createActionListenerMiddleware.test.ts b/src/createActionListenerMiddleware.test.ts index f90f9dcf02..4e825fbc2c 100644 --- a/src/createActionListenerMiddleware.test.ts +++ b/src/createActionListenerMiddleware.test.ts @@ -5,12 +5,15 @@ import { removeListenerAction } from './createActionListenerMiddleware' import { createAction } from './createAction' +import { AnyAction } from 'redux' const middlewareApi = { getState: expect.any(Function), dispatch: expect.any(Function) } +const noop = () => {} + describe('createActionListenerMiddleware', () => { let store = configureStore({ reducer: () => ({}), @@ -95,6 +98,10 @@ describe('createActionListenerMiddleware', () => { expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]]) }) + test('unsubscribing without any subscriptions does not trigger an error', () => { + middleware.removeListener(testAction1, noop) + }) + test('subscribing via action', () => { const listener = jest.fn((_: TestAction1) => {}) @@ -138,6 +145,26 @@ describe('createActionListenerMiddleware', () => { expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]]) }) + const unforwaredActions: [string, AnyAction][] = [ + ['addListenerAction', addListenerAction(testAction1, noop)], + ['removeListenerAction', removeListenerAction(testAction1, noop)] + ] + test.each(unforwaredActions)( + '"%s" is not forwarded to the reducer', + (_, action) => { + reducer.mockClear() + + store.dispatch(testAction1('a')) + store.dispatch(action) + store.dispatch(testAction2('b')) + + expect(reducer.mock.calls).toEqual([ + [{}, testAction1('a')], + [{}, testAction2('b')] + ]) + } + ) + test('"condition" allows to skip the listener', () => { const listener = jest.fn((_: TestAction1) => {}) diff --git a/src/createActionListenerMiddleware.ts b/src/createActionListenerMiddleware.ts index 54ffab6a6b..27b8b34cca 100644 --- a/src/createActionListenerMiddleware.ts +++ b/src/createActionListenerMiddleware.ts @@ -37,8 +37,6 @@ interface AddListenerAction< type: 'actionListenerMiddleware/add' payload: { type: string - } - meta: { listener: ActionListener options?: ActionListenerOptions } @@ -61,21 +59,19 @@ export const addListenerAction = createAction( return { payload: { - type - }, - meta: { + type, listener, options } } } ) as BaseActionCreator< - { type: string }, - 'actionListenerMiddleware/add', { + type: string listener: ActionListener options: ActionListenerOptions - } + }, + 'actionListenerMiddleware/add' > & { , S, D extends Dispatch>( actionCreator: C, @@ -98,8 +94,6 @@ interface RemoveListenerAction< type: 'actionListenerMiddleware/remove' payload: { type: string - } - meta: { listener: ActionListener } } @@ -120,19 +114,14 @@ export const removeListenerAction = createAction( return { payload: { - type - }, - meta: { + type, listener } } } ) as BaseActionCreator< - { type: string }, - 'actionListenerMiddleware/remove', - { - listener: ActionListener - } + { type: string; listener: ActionListener }, + 'actionListenerMiddleware/remove' > & { , S, D extends Dispatch>( actionCreator: C, @@ -156,7 +145,7 @@ export function createActionListenerMiddleware< listener: ActionListener } - const listenerMap: Record> = {} + const listenerMap: Record | undefined> = {} const middleware: Middleware< { (action: Action<'actionListenerMiddleware/add'>): Unsubscribe @@ -167,19 +156,20 @@ export function createActionListenerMiddleware< if (addListenerAction.match(action)) { const unsubscribe = addListener( action.payload.type, - action.meta.listener, - action.meta.options + action.payload.listener, + action.payload.options ) - delete action.meta - next(action) + return unsubscribe } if (removeListenerAction.match(action)) { - removeListener(action.payload.type, action.meta.listener) + removeListener(action.payload.type, action.payload.listener) + + return } - if (listenerMap[action.type]) { - const listeners = listenerMap[action.type] + const listeners = listenerMap[action.type] + if (listeners) { for (const entry of listeners) { if (!entry.condition || entry.condition(action, api.getState)) { entry.listener(action, api) @@ -217,11 +207,9 @@ export function createActionListenerMiddleware< ? typeOrActionCreator : typeOrActionCreator.type - if (!listenerMap[type]) { - listenerMap[type] = new Set() - } + const listeners = getListenerMap(type) - let entry = findListenerEntry(listenerMap[type], listener) + let entry = findListenerEntry(listeners, listener) if (!entry) { entry = { @@ -229,10 +217,17 @@ export function createActionListenerMiddleware< listener } - listenerMap[type].add(entry) + listeners.add(entry) } - return () => listenerMap[type].delete(entry!) + return () => listeners.delete(entry!) + } + + function getListenerMap(type: string) { + if (!listenerMap[type]) { + listenerMap[type] = new Set() + } + return listenerMap[type]! } function removeListener>( @@ -252,13 +247,19 @@ export function createActionListenerMiddleware< ? typeOrActionCreator : typeOrActionCreator.type - let entry = findListenerEntry(listenerMap[type], listener) + const listeners = listenerMap[type] + + if (!listeners) { + return false + } + + let entry = findListenerEntry(listeners, listener) if (!entry) { return false } - listenerMap[type].delete(entry!) + listeners.delete(entry) return true } From 91b19297d27fc93ca498c772136fd71bc421851c Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sun, 10 May 2020 18:22:08 +0200 Subject: [PATCH 06/11] remove comment --- src/createActionListenerMiddleware.test.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/createActionListenerMiddleware.test.ts b/src/createActionListenerMiddleware.test.ts index 4e825fbc2c..22de0b4cfa 100644 --- a/src/createActionListenerMiddleware.test.ts +++ b/src/createActionListenerMiddleware.test.ts @@ -51,11 +51,6 @@ describe('createActionListenerMiddleware', () => { }) test('subscribing with the same listener will not make it trigger twice (like EventTarget.addEventListener())', () => { - /** - * thoughts: allow to use this to override the options for a listener? - * right now it's just exiting if the listener is already registered - */ - const listener = jest.fn((_: TestAction1) => {}) middleware.addListener(testAction1, listener) From e20bddbd06e7c376f39c11a847b94043790e1079 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sun, 10 May 2020 19:02:17 +0200 Subject: [PATCH 07/11] change `preventPropagation` option to `api.stopPropagation` method --- etc/redux-toolkit.api.md | 9 +++++ src/createActionListenerMiddleware.test.ts | 26 +++---------- src/createActionListenerMiddleware.ts | 43 ++++++++++++++-------- src/index.ts | 4 +- 4 files changed, 45 insertions(+), 37 deletions(-) diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md index 6abf7051b2..230417b021 100644 --- a/etc/redux-toolkit.api.md +++ b/etc/redux-toolkit.api.md @@ -51,6 +51,15 @@ export interface ActionCreatorWithPreparedPayload; } +// @alpha (undocumented) +export type ActionListener> = (action: A, api: ActionListenerMiddlewareAPI) => void; + +// @alpha (undocumented) +export interface ActionListenerMiddlewareAPI> extends MiddlewareAPI { + // (undocumented) + stopPropagation(): void; +} + // @public export interface ActionReducerMapBuilder { addCase>(actionCreator: ActionCreator, reducer: CaseReducer>): ActionReducerMapBuilder; diff --git a/src/createActionListenerMiddleware.test.ts b/src/createActionListenerMiddleware.test.ts index 22de0b4cfa..5665985334 100644 --- a/src/createActionListenerMiddleware.test.ts +++ b/src/createActionListenerMiddleware.test.ts @@ -9,7 +9,8 @@ import { AnyAction } from 'redux' const middlewareApi = { getState: expect.any(Function), - dispatch: expect.any(Function) + dispatch: expect.any(Function), + stopPropagation: expect.any(Function) } const noop = () => {} @@ -222,27 +223,12 @@ describe('createActionListenerMiddleware', () => { expect(reducer.mock.calls).toEqual([[{}, testAction1('a')]]) }) - test('"preventPropagation" prevents actions from being forwarded to the store', () => { + test('calling `api.stopPropagation` in the listeners prevents actions from being forwarded to the store', () => { reducer.mockClear() - const listener = jest.fn((_: TestAction1) => {}) - - middleware.addListener(testAction1, listener, { preventPropagation: true }) - - store.dispatch(testAction1('a')) - - expect(reducer.mock.calls).toEqual([]) - }) - - test('combining "preventPropagation" and "condition', () => { - reducer.mockClear() - - const listener = jest.fn((_: TestAction1) => {}) - - middleware.addListener(testAction1, listener, { - preventPropagation: true, - condition(action) { - return action.payload === 'b' + middleware.addListener(testAction1, (action: TestAction1, api) => { + if (action.payload === 'b') { + api.stopPropagation() } }) diff --git a/src/createActionListenerMiddleware.ts b/src/createActionListenerMiddleware.ts index 27b8b34cca..f2fd4ed383 100644 --- a/src/createActionListenerMiddleware.ts +++ b/src/createActionListenerMiddleware.ts @@ -2,11 +2,24 @@ import { Middleware, Dispatch, AnyAction, MiddlewareAPI, Action } from 'redux' import { TypedActionCreator } from './mapBuilders' import { createAction, BaseActionCreator } from './createAction' -type ActionListener> = ( - action: A, - api: MiddlewareAPI -) => void -interface ActionListenerOptions< +/** + * @alpha + */ +export interface ActionListenerMiddlewareAPI> + extends MiddlewareAPI { + stopPropagation(): void +} + +/** + * @alpha + */ +export type ActionListener< + A extends AnyAction, + S, + D extends Dispatch +> = (action: A, api: ActionListenerMiddlewareAPI) => void + +export interface ActionListenerOptions< A extends AnyAction, S, _ extends Dispatch @@ -15,21 +28,13 @@ interface ActionListenerOptions< * Indicates that the listener should be removed after if has run once. */ once?: boolean - /** - * If set to true, the action will not be forwarded to - * * listeners that were registered after this listener - * * middlewares later in the middleware chain - * * reducers - * If this listener is skipped due to `options.condition`, this has no effect. - */ - preventPropagation?: boolean /** * A function that determines if the listener should run, depending on the action and probably the state. */ condition?(action: A, getState: () => S): boolean } -interface AddListenerAction< +export interface AddListenerAction< A extends AnyAction, S, D extends Dispatch @@ -172,11 +177,17 @@ export function createActionListenerMiddleware< if (listeners) { for (const entry of listeners) { if (!entry.condition || entry.condition(action, api.getState)) { - entry.listener(action, api) + let preventPropagation = false + entry.listener(action, { + ...api, + stopPropagation() { + preventPropagation = true + } + }) if (entry.once) { listeners.delete(entry) } - if (entry.preventPropagation) { + if (preventPropagation) { return action } } diff --git a/src/index.ts b/src/index.ts index 3ebe6c6785..0cb8290ab9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -107,7 +107,9 @@ export { export { createActionListenerMiddleware, addListenerAction, - removeListenerAction + removeListenerAction, + ActionListener, + ActionListenerMiddlewareAPI } from './createActionListenerMiddleware' export { nanoid } from './nanoid' From 55fb77b48d9795c1529514da4279f81917b54011 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sun, 10 May 2020 20:05:43 +0200 Subject: [PATCH 08/11] introduce "when" option to specify if listener should run "before" or "after" reducer --- etc/redux-toolkit.api.md | 28 ++-- src/createActionListenerMiddleware.test.ts | 52 +++++++- src/createActionListenerMiddleware.ts | 143 ++++++++++++++------- 3 files changed, 161 insertions(+), 62 deletions(-) diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md index 230417b021..f43acdfc29 100644 --- a/etc/redux-toolkit.api.md +++ b/etc/redux-toolkit.api.md @@ -52,12 +52,12 @@ export interface ActionCreatorWithPreparedPayload> = (action: A, api: ActionListenerMiddlewareAPI) => void; +export type ActionListener, O extends ActionListenerOptions> = (action: A, api: ActionListenerMiddlewareAPI) => void; // @alpha (undocumented) -export interface ActionListenerMiddlewareAPI> extends MiddlewareAPI { +export interface ActionListenerMiddlewareAPI, O extends ActionListenerOptions> extends MiddlewareAPI { // (undocumented) - stopPropagation(): void; + stopPropagation: WhenFromOptions extends 'after' ? undefined : () => void; } // @public @@ -72,11 +72,11 @@ export type Actions = Record; // @alpha (undocumented) export const addListenerAction: BaseActionCreator<{ type: string; - listener: ActionListener; - options: ActionListenerOptions; + listener: ActionListener; + options: ActionListenerOptions; }, "actionListenerMiddleware/add", never, never> & { - , S, D extends Dispatch>(actionCreator: C, listener: ActionListener, S, D>, options?: ActionListenerOptions, S, D> | undefined): AddListenerAction, S, D>; - >(type: string, listener: ActionListener, options?: ActionListenerOptions | undefined): AddListenerAction; + , S, D extends Dispatch, O extends ActionListenerOptions, S, D, When>>(actionCreator: C, listener: ActionListener, S, D, O>, options?: O | undefined): AddListenerAction, S, D, O>; + , O_1 extends ActionListenerOptions>(type: string, listener: ActionListener, options?: O_1 | undefined): AddListenerAction; }; // @public @@ -146,12 +146,12 @@ export function createAction, T extends string = s // @alpha (undocumented) export function createActionListenerMiddleware = Dispatch>(): Middleware<(action: Action<"actionListenerMiddleware/add">) => () => void, S, D> & { addListener: { - >(actionCreator: C, listener: ActionListener, S, D>, options?: ActionListenerOptions, S, D> | undefined): () => void; - (type: string, listener: ActionListener, options?: ActionListenerOptions | undefined): () => void; + , O extends ActionListenerOptions, S, D, When>>(actionCreator: C, listener: ActionListener, S, D, O>, options?: O | undefined): () => void; + , S, D, When>>(type: T, listener: ActionListener, S, D, O_1>, options?: O_1 | undefined): () => void; }; removeListener: { - >(actionCreator: C_1, listener: ActionListener, S, D>): boolean; - (type: string, listener: ActionListener): boolean; + >(actionCreator: C_1, listener: ActionListener, S, D, any>): boolean; + (type: string, listener: ActionListener): boolean; }; }; @@ -377,10 +377,10 @@ export type PrepareAction

= ((...args: any[]) => { // @alpha (undocumented) export const removeListenerAction: BaseActionCreator<{ type: string; - listener: ActionListener; + listener: ActionListener; }, "actionListenerMiddleware/remove", never, never> & { - , S, D extends Dispatch>(actionCreator: C, listener: ActionListener, S, D>): RemoveListenerAction, S, D>; - >(type: string, listener: ActionListener): RemoveListenerAction; + , S, D extends Dispatch>(actionCreator: C, listener: ActionListener, S, D, any>): RemoveListenerAction, S, D>; + >(type: string, listener: ActionListener): RemoveListenerAction; }; export { Selector } diff --git a/src/createActionListenerMiddleware.test.ts b/src/createActionListenerMiddleware.test.ts index 5665985334..88841725b3 100644 --- a/src/createActionListenerMiddleware.test.ts +++ b/src/createActionListenerMiddleware.test.ts @@ -2,7 +2,8 @@ import { configureStore } from './configureStore' import { createActionListenerMiddleware, addListenerAction, - removeListenerAction + removeListenerAction, + When } from './createActionListenerMiddleware' import { createAction } from './createAction' import { AnyAction } from 'redux' @@ -211,6 +212,33 @@ describe('createActionListenerMiddleware', () => { expect(listener.mock.calls).toEqual([[testAction1('b'), middlewareApi]]) }) + const whenMap: [When, string, string][] = [ + [undefined, 'listener', 'reducer'], + ['before', 'listener', 'reducer'], + ['after', 'reducer', 'listener'] + ] + test.each(whenMap)( + 'with "when" set to %s, %s runs before %s', + (when, _, shouldRunLast) => { + let whoRanLast = '' + + reducer.mockClear() + reducer.mockImplementationOnce(() => { + whoRanLast = 'reducer' + }) + const listener = jest.fn(() => { + whoRanLast = 'listener' + }) + + middleware.addListener(testAction1, listener, when ? { when } : {}) + + store.dispatch(testAction1('a')) + expect(reducer).toHaveBeenCalledTimes(1) + expect(listener).toHaveBeenCalledTimes(1) + expect(whoRanLast).toBe(shouldRunLast) + } + ) + test('by default, actions are forwarded to the store', () => { reducer.mockClear() @@ -241,4 +269,26 @@ describe('createActionListenerMiddleware', () => { [{}, testAction1('c')] ]) }) + + test('calling `api.stopPropagation` with `when` set to "after" causes an error to be thrown', () => { + reducer.mockClear() + + middleware.addListener( + testAction1, + (action: TestAction1, api) => { + if (action.payload === 'b') { + // @ts-ignore TypeScript would already prevent this from being called with "after" + api.stopPropagation() + } + }, + { when: 'after' } + ) + + store.dispatch(testAction1('a')) + expect(() => { + store.dispatch(testAction1('b')) + }).toThrowErrorMatchingInlineSnapshot( + `"stopPropagation can only be called by action listeners with the \`when\` option set to \\"before\\""` + ) + }) }) diff --git a/src/createActionListenerMiddleware.ts b/src/createActionListenerMiddleware.ts index f2fd4ed383..441ff131fa 100644 --- a/src/createActionListenerMiddleware.ts +++ b/src/createActionListenerMiddleware.ts @@ -2,12 +2,20 @@ import { Middleware, Dispatch, AnyAction, MiddlewareAPI, Action } from 'redux' import { TypedActionCreator } from './mapBuilders' import { createAction, BaseActionCreator } from './createAction' +export type When = 'before' | 'after' | undefined +type WhenFromOptions< + O extends ActionListenerOptions +> = O extends ActionListenerOptions ? W : never + /** * @alpha */ -export interface ActionListenerMiddlewareAPI> - extends MiddlewareAPI { - stopPropagation(): void +export interface ActionListenerMiddlewareAPI< + S, + D extends Dispatch, + O extends ActionListenerOptions +> extends MiddlewareAPI { + stopPropagation: WhenFromOptions extends 'after' ? undefined : () => void } /** @@ -16,13 +24,15 @@ export interface ActionListenerMiddlewareAPI> export type ActionListener< A extends AnyAction, S, - D extends Dispatch -> = (action: A, api: ActionListenerMiddlewareAPI) => void + D extends Dispatch, + O extends ActionListenerOptions +> = (action: A, api: ActionListenerMiddlewareAPI) => void export interface ActionListenerOptions< A extends AnyAction, S, - _ extends Dispatch + _ extends Dispatch, + W extends When = 'before' > { /** * Indicates that the listener should be removed after if has run once. @@ -32,18 +42,25 @@ export interface ActionListenerOptions< * A function that determines if the listener should run, depending on the action and probably the state. */ condition?(action: A, getState: () => S): boolean + /** + * Determines if the listener runs 'before' or 'after' the reducers have been called. + * If set to 'before', calling `api.stopPropagation()` from the listener becomes possible. + * Defaults to 'before'. + */ + when?: W } export interface AddListenerAction< A extends AnyAction, S, - D extends Dispatch + D extends Dispatch, + O extends ActionListenerOptions > { type: 'actionListenerMiddleware/add' payload: { type: string - listener: ActionListener - options?: ActionListenerOptions + listener: ActionListener + options?: O } } @@ -54,7 +71,7 @@ export const addListenerAction = createAction( 'actionListenerMiddleware/add', function prepare( typeOrActionCreator: string | TypedActionCreator, - listener: ActionListener, + listener: ActionListener, options?: ActionListenerOptions ) { const type = @@ -73,22 +90,31 @@ export const addListenerAction = createAction( ) as BaseActionCreator< { type: string - listener: ActionListener + listener: ActionListener options: ActionListenerOptions }, 'actionListenerMiddleware/add' > & { - , S, D extends Dispatch>( + < + C extends TypedActionCreator, + S, + D extends Dispatch, + O extends ActionListenerOptions, S, D, When> + >( actionCreator: C, - listener: ActionListener, S, D>, - options?: ActionListenerOptions, S, D> - ): AddListenerAction, S, D> + listener: ActionListener, S, D, O>, + options?: O + ): AddListenerAction, S, D, O> - ( + < + S, + D extends Dispatch, + O extends ActionListenerOptions + >( type: string, - listener: ActionListener, - options?: ActionListenerOptions - ): AddListenerAction + listener: ActionListener, + options?: O + ): AddListenerAction } interface RemoveListenerAction< @@ -99,7 +125,7 @@ interface RemoveListenerAction< type: 'actionListenerMiddleware/remove' payload: { type: string - listener: ActionListener + listener: ActionListener } } @@ -110,7 +136,7 @@ export const removeListenerAction = createAction( 'actionListenerMiddleware/remove', function prepare( typeOrActionCreator: string | TypedActionCreator, - listener: ActionListener + listener: ActionListener ) { const type = typeof typeOrActionCreator === 'string' @@ -125,17 +151,17 @@ export const removeListenerAction = createAction( } } ) as BaseActionCreator< - { type: string; listener: ActionListener }, + { type: string; listener: ActionListener }, 'actionListenerMiddleware/remove' > & { , S, D extends Dispatch>( actionCreator: C, - listener: ActionListener, S, D> + listener: ActionListener, S, D, any> ): RemoveListenerAction, S, D> ( type: string, - listener: ActionListener + listener: ActionListener ): RemoveListenerAction } @@ -146,8 +172,8 @@ export function createActionListenerMiddleware< S, D extends Dispatch = Dispatch >() { - type ListenerEntry = ActionListenerOptions & { - listener: ActionListener + type ListenerEntry = ActionListenerOptions & { + listener: ActionListener } const listenerMap: Record | undefined> = {} @@ -177,18 +203,35 @@ export function createActionListenerMiddleware< if (listeners) { for (const entry of listeners) { if (!entry.condition || entry.condition(action, api.getState)) { - let preventPropagation = false - entry.listener(action, { - ...api, - stopPropagation() { - preventPropagation = true - } - }) if (entry.once) { listeners.delete(entry) } - if (preventPropagation) { - return action + + if (entry.when != 'after') { + /* before */ + let stoppedPropagation = false + entry.listener(action, { + ...api, + stopPropagation() { + stoppedPropagation = true + } + }) + if (stoppedPropagation) { + return action + } + return next(action) + } else { + /* after */ + const result = next(action) + entry.listener(action, { + ...api, + stopPropagation: () => { + throw new Error( + 'stopPropagation can only be called by action listeners with the `when` option set to "before"' + ) + } + }) + return result } } } @@ -198,20 +241,26 @@ export function createActionListenerMiddleware< type Unsubscribe = () => void - function addListener>( + function addListener< + C extends TypedActionCreator, + O extends ActionListenerOptions, S, D, When> + >( actionCreator: C, - listener: ActionListener, S, D>, - options?: ActionListenerOptions, S, D> + listener: ActionListener, S, D, O>, + options?: O ): Unsubscribe - function addListener( - type: string, - listener: ActionListener, - options?: ActionListenerOptions + function addListener< + T extends string, + O extends ActionListenerOptions, S, D, When> + >( + type: T, + listener: ActionListener, S, D, O>, + options?: O ): Unsubscribe function addListener( typeOrActionCreator: string | TypedActionCreator, - listener: ActionListener, - options?: ActionListenerOptions + listener: ActionListener, + options?: ActionListenerOptions ): Unsubscribe { const type = typeof typeOrActionCreator === 'string' @@ -243,15 +292,15 @@ export function createActionListenerMiddleware< function removeListener>( actionCreator: C, - listener: ActionListener, S, D> + listener: ActionListener, S, D, any> ): boolean function removeListener( type: string, - listener: ActionListener + listener: ActionListener ): boolean function removeListener( typeOrActionCreator: string | TypedActionCreator, - listener: ActionListener + listener: ActionListener ): boolean { const type = typeof typeOrActionCreator === 'string' From 0d580778e17d0fdb15d3c2bc865ef0b9aa1f6113 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sun, 10 May 2020 23:13:11 +0200 Subject: [PATCH 09/11] split "before" and "after" calls with condition check at the right time, fix erroneous `return` statement --- src/createActionListenerMiddleware.test.ts | 57 ++++++++++++++++++++ src/createActionListenerMiddleware.ts | 63 +++++++++++++--------- 2 files changed, 96 insertions(+), 24 deletions(-) diff --git a/src/createActionListenerMiddleware.test.ts b/src/createActionListenerMiddleware.test.ts index 88841725b3..a0ffd0fccd 100644 --- a/src/createActionListenerMiddleware.test.ts +++ b/src/createActionListenerMiddleware.test.ts @@ -239,6 +239,63 @@ describe('createActionListenerMiddleware', () => { } ) + test('mixing "before" and "after"', () => { + const calls: Function[] = [] + function before1() { + calls.push(before1) + } + function before2() { + calls.push(before2) + } + function after1() { + calls.push(after1) + } + function after2() { + calls.push(after2) + } + + middleware.addListener(testAction1, before1) + middleware.addListener(testAction1, before2, { when: 'before' }) + middleware.addListener(testAction1, after1, { when: 'after' }) + middleware.addListener(testAction1, after2, { when: 'after' }) + + store.dispatch(testAction1('a')) + store.dispatch(testAction2('a')) + + expect(calls).toEqual([before1, before2, after1, after2]) + }) + + test('mixing "before" and "after" with stopPropagation', () => { + const calls: Function[] = [] + function before1() { + calls.push(before1) + } + function before2(_: any, api: any) { + calls.push(before2) + api.stopPropagation() + } + function before3() { + calls.push(before3) + } + function after1() { + calls.push(after1) + } + function after2() { + calls.push(after2) + } + + middleware.addListener(testAction1, before1) + middleware.addListener(testAction1, before2, { when: 'before' }) + middleware.addListener(testAction1, before3, { when: 'before' }) + middleware.addListener(testAction1, after1, { when: 'after' }) + middleware.addListener(testAction1, after2, { when: 'after' }) + + store.dispatch(testAction1('a')) + store.dispatch(testAction2('a')) + + expect(calls).toEqual([before1, before2]) + }) + test('by default, actions are forwarded to the store', () => { reducer.mockClear() diff --git a/src/createActionListenerMiddleware.ts b/src/createActionListenerMiddleware.ts index 441ff131fa..05d7343d8d 100644 --- a/src/createActionListenerMiddleware.ts +++ b/src/createActionListenerMiddleware.ts @@ -201,40 +201,55 @@ export function createActionListenerMiddleware< const listeners = listenerMap[action.type] if (listeners) { + /* before */ for (const entry of listeners) { + if (entry.when == 'after') { + continue + } + if (!entry.condition || entry.condition(action, api.getState)) { if (entry.once) { listeners.delete(entry) } - if (entry.when != 'after') { - /* before */ - let stoppedPropagation = false - entry.listener(action, { - ...api, - stopPropagation() { - stoppedPropagation = true - } - }) - if (stoppedPropagation) { - return action + let stoppedPropagation = false + entry.listener(action, { + ...api, + stopPropagation() { + stoppedPropagation = true } - return next(action) - } else { - /* after */ - const result = next(action) - entry.listener(action, { - ...api, - stopPropagation: () => { - throw new Error( - 'stopPropagation can only be called by action listeners with the `when` option set to "before"' - ) - } - }) - return result + }) + if (stoppedPropagation) { + return action } } } + + const result = next(action) + + /* after */ + for (const entry of listeners) { + if (entry.when != 'after') { + continue + } + if (!entry.condition || entry.condition(action, api.getState)) { + if (entry.once) { + listeners.delete(entry) + } + + /* after */ + entry.listener(action, { + ...api, + stopPropagation: () => { + throw new Error( + 'stopPropagation can only be called by action listeners with the `when` option set to "before"' + ) + } + }) + } + } + + return result } return next(action) } From 6330ef99ce98849c075c83394ba990ef47ce9ed5 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sun, 10 May 2020 23:48:01 +0200 Subject: [PATCH 10/11] add DeclaredMiddlewareType helper type to specify middleware signature on extended function objects --- etc/redux-toolkit.api.md | 2 +- src/createActionListenerMiddleware.test.ts | 1 - src/createActionListenerMiddleware.ts | 7 ++++++- src/tsHelpers.ts | 19 ++++++++++++++++++- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md index f43acdfc29..003a0dd67c 100644 --- a/etc/redux-toolkit.api.md +++ b/etc/redux-toolkit.api.md @@ -153,7 +153,7 @@ export function createActionListenerMiddleware >(actionCreator: C_1, listener: ActionListener, S, D, any>): boolean; (type: string, listener: ActionListener): boolean; }; -}; +} & WithMiddlewareType) => () => void, S, D>>; // @public (undocumented) export function createAsyncThunk(typePrefix: string, payloadCreator: (arg: ThunkArg, thunkAPI: GetThunkAPI) => Promise>> | Returned | RejectWithValue>, options?: AsyncThunkOptions): IsAny AsyncThunkAction, unknown extends ThunkArg ? (arg: ThunkArg) => AsyncThunkAction : [ThunkArg] extends [void] | [undefined] ? () => AsyncThunkAction : [void] extends [ThunkArg] ? (arg?: ThunkArg | undefined) => AsyncThunkAction : [undefined] extends [ThunkArg] ? (arg?: ThunkArg | undefined) => AsyncThunkAction : (arg: ThunkArg) => AsyncThunkAction> & { diff --git a/src/createActionListenerMiddleware.test.ts b/src/createActionListenerMiddleware.test.ts index a0ffd0fccd..479690e18b 100644 --- a/src/createActionListenerMiddleware.test.ts +++ b/src/createActionListenerMiddleware.test.ts @@ -120,7 +120,6 @@ describe('createActionListenerMiddleware', () => { const unsubscribe = store.dispatch(addListenerAction(testAction1, listener)) store.dispatch(testAction1('a')) - // @ts-ignore TODO types unsubscribe() store.dispatch(testAction2('b')) store.dispatch(testAction1('c')) diff --git a/src/createActionListenerMiddleware.ts b/src/createActionListenerMiddleware.ts index 05d7343d8d..bdf9fd65bf 100644 --- a/src/createActionListenerMiddleware.ts +++ b/src/createActionListenerMiddleware.ts @@ -1,6 +1,7 @@ import { Middleware, Dispatch, AnyAction, MiddlewareAPI, Action } from 'redux' import { TypedActionCreator } from './mapBuilders' import { createAction, BaseActionCreator } from './createAction' +import { WithMiddlewareType } from './tsHelpers' export type When = 'before' | 'after' | undefined type WhenFromOptions< @@ -349,5 +350,9 @@ export function createActionListenerMiddleware< } } - return Object.assign(middleware, { addListener, removeListener }) + return Object.assign( + middleware, + { addListener, removeListener }, + {} as WithMiddlewareType + ) } diff --git a/src/tsHelpers.ts b/src/tsHelpers.ts index d624cd359b..5899565d98 100644 --- a/src/tsHelpers.ts +++ b/src/tsHelpers.ts @@ -65,6 +65,11 @@ export type IsUnknownOrNonInferrable = AtLeastTS35< IsEmptyObj> > +const declaredMiddlewareType: unique symbol = undefined as any +export type WithMiddlewareType> = { + [declaredMiddlewareType]: T +} + /** * Combines all dispatch signatures of all middlewares in the array `M` into * one intersected dispatch signature. @@ -72,7 +77,19 @@ export type IsUnknownOrNonInferrable = AtLeastTS35< export type DispatchForMiddlewares = M extends ReadonlyArray ? UnionToIntersection< M[number] extends infer MiddlewareValues - ? MiddlewareValues extends Middleware + ? MiddlewareValues extends WithMiddlewareType< + infer DeclaredMiddlewareType + > + ? DeclaredMiddlewareType extends Middleware< + infer DispatchExt, + any, + any + > + ? DispatchExt extends Function + ? DispatchExt + : never + : never + : MiddlewareValues extends Middleware ? DispatchExt extends Function ? DispatchExt : never From cddf92e8fe21c5c77f9284c5ab2daef7b6ff6472 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Mon, 11 May 2020 22:40:05 +0200 Subject: [PATCH 11/11] requested changes * default `when` to "after" * remove `once` and `condition` * add `unsubscribe` to api * consolidate "before" and "after" handling --- etc/redux-toolkit.api.md | 18 ++-- src/createActionListenerMiddleware.test.ts | 102 +++++++++--------- src/createActionListenerMiddleware.ts | 118 ++++++++------------- 3 files changed, 109 insertions(+), 129 deletions(-) diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md index 003a0dd67c..ff4d4e41ff 100644 --- a/etc/redux-toolkit.api.md +++ b/etc/redux-toolkit.api.md @@ -52,12 +52,14 @@ export interface ActionCreatorWithPreparedPayload, O extends ActionListenerOptions> = (action: A, api: ActionListenerMiddlewareAPI) => void; +export type ActionListener, O extends ActionListenerOptions> = (action: A, api: ActionListenerMiddlewareAPI) => void; // @alpha (undocumented) -export interface ActionListenerMiddlewareAPI, O extends ActionListenerOptions> extends MiddlewareAPI { +export interface ActionListenerMiddlewareAPI, O extends ActionListenerOptions> extends MiddlewareAPI { // (undocumented) - stopPropagation: WhenFromOptions extends 'after' ? undefined : () => void; + stopPropagation: WhenFromOptions extends 'before' ? () => void : undefined; + // (undocumented) + unsubscribe(): void; } // @public @@ -73,10 +75,10 @@ export type Actions = Record; export const addListenerAction: BaseActionCreator<{ type: string; listener: ActionListener; - options: ActionListenerOptions; + options: ActionListenerOptions; }, "actionListenerMiddleware/add", never, never> & { - , S, D extends Dispatch, O extends ActionListenerOptions, S, D, When>>(actionCreator: C, listener: ActionListener, S, D, O>, options?: O | undefined): AddListenerAction, S, D, O>; - , O_1 extends ActionListenerOptions>(type: string, listener: ActionListener, options?: O_1 | undefined): AddListenerAction; + , S, D extends Dispatch, O extends ActionListenerOptions>(actionCreator: C, listener: ActionListener, S, D, O>, options?: O | undefined): AddListenerAction, S, D, O>; + , O_1 extends ActionListenerOptions>(type: string, listener: ActionListener, options?: O_1 | undefined): AddListenerAction; }; // @public @@ -146,8 +148,8 @@ export function createAction, T extends string = s // @alpha (undocumented) export function createActionListenerMiddleware = Dispatch>(): Middleware<(action: Action<"actionListenerMiddleware/add">) => () => void, S, D> & { addListener: { - , O extends ActionListenerOptions, S, D, When>>(actionCreator: C, listener: ActionListener, S, D, O>, options?: O | undefined): () => void; - , S, D, When>>(type: T, listener: ActionListener, S, D, O_1>, options?: O_1 | undefined): () => void; + , O extends ActionListenerOptions>(actionCreator: C, listener: ActionListener, S, D, O>, options?: O | undefined): () => void; + (type: T, listener: ActionListener, S, D, O_1>, options?: O_1 | undefined): () => void; }; removeListener: { >(actionCreator: C_1, listener: ActionListener, S, D, any>): boolean; diff --git a/src/createActionListenerMiddleware.test.ts b/src/createActionListenerMiddleware.test.ts index 479690e18b..b3bf831db0 100644 --- a/src/createActionListenerMiddleware.test.ts +++ b/src/createActionListenerMiddleware.test.ts @@ -3,7 +3,8 @@ import { createActionListenerMiddleware, addListenerAction, removeListenerAction, - When + When, + ActionListenerMiddlewareAPI } from './createActionListenerMiddleware' import { createAction } from './createAction' import { AnyAction } from 'redux' @@ -11,7 +12,8 @@ import { AnyAction } from 'redux' const middlewareApi = { getState: expect.any(Function), dispatch: expect.any(Function), - stopPropagation: expect.any(Function) + stopPropagation: expect.any(Function), + unsubscribe: expect.any(Function) } const noop = () => {} @@ -161,14 +163,19 @@ describe('createActionListenerMiddleware', () => { } ) - test('"condition" allows to skip the listener', () => { - const listener = jest.fn((_: TestAction1) => {}) - - middleware.addListener(testAction1, listener, { - condition(action) { - return action.payload !== 'b' + test('"can unsubscribe via middleware api', () => { + const listener = jest.fn( + ( + action: TestAction1, + api: ActionListenerMiddlewareAPI + ) => { + if (action.payload === 'b') { + api.unsubscribe() + } } - }) + ) + + middleware.addListener(testAction1, listener) store.dispatch(testAction1('a')) store.dispatch(testAction1('b')) @@ -176,43 +183,12 @@ describe('createActionListenerMiddleware', () => { expect(listener.mock.calls).toEqual([ [testAction1('a'), middlewareApi], - [testAction1('c'), middlewareApi] + [testAction1('b'), middlewareApi] ]) }) - test('"once" unsubscribes the listener automatically after one use', () => { - const listener = jest.fn((_: TestAction1) => {}) - - middleware.addListener(testAction1, listener, { - once: true - }) - - store.dispatch(testAction1('a')) - store.dispatch(testAction1('b')) - store.dispatch(testAction1('c')) - - expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]]) - }) - - test('combining "once" with "condition', () => { - const listener = jest.fn((_: TestAction1) => {}) - - middleware.addListener(testAction1, listener, { - once: true, - condition(action) { - return action.payload === 'b' - } - }) - - store.dispatch(testAction1('a')) - store.dispatch(testAction1('b')) - store.dispatch(testAction1('c')) - - expect(listener.mock.calls).toEqual([[testAction1('b'), middlewareApi]]) - }) - const whenMap: [When, string, string][] = [ - [undefined, 'listener', 'reducer'], + [undefined, 'reducer', 'listener'], ['before', 'listener', 'reducer'], ['after', 'reducer', 'listener'] ] @@ -253,7 +229,7 @@ describe('createActionListenerMiddleware', () => { calls.push(after2) } - middleware.addListener(testAction1, before1) + middleware.addListener(testAction1, before1, { when: 'before' }) middleware.addListener(testAction1, before2, { when: 'before' }) middleware.addListener(testAction1, after1, { when: 'after' }) middleware.addListener(testAction1, after2, { when: 'after' }) @@ -283,7 +259,7 @@ describe('createActionListenerMiddleware', () => { calls.push(after2) } - middleware.addListener(testAction1, before1) + middleware.addListener(testAction1, before1, { when: 'before' }) middleware.addListener(testAction1, before2, { when: 'before' }) middleware.addListener(testAction1, before3, { when: 'before' }) middleware.addListener(testAction1, after1, { when: 'after' }) @@ -310,11 +286,15 @@ describe('createActionListenerMiddleware', () => { test('calling `api.stopPropagation` in the listeners prevents actions from being forwarded to the store', () => { reducer.mockClear() - middleware.addListener(testAction1, (action: TestAction1, api) => { - if (action.payload === 'b') { - api.stopPropagation() - } - }) + middleware.addListener( + testAction1, + (action: TestAction1, api) => { + if (action.payload === 'b') { + api.stopPropagation() + } + }, + { when: 'before' } + ) store.dispatch(testAction1('a')) store.dispatch(testAction1('b')) @@ -347,4 +327,28 @@ describe('createActionListenerMiddleware', () => { `"stopPropagation can only be called by action listeners with the \`when\` option set to \\"before\\""` ) }) + + test('calling `api.stopPropagation` asynchronously causes an error to be thrown', finish => { + reducer.mockClear() + + middleware.addListener( + testAction1, + (action: TestAction1, api) => { + if (action.payload === 'b') { + setTimeout(() => { + expect(() => { + api.stopPropagation() + }).toThrowErrorMatchingInlineSnapshot( + `"stopPropagation can only be called synchronously"` + ) + finish() + }) + } + }, + { when: 'before' } + ) + + store.dispatch(testAction1('a')) + store.dispatch(testAction1('b')) + }) }) diff --git a/src/createActionListenerMiddleware.ts b/src/createActionListenerMiddleware.ts index bdf9fd65bf..f554086337 100644 --- a/src/createActionListenerMiddleware.ts +++ b/src/createActionListenerMiddleware.ts @@ -5,8 +5,8 @@ import { WithMiddlewareType } from './tsHelpers' export type When = 'before' | 'after' | undefined type WhenFromOptions< - O extends ActionListenerOptions -> = O extends ActionListenerOptions ? W : never + O extends ActionListenerOptions +> = O extends ActionListenerOptions ? O['when'] : never /** * @alpha @@ -14,9 +14,10 @@ type WhenFromOptions< export interface ActionListenerMiddlewareAPI< S, D extends Dispatch, - O extends ActionListenerOptions + O extends ActionListenerOptions > extends MiddlewareAPI { - stopPropagation: WhenFromOptions extends 'after' ? undefined : () => void + stopPropagation: WhenFromOptions extends 'before' ? () => void : undefined + unsubscribe(): void } /** @@ -26,36 +27,23 @@ export type ActionListener< A extends AnyAction, S, D extends Dispatch, - O extends ActionListenerOptions + O extends ActionListenerOptions > = (action: A, api: ActionListenerMiddlewareAPI) => void -export interface ActionListenerOptions< - A extends AnyAction, - S, - _ extends Dispatch, - W extends When = 'before' -> { - /** - * Indicates that the listener should be removed after if has run once. - */ - once?: boolean - /** - * A function that determines if the listener should run, depending on the action and probably the state. - */ - condition?(action: A, getState: () => S): boolean +export interface ActionListenerOptions { /** * Determines if the listener runs 'before' or 'after' the reducers have been called. * If set to 'before', calling `api.stopPropagation()` from the listener becomes possible. * Defaults to 'before'. */ - when?: W + when?: When } export interface AddListenerAction< A extends AnyAction, S, D extends Dispatch, - O extends ActionListenerOptions + O extends ActionListenerOptions > { type: 'actionListenerMiddleware/add' payload: { @@ -73,7 +61,7 @@ export const addListenerAction = createAction( function prepare( typeOrActionCreator: string | TypedActionCreator, listener: ActionListener, - options?: ActionListenerOptions + options?: ActionListenerOptions ) { const type = typeof typeOrActionCreator === 'string' @@ -92,7 +80,7 @@ export const addListenerAction = createAction( { type: string listener: ActionListener - options: ActionListenerOptions + options: ActionListenerOptions }, 'actionListenerMiddleware/add' > & { @@ -100,18 +88,14 @@ export const addListenerAction = createAction( C extends TypedActionCreator, S, D extends Dispatch, - O extends ActionListenerOptions, S, D, When> + O extends ActionListenerOptions >( actionCreator: C, listener: ActionListener, S, D, O>, options?: O ): AddListenerAction, S, D, O> - < - S, - D extends Dispatch, - O extends ActionListenerOptions - >( + ( type: string, listener: ActionListener, options?: O @@ -173,7 +157,7 @@ export function createActionListenerMiddleware< S, D extends Dispatch = Dispatch >() { - type ListenerEntry = ActionListenerOptions & { + type ListenerEntry = ActionListenerOptions & { listener: ActionListener } @@ -202,55 +186,48 @@ export function createActionListenerMiddleware< const listeners = listenerMap[action.type] if (listeners) { - /* before */ - for (const entry of listeners) { - if (entry.when == 'after') { - continue - } - - if (!entry.condition || entry.condition(action, api.getState)) { - if (entry.once) { - listeners.delete(entry) + const defaultWhen = 'after' + let result: unknown + for (const phase of ['before', 'after'] as const) { + for (const entry of listeners) { + if (phase !== (entry.when || defaultWhen)) { + continue } - let stoppedPropagation = false + let currentPhase = phase + let synchronousListenerFinished = false entry.listener(action, { ...api, stopPropagation() { - stoppedPropagation = true + if (currentPhase === 'before') { + if (!synchronousListenerFinished) { + stoppedPropagation = true + } else { + throw new Error( + 'stopPropagation can only be called synchronously' + ) + } + } else { + throw new Error( + 'stopPropagation can only be called by action listeners with the `when` option set to "before"' + ) + } + }, + unsubscribe() { + listeners.delete(entry) } }) + synchronousListenerFinished = true if (stoppedPropagation) { return action } } - } - - const result = next(action) - - /* after */ - for (const entry of listeners) { - if (entry.when != 'after') { - continue - } - if (!entry.condition || entry.condition(action, api.getState)) { - if (entry.once) { - listeners.delete(entry) - } - - /* after */ - entry.listener(action, { - ...api, - stopPropagation: () => { - throw new Error( - 'stopPropagation can only be called by action listeners with the `when` option set to "before"' - ) - } - }) + if (phase === 'before') { + result = next(action) + } else { + return result } } - - return result } return next(action) } @@ -259,16 +236,13 @@ export function createActionListenerMiddleware< function addListener< C extends TypedActionCreator, - O extends ActionListenerOptions, S, D, When> + O extends ActionListenerOptions >( actionCreator: C, listener: ActionListener, S, D, O>, options?: O ): Unsubscribe - function addListener< - T extends string, - O extends ActionListenerOptions, S, D, When> - >( + function addListener( type: T, listener: ActionListener, S, D, O>, options?: O @@ -276,7 +250,7 @@ export function createActionListenerMiddleware< function addListener( typeOrActionCreator: string | TypedActionCreator, listener: ActionListener, - options?: ActionListenerOptions + options?: ActionListenerOptions ): Unsubscribe { const type = typeof typeOrActionCreator === 'string'