Remove stopPropagation, clarify phase names, add "both" option

Author: markerikson

packages/action-listener-middleware/src/index.ts

@@ -45,7 +45,9 @@ export type WithMiddlewareType<T extends Middleware<any, any, any>> = {
   [declaredMiddlewareType]: T
 }
 
-export type When = 'before' | 'after' | undefined
+export type MiddlewarePhase = 'beforeReducer' | 'afterReducer'
+
+export type When = MiddlewarePhase | 'both' | undefined
 type WhenFromOptions<O extends ActionListenerOptions> =
   O extends ActionListenerOptions ? O['when'] : never
 
@@ -57,8 +59,8 @@ export interface ActionListenerMiddlewareAPI<
   D extends Dispatch<AnyAction>,
   O extends ActionListenerOptions
 > extends MiddlewareAPI<D, S> {
-  stopPropagation: WhenFromOptions<O> extends 'before' ? () => void : undefined
   unsubscribe(): void
+  currentPhase: MiddlewarePhase
 }
 
 /**
@@ -191,6 +193,9 @@ export const removeListenerAction = createAction(
   ): RemoveListenerAction<AnyAction, S, D>
 }
 
+const defaultWhen: MiddlewarePhase = 'afterReducer'
+const actualMiddlewarePhases = ['beforeReducer', 'afterReducer'] as const
+
 /**
  * @alpha
  */
@@ -233,46 +238,24 @@ export function createActionListenerMiddleware<
 
     let stateBefore = api.getState()
 
-    const defaultWhen: When = 'after'
     let result: unknown
-    for (const phase of ['before', 'after'] as const) {
+    for (const currentPhase of actualMiddlewarePhases) {
       let stateNow = api.getState()
       for (let entry of listenerMap.values()) {
-        if (
-          (entry.when || defaultWhen) !== phase ||
-          !entry.predicate(action, stateNow)
-        ) {
+        const runThisPhase =
+          entry.when === 'both' || entry.when === currentPhase
+        const runListener = runThisPhase && entry.predicate(action, stateNow)
+        if (!runListener) {
           continue
         }
 
-        let stoppedPropagation = false
-        let currentPhase = phase
-        let synchronousListenerFinished = false
         entry.listener(action, {
           ...api,
-          stopPropagation() {
-            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"'
-              )
-            }
-          },
+          currentPhase,
           unsubscribe: entry.unsubscribe,
         })
-        synchronousListenerFinished = true
-        if (stoppedPropagation) {
-          return action
-        }
       }
-      if (phase === 'before') {
+      if (currentPhase === 'beforeReducer') {
         result = next(action)
       } else {
         return result
@@ -345,6 +328,7 @@ export function createActionListenerMiddleware<
       const id = nanoid()
       const unsubscribe = () => listenerMap.delete(id)
       entry = {
+        when: defaultWhen,
         ...options,
         id,
         listener,

packages/action-listener-middleware/src/tests/listenerMiddleware.test.ts

@@ -15,7 +15,7 @@ import {
 const middlewareApi = {
   getState: expect.any(Function),
   dispatch: expect.any(Function),
-  stopPropagation: expect.any(Function),
+  currentPhase: expect.stringMatching(/beforeReducer|afterReducer/),
   unsubscribe: expect.any(Function),
 }
 
@@ -233,14 +233,15 @@ describe('createActionListenerMiddleware', () => {
     ])
   })
 
-  const whenMap: [When, string, string][] = [
-    [undefined, 'reducer', 'listener'],
-    ['before', 'listener', 'reducer'],
-    ['after', 'reducer', 'listener'],
+  const whenMap: [When, string, string, number][] = [
+    [undefined, 'reducer', 'listener', 1],
+    ['beforeReducer', 'listener', 'reducer', 1],
+    ['afterReducer', 'reducer', 'listener', 1],
+    ['both', 'reducer', 'listener', 2],
   ]
   test.each(whenMap)(
     'with "when" set to %s, %s runs before %s',
-    (when, _, shouldRunLast) => {
+    (when, _, shouldRunLast, listenerCalls) => {
       let whoRanLast = ''
 
       reducer.mockClear()
@@ -255,7 +256,7 @@ describe('createActionListenerMiddleware', () => {
 
       store.dispatch(testAction1('a'))
       expect(reducer).toHaveBeenCalledTimes(1)
-      expect(listener).toHaveBeenCalledTimes(1)
+      expect(listener).toHaveBeenCalledTimes(listenerCalls)
       expect(whoRanLast).toBe(shouldRunLast)
     }
   )
@@ -275,48 +276,17 @@ describe('createActionListenerMiddleware', () => {
       calls.push(after2)
     }
 
-    middleware.addListener(testAction1, before1, { when: 'before' })
-    middleware.addListener(testAction1, before2, { when: 'before' })
-    middleware.addListener(testAction1, after1, { when: 'after' })
-    middleware.addListener(testAction1, after2, { when: 'after' })
+    middleware.addListener(testAction1, before1, { when: 'beforeReducer' })
+    middleware.addListener(testAction1, before2, { when: 'beforeReducer' })
+    middleware.addListener(testAction1, after1, { when: 'afterReducer' })
+    middleware.addListener(testAction1, after2, { when: 'afterReducer' })
 
     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, { when: 'before' })
-    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()
 
@@ -328,73 +298,4 @@ describe('createActionListenerMiddleware', () => {
 
     expect(reducer.mock.calls).toEqual([[{}, testAction1('a')]])
   })
-
-  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()
-        }
-      },
-      { when: 'before' }
-    )
-
-    store.dispatch(testAction1('a'))
-    store.dispatch(testAction1('b'))
-    store.dispatch(testAction1('c'))
-
-    expect(reducer.mock.calls).toEqual([
-      [{}, testAction1('a')],
-      [{}, 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\\""`
-    )
-  })
-
-  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'))
-  })
 })