Rewrite gDM type inference for better extraction of dispatch

`getDefaultMiddleware` defines an initial type that may or may not
include `thunk`, depending on options. From there, `EnhancedStore`
tries to extract any extensions to `dispatch` from "the type of all
middleware as an array", to fully infer the correct type of
`store.dispatch`.

This was flaky. In particular, the `MiddlewareArray` type ended up
with "a union of all middleware" type, and that meant the extensions
weren't always getting read correctly.

This commit rewrites the inference to be more correct:

- `MiddlewareArray` now tracks an exact tuple type for its contents
- `.concat/prepend` now update that middleware tuple type
- We exactly extract the dispatch extensions from the tuple,
  and retain the exact order of the middleware as declared
- All the extensions are intersected together with `Dispatch`

This should now correctly represent cases like the listener
middleware returning an `Unsubscribe` callback in response to
`dispatch(addListenerAction())`

This commit also drops support for passing `null` as the `extraArg`
generic for `ThunkAction`, as that is an outdated pattern and
we want to discourage it.

Author: markerikson

packages/toolkit/src/configureStore.ts

@@ -20,7 +20,7 @@ import type {
   CurriedGetDefaultMiddleware,
 } from './getDefaultMiddleware'
 import { curryGetDefaultMiddleware } from './getDefaultMiddleware'
-import type { DispatchForMiddlewares, NoInfer } from './tsHelpers'
+import type { NoInfer, ExtractDispatchExtensions } from './tsHelpers'
 
 const IS_PRODUCTION = process.env.NODE_ENV === 'production'
 
@@ -110,7 +110,7 @@ export interface EnhancedStore<
    *
    * @inheritdoc
    */
-  dispatch: Dispatch<A> & DispatchForMiddlewares<M>
+  dispatch: ExtractDispatchExtensions<M> & Dispatch<A>
 }
 
 /**
@@ -160,7 +160,7 @@ export function configureStore<
   }
   if (
     !IS_PRODUCTION &&
-    finalMiddleware.some((item) => typeof item !== 'function')
+    finalMiddleware.some((item: any) => typeof item !== 'function')
   ) {
     throw new Error(
       'each middleware provided to configureStore must be a function'

packages/toolkit/src/getDefaultMiddleware.ts

@@ -8,6 +8,7 @@ import { createImmutableStateInvariantMiddleware } from './immutableStateInvaria
 
 import type { SerializableStateInvariantMiddlewareOptions } from './serializableStateInvariantMiddleware'
 import { createSerializableStateInvariantMiddleware } from './serializableStateInvariantMiddleware'
+import type { ExcludeFromTuple } from './tsHelpers'
 import { MiddlewareArray } from './utils'
 
 function isBoolean(x: any): x is boolean {
@@ -33,9 +34,7 @@ export type ThunkMiddlewareFor<
   ? never
   : O extends { thunk: { extraArgument: infer E } }
   ? ThunkMiddleware<S, AnyAction, E>
-  :
-      | ThunkMiddleware<S, AnyAction, null> //The ThunkMiddleware with a `null` ExtraArgument is here to provide backwards-compatibility.
-      | ThunkMiddleware<S, AnyAction>
+  : ThunkMiddleware<S, AnyAction>
 
 export type CurriedGetDefaultMiddleware<S = any> = <
   O extends Partial<GetDefaultMiddlewareOptions> = {
@@ -45,7 +44,7 @@ export type CurriedGetDefaultMiddleware<S = any> = <
   }
 >(
   options?: O
-) => MiddlewareArray<Middleware<{}, S> | ThunkMiddlewareFor<S, O>>
+) => MiddlewareArray<ExcludeFromTuple<[ThunkMiddlewareFor<S, O>], never>>
 
 export function curryGetDefaultMiddleware<
   S = any
@@ -76,14 +75,14 @@ export function getDefaultMiddleware<
   }
 >(
   options: O = {} as O
-): MiddlewareArray<Middleware<{}, S> | ThunkMiddlewareFor<S, O>> {
+): MiddlewareArray<ExcludeFromTuple<[ThunkMiddlewareFor<S, O>], never>> {
   const {
     thunk = true,
     immutableCheck = true,
     serializableCheck = true,
   } = options
 
-  let middlewareArray: Middleware<{}, S>[] = new MiddlewareArray()
+  let middlewareArray = new MiddlewareArray<Middleware[]>()
 
   if (thunk) {
     if (isBoolean(thunk)) {

packages/toolkit/src/tests/MiddlewareArray.typetest.ts

@@ -1,6 +1,8 @@
 import { getDefaultMiddleware } from '@reduxjs/toolkit'
 import type { Middleware } from 'redux'
-import type { DispatchForMiddlewares } from '@internal/tsHelpers'
+import type { ExtractDispatchExtensions } from '@internal/tsHelpers'
+import thunk from 'redux-thunk'
+import { MiddlewareArray } from '../utils'
 
 declare const expectType: <T>(t: T) => T
 
@@ -14,7 +16,7 @@ declare const middleware2: Middleware<{
 
 declare const getDispatch: <M extends Array<Middleware>>(
   m: M
-) => DispatchForMiddlewares<M>
+) => ExtractDispatchExtensions<M>
 
 type ThunkReturn = Promise<'thunk'>
 declare const thunkCreator: () => () => ThunkReturn

packages/toolkit/src/tests/configureStore.typetest.ts

@@ -1,15 +1,23 @@
 /* eslint-disable no-lone-blocks */
