Merge pull request #1973 from FaberVitale/feat/alm-change-remove-listener-signature

Author: markerikson

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,14 +175,26 @@ 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`
 
@@ -197,9 +210,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.
+
 
 ```js
-store.dispatch(removeListenerAction('todos/todoAdded', listener))
+store.dispatch(removeListenerAction({ predicate, listener }))
 ```
 
 ### `listenerApi`

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

@@ -9,16 +9,14 @@ import type {
 import { createAction, nanoid } from '@reduxjs/toolkit'
 
 import type {
-  ActionListener,
+  ActionListenerMiddleware,
   AddListenerOverloads,
-  BaseActionCreator,
   AnyActionListenerPredicate,
   CreateListenerMiddlewareOptions,
   TypedActionCreator,
   TypedAddListener,
   TypedAddListenerAction,
   TypedCreateListenerEntry,
-  RemoveListenerAction,
   FallbackAddListenerOptions,
   ListenerEntry,
   ListenerErrorHandler,
@@ -28,6 +26,8 @@ import type {
   ListenerErrorInfo,
   ForkedTaskExecutor,
   ForkedTask,
+  TypedRemoveListenerAction,
+  TypedRemoveListener,
 } from './types'
 import { assertFunction, catchRejection } from './utils'
 import { TaskAbortError } from './exceptions'
@@ -48,6 +48,8 @@ export type {
   ListenerErrorHandler,
   TypedAddListener,
   TypedAddListenerAction,
+  TypedRemoveListener,
+  TypedRemoveListenerAction,
   Unsubscribe,
   ForkedTaskExecutor,
   ForkedTask,
@@ -162,10 +164,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) {
@@ -179,10 +178,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,
@@ -239,17 +249,7 @@ 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>
 
 /**
@@ -261,37 +261,8 @@ export const clearListenerMiddlewareAction = createAction(`${alm}/clear`)
  * @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)
@@ -330,7 +301,7 @@ export function createActionListenerMiddleware<
     return undefined
   }
 
-  const addListener = ((options: FallbackAddListenerOptions) => {
+  const addListener = (options: FallbackAddListenerOptions) => {
     let entry = findListenerEntry(
       (existingEntry) => existingEntry.listener === options.listener
     )
@@ -340,39 +311,23 @@ 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
+    })
 
-    listenerMap.delete(entry.id)
-    return true
+    entry?.unsubscribe()
+
+    return !!entry
   }
 
   const notifyListener = async (
@@ -439,15 +394,7 @@ 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 (clearListenerMiddlewareAction.match(action)) {
@@ -456,8 +403,7 @@ export function createActionListenerMiddleware<
     }
 
     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
@@ -514,10 +460,9 @@ export function createActionListenerMiddleware<
   return assign(
     middleware,
     {
-      addListener,
-      removeListener,
-      clear: clearListenerMiddleware,
-      addListenerAction: addListenerAction as TypedAddListenerAction<S>,
+      addListener: addListener as TypedAddListener<S, D>,
+      removeListener: removeListener as TypedRemoveListener<S, D>,
+      clearListeners: clearListenerMiddleware,
     },
     {} as WithMiddlewareType<typeof middleware>
   )