feat(alm): conform removeListener & removeListenerAction overloads to addListener

Why

It is now possible to remove entries created with
addListener({ predicate, listener })` or `addListener({ matcher, listener })`.

Discussion: #1648 (reply in thread)

Breaking changes

BREAKING CHANGE: Synchronously throw error, if listener is not a function
instead of throwing when the predicate matches the action disptched.

BREAKING CHANGE: removeListener(type, listener) is not longer a valid function signature

Test log

```bash
$ yarn workspace @rtk-incubator/action-listener-middleware run test --coverage
 PASS  src/tests/listenerMiddleware.test.ts
 PASS  src/tests/effectScenarios.test.ts
 PASS  src/tests/fork.test.ts
 PASS  src/tests/useCases.test.ts
---------------|---------|----------|---------|---------|-------------------
File           | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
---------------|---------|----------|---------|---------|-------------------
All files      |   97.37 |    94.23 |      94 |   97.16 |
 exceptions.ts |     100 |      100 |     100 |     100 |
 index.ts      |   97.16 |    97.37 |   91.43 |   97.06 | 178,202,226-227
 task.ts       |   97.06 |       80 |     100 |    96.3 | 31
 utils.ts      |     100 |      100 |     100 |     100 |
---------------|---------|----------|---------|---------|-------------------

Test Suites: 4 passed, 4 total
Tests:       66 passed, 66 total
Snapshots:   0 total
Time:        5.928 s
Ran all test suites.
```

Build log

```bash
$ yarn workspace @rtk-incubator/action-listener-middleware run build
Build "actionListenerMiddleware" to dist/esm:
       1596 B: index.modern.js.gz
       1439 B: index.modern.js.br
Build "actionListenerMiddleware" to dist/module:
      2.27 kB: index.js.gz
      2.02 kB: index.js.br
Build "actionListenerMiddleware" to dist/cjs:
      2.27 kB: index.js.gz
      2.02 kB: index.js.br
```

Author: FaberVitale

packages/action-listener-middleware/README.md

@@ -114,7 +114,7 @@ Current options are:
 
 - `onError`: an optional error handler that gets called with synchronous and async errors raised by `listener` and synchronous errors thrown by `predicate`.
 
-### `listenerMiddleware.addListener(options?: AddListenerOptions) : Unsubscribe`
+### `listenerMiddleware.addListener(options: AddListenerOptions) : Unsubscribe`
 
 Statically adds a new listener callback to the middleware.
 
@@ -166,6 +166,7 @@ middleware.addListener({
 })
 ```
 