-import type { Dispatch, AnyAction, Middleware, Reducer, Store } from 'redux'
+import type {
+  Dispatch,
+  AnyAction,
+  Middleware,
+  Reducer,
+  Store,
+  Action,
+} from 'redux'
 import { applyMiddleware } from 'redux'
-import type { PayloadAction } from '@reduxjs/toolkit'
+import type { PayloadAction, MiddlewareArray } from '@reduxjs/toolkit'
 import {
   configureStore,
   getDefaultMiddleware,
   createSlice,
 } from '@reduxjs/toolkit'
-import type { ThunkMiddleware, ThunkAction } from 'redux-thunk'
-import thunk, { ThunkDispatch } from 'redux-thunk'
+import type { ThunkMiddleware, ThunkAction, ThunkDispatch } from 'redux-thunk'
+import thunk from 'redux-thunk'
 import { expectNotAny, expectType } from './helpers'
+import type { IsAny, ExtractDispatchExtensions } from '../tsHelpers'
 
 const _anyMiddleware: any = () => () => () => {}
 
@@ -300,14 +308,16 @@ const _anyMiddleware: any = () => () => () => {}
    * Test: multiple custom middleware
    */
   {
+    const middleware = [] as any as [
+      Middleware<(a: 'a') => 'A', StateA>,
+      Middleware<(b: 'b') => 'B', StateA>,
+      ThunkMiddleware<StateA>
+    ]
     const store = configureStore({
       reducer: reducerA,
-      middleware: [] as any as [
-        Middleware<(a: 'a') => 'A', StateA>,
-        Middleware<(b: 'b') => 'B', StateA>,
-        ThunkMiddleware<StateA>
-      ],
+      middleware,
     })
+
     const result: 'A' = store.dispatch('a')
     const result2: 'B' = store.dispatch('b')
     const result3: Promise<'A'> = store.dispatch(thunkA())
@@ -324,7 +334,9 @@ const _anyMiddleware: any = () => () => () => {}
       undefined,
       AnyAction
     >)
-    // null was previously documented in the redux docs
+    // `null` for the `extra` generic was previously documented in the RTK "Advanced Tutorial", but
+    // is a bad pattern and users should use `unknown` instead
+    // @ts-expect-error
     store.dispatch(function () {} as ThunkAction<void, {}, null, AnyAction>)
     // unknown is the best way to type a ThunkAction if you do not care
     // about the value of the extraArgument, as it will always work with every
@@ -338,13 +350,14 @@ const _anyMiddleware: any = () => () => () => {}
    * Test: custom middleware and getDefaultMiddleware
    */
   {
+    const middleware = getDefaultMiddleware<StateA>().prepend(
+      (() => {}) as any as Middleware<(a: 'a') => 'A', StateA>
+    )
     const store = configureStore({
       reducer: reducerA,
-      middleware: [
-        (() => {}) as any as Middleware<(a: 'a') => 'A', StateA>,
-        ...getDefaultMiddleware<StateA>(),
-      ] as const,
+      middleware,
     })
+
     const result1: 'A' = store.dispatch('a')
     const result2: Promise<'A'> = store.dispatch(thunkA())
     // @ts-expect-error
@@ -400,10 +413,10 @@ const _anyMiddleware: any = () => () => () => {}
     const store = configureStore({
       reducer: reducerA,
       middleware: (getDefaultMiddleware) =>
-        [
-          (() => {}) as any as Middleware<(a: 'a') => 'A', StateA>,
-          ...getDefaultMiddleware(),
-        ] as const,
+        getDefaultMiddleware().prepend((() => {}) as any as Middleware<
+          (a: 'a') => 'A',
+          StateA
+        >),
     })
     const result1: 'A' = store.dispatch('a')
     const result2: Promise<'A'> = store.dispatch(thunkA())
@@ -438,10 +451,9 @@ const _anyMiddleware: any = () => () => () => {}
     const store = configureStore({
       reducer: reducerA,
       middleware: (getDefaultMiddleware) =>
-        [
-          (() => {}) as any as Middleware<(a: 'a') => 'A', StateA>,
-          ...getDefaultMiddleware({ thunk: false }),
-        ] as const,
+        getDefaultMiddleware({ thunk: false }).prepend(
+          (() => {}) as any as Middleware<(a: 'a') => 'A', StateA>
+        ),
     })
     const result1: 'A' = store.dispatch('a')
     // @ts-expect-error
@@ -460,4 +472,57 @@ const _anyMiddleware: any = () => () => () => {}
 
     expectNotAny(store.dispatch)
   }
+
+  {
+    interface CounterState {
+      value: number
+    }
+
+    const counterSlice = createSlice({
+      name: 'counter',
+      initialState: { value: 0 } as CounterState,
+      reducers: {
+        increment(state) {
+          state.value += 1
+        },
+        decrement(state) {
+          state.value -= 1
+        },
+        // Use the PayloadAction type to declare the contents of `action.payload`
+        incrementByAmount: (state, action: PayloadAction<number>) => {
+          state.value += action.payload
+        },
+      },
+    })
+
+    type Unsubscribe = () => void
+
+    // A fake middleware that tells TS that an unsubscribe callback is being returned for a given action
+    // This is the same signature that the "listener" middleware uses
+    const dummyMiddleware: Middleware<
+      {
+        (action: Action<'actionListenerMiddleware/add'>): Unsubscribe
+      },
+      CounterState
+    > = (storeApi) => (next) => (action) => {}
+
+    const store = configureStore({
+      reducer: counterSlice.reducer,
+      middleware: (gDM) => gDM().prepend(dummyMiddleware),
+    })
+
+    // Order matters here! We need the listener type to come first, otherwise
+    // the thunk middleware type kicks in and TS thinks a plain action is being returned
+    expectType<
+      ((action: Action<'actionListenerMiddleware/add'>) => Unsubscribe) &
+        ThunkDispatch<CounterState, undefined, AnyAction> &
+        Dispatch<AnyAction>
+    >(store.dispatch)
+
+    const unsubscribe = store.dispatch({
+      type: 'actionListenerMiddleware/add',
+    } as const)
+
+    expectType<Unsubscribe>(unsubscribe)
+  }
 }

packages/toolkit/src/tests/getDefaultMiddleware.test.ts

@@ -1,10 +1,20 @@
-import type { AnyAction, Middleware, ThunkAction } from '@reduxjs/toolkit'
+import type {
+  AnyAction,
+  Middleware,
+  ThunkAction,
+  Action,
+  ThunkDispatch,
+  Dispatch,
+} from '@reduxjs/toolkit'
 import {
   getDefaultMiddleware,
   MiddlewareArray,
   configureStore,
 } from '@reduxjs/toolkit'
 import thunk from 'redux-thunk'