+It throws error if listener is not a function.
 The ["matcher" utility functions included in RTK](https://redux-toolkit.js.org/api/matching-utilities) are acceptable as predicates.
 
 The return value is a standard `unsubscribe()` callback that will remove this listener. If you try to add a listener entry but another entry with this exact function reference already exists, no new entry will be added, and the existing `unsubscribe` method will be returned.
@@ -174,20 +175,33 @@ The `listener` callback will receive the current action as its first argument, a
 
 All listener predicates and callbacks are checked _after_ the root reducer has already processed the action and updated the state. The `listenerApi.getOriginalState()` method can be used to get the state value that existed before the action that triggered this listener was processed.
 
-### `listenerMiddleware.removeListener(typeOrActionCreator, listener)`
+### `listenerMiddleware.removeListener(options: AddListenerOptions): boolean`
 
-Removes a given listener. Accepts two arguments:
+Removes a given listener. Accepts the same arguments as `middleware.addListener()` and throws error if listener is not a function.
+Returns `true` if the `options.listener` listener has been removed, `false` if no subscription matching the input provided has been found.
 
-- `typeOrActionCreator: string | ActionCreator`: the same action type / action creator that was used to add the listener
-- `listener: ListenerCallback`: the same listener callback reference that was added originally
-
-Note that matcher-based listeners currently cannot be removed with this approach - you must use the `unsubscribe()` callback that was returned when adding the listener.
+```ts
+// 1) Action type string
+middleware.removeListener({ type: 'todos/todoAdded', listener })
+// 2) RTK action creator
+middleware.removeListener({ actionCreator: todoAdded, listener })
+// 3) RTK matcher function
+middleware.removeListener({ matcher: isAnyOf(todoAdded, todoToggled), listener })
+// 4) Listener predicate
+middleware.removeListener({
+  predicate: (action, currentState, previousState) => {
+    // return true when the listener should run
+  },
+  listener,
+})
+```
 
 ### `addListenerAction`
 
 A standard RTK action creator that tells the middleware to dynamically add a new listener at runtime. It accepts exactly the same options as `middleware.addListener()`
 
 Dispatching this action returns an `unsubscribe()` callback from `dispatch`.
+It throws error if listener is not a function.
 
 ```js
 // Per above, provide `predicate` or any of the other comparison options
@@ -197,9 +211,11 @@ const unsubscribe = store.dispatch(addListenerAction({ predicate, listener }))
 ### `removeListenerAction`
 
 A standard RTK action creator that tells the middleware to remove a listener at runtime. Accepts the same arguments as `middleware.removeListener()`:
+Returns `true` if the `options.listener` listener has been removed, `false` if no subscription matching the input provided has been found.
+It throws error if listener is not a function.
 
 ```js
-store.dispatch(removeListenerAction('todos/todoAdded', listener))
+store.dispatch(removeListenerAction({ predicate, listener }))
 ```
 
 ### `listenerApi`

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

@@ -9,18 +9,12 @@ import type {
 import { createAction, nanoid } from '@reduxjs/toolkit'
 
 import type {
-  ActionListener,
   AddListenerOverloads,
-  BaseActionCreator,
   AnyActionListenerPredicate,
   CreateListenerMiddlewareOptions,
-  ConditionFunction,
-  ListenerPredicate,
-  TypedActionCreator,
   TypedAddListener,
   TypedAddListenerAction,
   TypedCreateListenerEntry,
-  RemoveListenerAction,
   FallbackAddListenerOptions,
   ListenerEntry,
   ListenerErrorHandler,
@@ -30,6 +24,8 @@ import type {
   ListenerErrorInfo,
   ForkedTaskExecutor,
   ForkedTask,
+  TypedRemoveListenerAction,
+  TypedRemoveListener,
 } from './types'
 import { assertFunction, catchRejection } from './utils'
 import { TaskAbortError } from './exceptions'
@@ -50,6 +46,8 @@ export type {
   ListenerErrorHandler,
   TypedAddListener,
   TypedAddListenerAction,
+  TypedRemoveListener,
+  TypedRemoveListenerAction,
   Unsubscribe,
   ForkedTaskExecutor,
   ForkedTask,
@@ -164,10 +162,7 @@ const createTakePattern = <S>(
   ) => catchRejection(take(predicate, timeout))) as TakePattern<S>
 }
 
-/** Accepts the possible options for creating a listener, and returns a formatted listener entry */
-export const createListenerEntry: TypedCreateListenerEntry<unknown> = (
-  options: FallbackAddListenerOptions
-) => {
+const getListenerEntryPropsFrom = (options: FallbackAddListenerOptions) => {
   let { type, actionCreator, matcher, predicate, listener } = options
 
   if (type) {
@@ -181,10 +176,21 @@ export const createListenerEntry: TypedCreateListenerEntry<unknown> = (
     // pass
   } else {
     throw new Error(
-      'Creating a listener requires one of the known fields for matching an action'
+      'Creating or removing a listener requires one of the known fields for matching an action'
     )
   }
 
+  assertFunction(listener, 'options.listener');
+
+  return { predicate, type, listener }
+}
+
+/** Accepts the possible options for creating a listener, and returns a formatted listener entry */
+export const createListenerEntry: TypedCreateListenerEntry<unknown> = (
+  options: FallbackAddListenerOptions
+) => {
+  const { type, predicate, listener } = getListenerEntryPropsFrom(options)
+
   const id = nanoid()
   const entry: ListenerEntry<unknown> = {
     id,
@@ -227,54 +233,15 @@ const safelyNotifyError = (
  * @alpha
  */
 export const addListenerAction = createAction(
-  `${alm}/add`,
-  (options: unknown) => {
-    const entry = createListenerEntry(
-      // Fake out TS here
-      options as Parameters<AddListenerOverloads<unknown>>[0]
-    )
-
-    return {
-      payload: entry,
-    }
-  }
+  `${alm}/add`
 ) as TypedAddListenerAction<unknown>
 
 /**
  * @alpha
  */
 export const removeListenerAction = createAction(
-  `${alm}/remove`,
-  (
-    typeOrActionCreator: string | TypedActionCreator<string>,
-    listener: ActionListener<any, any, any>
-  ) => {
-    const type =
-      typeof typeOrActionCreator === 'string'
-        ? typeOrActionCreator
-        : (typeOrActionCreator as TypedActionCreator<string>).type
-
-    return {
-      payload: {
-        type,
-        listener,
-      },
-    }
-  }
-) as BaseActionCreator<
-  { type: string; listener: ActionListener<any, any, any> },
-  'actionListenerMiddleware/remove'
-> & {
-  <C extends TypedActionCreator<any>, S, D extends Dispatch>(
-    actionCreator: C,
-    listener: ActionListener<ReturnType<C>, S, D>
-  ): RemoveListenerAction<ReturnType<C>, S, D>
-
-  <S, D extends Dispatch>(
-    type: string,
-    listener: ActionListener<AnyAction, S, D>
-  ): RemoveListenerAction<AnyAction, S, D>
-}
+  `${alm}/remove`
+) as TypedRemoveListenerAction<unknown>
 
 const defaultErrorHandler: ListenerErrorHandler = (...args: unknown[]) => {
   console.error(`${alm}/error`, ...args)
@@ -313,7 +280,7 @@ export function createActionListenerMiddleware<
     return undefined
   }
 
-  const addListener = ((options: FallbackAddListenerOptions) => {
+  const addListener = (options: FallbackAddListenerOptions) => {
     let entry = findListenerEntry(
       (existingEntry) => existingEntry.listener === options.listener
     )
@@ -323,39 +290,21 @@ export function createActionListenerMiddleware<
     }
 
     return insertEntry(entry)
-  }) as TypedAddListener<S, D>
-
-  type RemoveListener = {
-    <C extends TypedActionCreator<any>>(
-      actionCreator: C,
-      listener: ActionListener<ReturnType<C>, S, D>
-    ): boolean
-    (type: string, listener: ActionListener<AnyAction, S, D>): boolean
-    (
-      typeOrActionCreator: string | TypedActionCreator<any>,
-      listener: ActionListener<AnyAction, S, D>
-    ): boolean
   }
 
-  const removeListener: RemoveListener = (
-    typeOrActionCreator: string | TypedActionCreator<any>,
-    listener: ActionListener<AnyAction, S, D>
-  ): boolean => {
-    const type =
-      typeof typeOrActionCreator === 'string'
-        ? typeOrActionCreator
-        : typeOrActionCreator.type
+  const removeListener = (options: FallbackAddListenerOptions): boolean => {
+    const { type, listener, predicate } = getListenerEntryPropsFrom(options)
 
-    let entry = findListenerEntry(
-      (entry) => entry.type === type && entry.listener === listener
-    )
+    const entry = findListenerEntry((entry) => {
+      const matchPredicateOrType =
+        typeof type === 'string'
+          ? entry.type === type
+          : entry.predicate === predicate
 
-    if (!entry) {
-      return false
-    }
+      return matchPredicateOrType && entry.listener === listener
+    })?.unsubscribe()
 
-    listenerMap.delete(entry.id)
-    return true
+    return !!entry
   }
 
   const notifyListener = async (
@@ -420,19 +369,10 @@ export function createActionListenerMiddleware<
     D
   > = (api) => (next) => (action) => {
     if (addListenerAction.match(action)) {
-      let entry = findListenerEntry(
-        (existingEntry) => existingEntry.listener === action.payload.listener
-      )
-
-      if (!entry) {
-        entry = action.payload
-      }
-
-      return insertEntry(entry)
+      return addListener(action.payload)
     }
     if (removeListenerAction.match(action)) {
-      removeListener(action.payload.type, action.payload.listener)
-      return
+      return removeListener(action.payload)
     }
 
     // Need to get this state _before_ the reducer processes the action
@@ -489,9 +429,10 @@ export function createActionListenerMiddleware<
   return assign(
     middleware,
     {
-      addListener,
-      removeListener,
+      addListener: addListener as TypedAddListener<S, D>,
+      removeListener: removeListener as TypedRemoveListener<S, D>,
       addListenerAction: addListenerAction as TypedAddListenerAction<S>,
+      removeListenerAction: removeListenerAction as TypedRemoveListenerAction<S>,
     },
     {} as WithMiddlewareType<typeof middleware>
   )

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

@@ -23,6 +23,10 @@ import type {
   Unsubscribe,
   ActionListenerMiddleware,
 } from '../index'
+import type {
+  ActionListener,
+  AddListenerOverloads,
+} from '../types'
 
 const middlewareApi = {
   getState: expect.any(Function),
@@ -222,7 +226,7 @@ describe('createActionListenerMiddleware', () => {
       store.dispatch(testAction2('b'))
       expect(listener.mock.calls).toEqual([[testAction2('b'), middlewareApi]])
 
-      store.dispatch(removeListenerAction(testAction2.type, listener))
+      store.dispatch(removeListenerAction({ type: testAction2.type, listener }))
 
       store.dispatch(testAction2('b'))
       expect(listener.mock.calls).toEqual([[testAction2('b'), middlewareApi]])
@@ -343,15 +347,15 @@ describe('createActionListenerMiddleware', () => {
 
       store.dispatch(testAction1('a'))
 
-      middleware.removeListener(testAction1, listener)
+      middleware.removeListener({ actionCreator: testAction1, listener })
       store.dispatch(testAction2('b'))
       store.dispatch(testAction1('c'))
 
       expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]])
     })
 
     test('unsubscribing without any subscriptions does not trigger an error', () => {
-      middleware.removeListener(testAction1, noop)
+      middleware.removeListener({ matcher: testAction1.match, listener: noop })
     })
 
     test('subscribing via action', () => {
@@ -405,21 +409,65 @@ describe('createActionListenerMiddleware', () => {
         listener,
       })
 
+      middleware.addListener({
+        actionCreator: testAction1,
+        listener,
+      })
+
       store.dispatch(testAction1('a'))
 
-      store.dispatch(removeListenerAction(testAction1, listener))
+      store.dispatch(
+        removeListenerAction({ actionCreator: testAction1, listener })
+      )
       store.dispatch(testAction2('b'))
       store.dispatch(testAction1('c'))
 
       expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]])
     })
 
+    const addListenerOptions: [
+      string,
+      Omit<
+        AddListenerOverloads<() => void, typeof store.getState, typeof store.dispatch>,
+        'listener'
+      >
+    ][] = [
+      ['predicate', { predicate: () => true }],
+      ['actionCreator', { actionCreator: testAction1 }],
+      ['matcher', { matcher: isAnyOf(testAction1, testAction2) }],
+      ['type', { type: testAction1.type }],
+    ]
+
+    test.each(addListenerOptions)(
+      'add and remove listener with "%s" param correctly',
+      (_, params) => {
+        const listener: ActionListener<
+          AnyAction,
+          typeof store.getState,
+          typeof store.dispatch
+        > = jest.fn()
+
+        middleware.addListener({ ...params, listener } as any)
+
+        store.dispatch(testAction1('a'))
+        expect(listener).toBeCalledTimes(1)
+
+        middleware.removeListener({ ...params, listener } as any)
+
+        store.dispatch(testAction1('b'))
+        expect(listener).toBeCalledTimes(1)
+      }
+    )
+
     const unforwardedActions: [string, AnyAction][] = [
       [
         'addListenerAction',
         addListenerAction({ actionCreator: testAction1, listener: noop }),
       ],
-      ['removeListenerAction', removeListenerAction(testAction1, noop)],
+      [
+        'removeListenerAction',
+        removeListenerAction({ actionCreator: testAction1, listener: noop }),
+      ],
     ]
     test.each(unforwardedActions)(
       '"%s" is not forwarded to the reducer',