+import type { ThunkMiddleware } from 'redux-thunk'
+
+import { expectType } from './helpers'
 
 describe('getDefaultMiddleware', () => {
   const ORIGINAL_NODE_ENV = process.env.NODE_ENV
@@ -27,6 +37,7 @@ describe('getDefaultMiddleware', () => {
 
   it('removes the thunk middleware if disabled', () => {
     const middleware = getDefaultMiddleware({ thunk: false })
+    // @ts-ignore
     expect(middleware.includes(thunk)).toBe(false)
     expect(middleware.length).toBe(2)
   })
@@ -44,13 +55,46 @@ describe('getDefaultMiddleware', () => {
   })
 
   it('allows passing options to thunk', () => {
-    const extraArgument = 42
+    const extraArgument = 42 as const
     const middleware = getDefaultMiddleware({
       thunk: { extraArgument },
       immutableCheck: false,
       serializableCheck: false,
     })
 
+    const m2 = getDefaultMiddleware({
+      thunk: false,
+    })
+
+    expectType<MiddlewareArray<[]>>(m2)
+
+    const dummyMiddleware: Middleware<
+      {
+        (action: Action<'actionListenerMiddleware/add'>): () => void
+      },
+      { counter: number }
+    > = (storeApi) => (next) => (action) => {}
+
+    const dummyMiddleware2: Middleware = (storeApi) => (next) => (action) => {}
+
+    const m3 = middleware.concat(dummyMiddleware, dummyMiddleware2)
+
+    expectType<
+      MiddlewareArray<
+        [
+          ThunkMiddleware<any, AnyAction, 42>,
+          Middleware<
+            (action: Action<'actionListenerMiddleware/add'>) => () => void,
+            {
+              counter: number
+            },
+            Dispatch<AnyAction>
+          >,
+          Middleware<{}, any, Dispatch<AnyAction>>
+        ]
+      >
+    >(m3)
+
     const testThunk: ThunkAction<void, {}, number, AnyAction> = (
       dispatch,
       getState,
@@ -66,6 +110,10 @@ describe('getDefaultMiddleware', () => {
       middleware,
     })
 
+    expectType<ThunkDispatch<any, 42, AnyAction> & Dispatch<AnyAction>>(
+      store.dispatch
+    )
+
     store.dispatch(testThunk)
   })
 

packages/toolkit/src/tsHelpers.ts

@@ -1,4 +1,5 @@
-import type { Middleware } from 'redux'
+import type { Middleware, Dispatch } from 'redux'
+import type { MiddlewareArray } from './utils'
 
 /**
  * return True if T is `any`, otherwise return False
@@ -65,20 +66,30 @@ export type IsUnknownOrNonInferrable<T, True, False> = AtLeastTS35<
   IsEmptyObj<T, True, IsUnknown<T, True, False>>
 >
 
-/**
- * Combines all dispatch signatures of all middlewares in the array `M` into
- * one intersected dispatch signature.
- */
-export type DispatchForMiddlewares<M> = M extends ReadonlyArray<any>
-  ? UnionToIntersection<
-      M[number] extends infer MiddlewareValues
-        ? MiddlewareValues extends Middleware<infer DispatchExt, any, any>
-          ? DispatchExt extends Function
-            ? IsAny<DispatchExt, never, DispatchExt>
-            : never
-          : never
-        : never
+// Appears to have a convenient side effect of ignoring `never` even if that's not what you specified
+export type ExcludeFromTuple<T, E, Acc extends unknown[] = []> = T extends [
+  infer Head,
+  ...infer Tail
+]
+  ? ExcludeFromTuple<Tail, E, [...Acc, ...([Head] extends [E] ? [] : [Head])]>
+  : Acc
+
+type ExtractDispatchFromMiddlewareTuple<
+  MiddlewareTuple extends any[],
+  Acc extends {}
+> = MiddlewareTuple extends [infer Head, ...infer Tail]
+  ? ExtractDispatchFromMiddlewareTuple<
+      Tail,
+      Acc & (Head extends Middleware<infer D, any> ? IsAny<D, {}, D> : {})
     >
+  : Acc
+
+export type ExtractDispatchExtensions<M> = M extends MiddlewareArray<
+  infer MiddlewareTuple
+>
+  ? ExtractDispatchFromMiddlewareTuple<MiddlewareTuple, {}>
+  : M extends Middleware[]
+  ? ExtractDispatchFromMiddlewareTuple<[...M], {}>
   : never
 
 /**