Repository: reduxjs/redux-toolkit
Pull request #428 (2020-03-11 22:27:30)

simplify signatures of `ActionCreatorWithOptionalPayload` and `A…
This would probably reduce the pain of #426, #399 and a few other issues where people had `strictNullChecks: false`. Also, it might improve IntelliSense for `ActionCreatorWithPayload` with a little tradeoff.

Changes for `ActionCreatorWithOptionalPayload`:
Before, assuming actionCreator was a `ActionCreatorWithOptionalPayload<string|undefined>
* calling actionCreator() returned a `PayloadAction<undefined>`
* calling actionCreator(undefined) returned a `PayloadAction<undefined>`
* calling actionCreator("test") returned a `PayloadAction<"test">`
* calling actionCreator(someStringTypedVariable) returned a `PayloadAction<string>`

Now, all those calls will return `PayloadAction<string | undefined>` - but autocompletion will work quite better for object types payloads.

Simlarily, assuming actionCreator was a `ActionCreatorWithPayload<string>
* calling actionCreator("test") returned a `PayloadAction<"test">`
* calling actionCreator(someStringTypedVariable) returned a `PayloadAction<string>`

Now, both those cases will return `PayloadAction<string>`, but autocompletion will probably work better for object type payloads.

I've been thinking about this for a bit and I believe the extra granularity we had before might not be worth the frustration people are having with this. What do you think about this simplification?

codesandbox[bot]: This pull request is automatically built and testable in [CodeSandbox](https://codesandbox.io).

  To see build info of the built libraries, click [here](https://ci.codesandbox.io/status/reduxjs/redux-toolkit/pr/428/builds/14362) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 982da2ca132ab7f965e93e72d82bb8d2c1aec6a4:

|Sandbox| Source |
|--|--|
|[angry-jones-5b74e](https://codesandbox.io/s/angry-jones-5b74e)| Configuration |
|[awesome-mccarthy-3u291](https://codesandbox.io/s/awesome-mccarthy-3u291)| Configuration |
|[affectionate-dubinsky-m5nx0](https://codesandbox.io/s/affectionate-dubinsky-m5nx0)| Configuration |

markerikson: If you think it's an improvement, we can go with it.  Is this good as-is?
phryneas: I think so. It might be breaking if someone actually relied on the old behaviour, but I doubt anyone really used that.

We might want to ask for feedback in the old issues though.

Patch:
diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md
index 8b19b879..d8e0eede 100644
--- a/etc/redux-toolkit.api.md
+++ b/etc/redux-toolkit.api.md
@@ -28,8 +28,7 @@ export interface ActionCreatorWithNonInferrablePayload<T extends string = string
 
 // @public
 export interface ActionCreatorWithOptionalPayload<P, T extends string = string> extends BaseActionCreator<P, T> {
-    (payload?: undefined): PayloadAction<undefined, T>;
-    <PT extends Diff<P, undefined>>(payload?: PT): PayloadAction<PT, T>;
+    (payload?: P): PayloadAction<P, T>;
 }
 
 // @public
@@ -39,7 +38,6 @@ export interface ActionCreatorWithoutPayload<T extends string = string> extends
 
 // @public
 export interface ActionCreatorWithPayload<P, T extends string = string> extends BaseActionCreator<P, T> {
-    <PT extends P>(payload: PT): PayloadAction<PT, T>;
     (payload: P): PayloadAction<P, T>;
 }
 
@@ -251,6 +249,7 @@ export interface SerializableStateInvariantMiddlewareOptions {
     ignoredActions?: string[];
     ignoredPaths?: string[];
     isSerializable?: (value: any) => boolean;
+    warnAfter?: number;
 }
 
 // @alpha (undocumented)
diff --git a/src/createAction.ts b/src/createAction.ts
index 2dea13b3..93558d7b 100644
--- a/src/createAction.ts
+++ b/src/createAction.ts
@@ -123,16 +123,12 @@ export interface ActionCreatorWithPreparedPayload<
  */
 export interface ActionCreatorWithOptionalPayload<P, T extends string = string>
   extends BaseActionCreator<P, T> {
-  /**
-   * Calling this {@link redux#ActionCreator} without arguments will
-   * return a {@link PayloadAction} of type `T` with a payload of `undefined`
-   */
-  (payload?: undefined): PayloadAction<undefined, T>
   /**
    * Calling this {@link redux#ActionCreator} with an argument will
-   * return a {@link PayloadAction} of type `T` with a payload of `P`
+   * return a {@link PayloadAction} of type `T` with a payload of `P`.
+   * Calling it without an argument will return a PayloadAction with a payload of `undefined`.
    */
-  <PT extends Diff<P, undefined>>(payload?: PT): PayloadAction<PT, T>
+  (payload?: P): PayloadAction<P, T>
 }
 
 /**
@@ -160,12 +156,6 @@ export interface ActionCreatorWithoutPayload<T extends string = string>
  */
 export interface ActionCreatorWithPayload<P, T extends string = string>
   extends BaseActionCreator<P, T> {
-  /**
-   * Calling this {@link redux#ActionCreator} with an argument will
-   * return a {@link PayloadAction} of type `T` with a payload of `P`
-   * If possible, `P` will be narrowed down to the exact type of the payload argument.
-   */
-  <PT extends P>(payload: PT): PayloadAction<PT, T>
   /**
    * Calling this {@link redux#ActionCreator} with an argument will
    * return a {@link PayloadAction} of type `T` with a payload of `P`
@@ -333,8 +323,6 @@ export function getType<T extends string>(
 
 // helper types for more readable typings
 
-type Diff<T, U> = T extends U ? never : T
-
 type IfPrepareActionMethodProvided<
   PA extends PrepareAction<any> | void,
   True,
diff --git a/src/immutableStateInvariantMiddleware.ts b/src/immutableStateInvariantMiddleware.ts
index 22bab404..b80f3731 100644
--- a/src/immutableStateInvariantMiddleware.ts
+++ b/src/immutableStateInvariantMiddleware.ts
@@ -1,4 +1,5 @@
 import { Middleware } from 'redux'
+import { getTimeMeasureUtils } from './utils'
 
 type EntryProcessor = (key: string, value: any) => any
 
@@ -174,6 +175,7 @@ type IsImmutableFunc = (value: any) => boolean
 export interface ImmutableStateInvariantMiddlewareOptions {
   isImmutable?: IsImmutableFunc
   ignoredPaths?: string[]
+  warnAfter?: number
 }
 
 export function createImmutableStateInvariantMiddleware(
@@ -183,7 +185,11 @@ export function createImmutableStateInvariantMiddleware(
     return () => next => action => next(action)
   }
 
-  const { isImmutable = isImmutableDefault, ignoredPaths } = options
+  const {
+    isImmutable = isImmutableDefault,
+    ignoredPaths,
+    warnAfter = 32
+  } = options
   const track = trackForMutations.bind(null, isImmutable, ignoredPaths)
 
   return ({ getState }) => {
@@ -192,39 +198,51 @@ export function createImmutableStateInvariantMiddleware(
 
     let result
     return next => action => {
-      state = getState()
-
-      result = tracker.detectMutations()
-      // Track before potentially not meeting the invariant
-      tracker = track(state)
-
-      invariant(
-        !result.wasMutated,
-        `A state mutation was detected between dispatches, in the path '${(
-          result.path || []
-        ).join(
-          '.'
-        )}'.  This may cause incorrect behavior. (http://redux.js.org/docs/Troubleshooting.html#never-mutate-reducer-arguments)`
+      const measureUtils = getTimeMeasureUtils(
+        warnAfter,
+        'ImmutableStateInvariantMiddleware'
       )
 
-      const dispatchedAction = next(action)
-      state = getState()
+      measureUtils.measureTime(() => {
+        state = getState()
 
-      result = tracker.detectMutations()
-      // Track before potentially not meeting the invariant
-      tracker = track(state)
+        result = tracker.detectMutations()
+        // Track before potentially not meeting the invariant
+        tracker = track(state)
 
-      result.wasMutated &&
         invariant(
           !result.wasMutated,
-          `A state mutation was detected inside a dispatch, in the path: ${(
+          `A state mutation was detected between dispatches, in the path '${(
             result.path || []
           ).join(
             '.'
-          )}. Take a look at the reducer(s) handling the action ${stringify(
-            action
-          )}. (http://redux.js.org/docs/Troubleshooting.html#never-mutate-reducer-arguments)`
+          )}'.  This may cause incorrect behavior. (http://redux.js.org/docs/Troubleshooting.html#never-mutate-reducer-arguments)`
         )
+      })
+
+      const dispatchedAction = next(action)
+
+      measureUtils.measureTime(() => {
+        state = getState()
+
+        result = tracker.detectMutations()
+        // Track before potentially not meeting the invariant
+        tracker = track(state)
+
+        result.wasMutated &&
+          invariant(
+            !result.wasMutated,
+            `A state mutation was detected inside a dispatch, in the path: ${(
+              result.path || []
+            ).join(
+              '.'
+            )}. Take a look at the reducer(s) handling the action ${stringify(
+              action
+            )}. (http://redux.js.org/docs/Troubleshooting.html#never-mutate-reducer-arguments)`
+          )
+      })
+
+      measureUtils.warnIfExceeded()
 
       return dispatchedAction
     }
diff --git a/src/serializableStateInvariantMiddleware.ts b/src/serializableStateInvariantMiddleware.ts
index 978b0a7a..a4955425 100644
--- a/src/serializableStateInvariantMiddleware.ts
+++ b/src/serializableStateInvariantMiddleware.ts
@@ -1,5 +1,6 @@
 import isPlainObject from './isPlainObject'
 import { Middleware } from 'redux'
+import { getTimeMeasureUtils } from './utils'
 
 /**
  * Returns true if the passed value is "plain", i.e. a value that is either
@@ -114,6 +115,10 @@ export interface SerializableStateInvariantMiddlewareOptions {
    * An array of dot-separated path strings to ignore when checking for serializability, Defaults to []
    */
   ignoredPaths?: string[]
+  /**
+   * Execution time warning threshold. If the middleware takes longer than `warnAfter` ms, a warning will be displayed in the console. Defaults to 32
+   */
+  warnAfter?: number
 }
 
 /**
@@ -135,7 +140,8 @@ export function createSerializableStateInvariantMiddleware(
     isSerializable = isPlain,
     getEntries,
     ignoredActions = [],
-    ignoredPaths = []
+    ignoredPaths = [],
+    warnAfter = 32
   } = options
 
   return storeAPI => next => action => {
@@ -143,48 +149,58 @@ export function createSerializableStateInvariantMiddleware(
       return next(action)
     }
 
-    const foundActionNonSerializableValue = findNonSerializableValue(
-      action,
-      [],
-      isSerializable,
-      getEntries
+    const measureUtils = getTimeMeasureUtils(
+      warnAfter,
+      'SerializableStateInvariantMiddleware'
     )
-
-    if (foundActionNonSerializableValue) {
-      const { keyPath, value } = foundActionNonSerializableValue
-
-      console.error(
-        `A non-serializable value was detected in an action, in the path: \`${keyPath}\`. Value:`,
-        value,
-        '\nTake a look at the logic that dispatched this action: ',
+    measureUtils.measureTime(() => {
+      const foundActionNonSerializableValue = findNonSerializableValue(
         action,
-        '\n(See https://redux.js.org/faq/actions#why-should-type-be-a-string-or-at-least-serializable-why-should-my-action-types-be-constants)'
+        [],
+        isSerializable,
+        getEntries
       )
-    }
+
+      if (foundActionNonSerializableValue) {
+        const { keyPath, value } = foundActionNonSerializableValue
+
+        console.error(
+          `A non-serializable value was detected in an action, in the path: \`${keyPath}\`. Value:`,
+          value,
+          '\nTake a look at the logic that dispatched this action: ',
+          action,
+          '\n(See https://redux.js.org/faq/actions#why-should-type-be-a-string-or-at-least-serializable-why-should-my-action-types-be-constants)'
+        )
+      }
+    })
 
     const result = next(action)
 
-    const state = storeAPI.getState()
+    measureUtils.measureTime(() => {
+      const state = storeAPI.getState()
 
-    const foundStateNonSerializableValue = findNonSerializableValue(
-      state,
-      [],
-      isSerializable,
-      getEntries,
-      ignoredPaths
-    )
+      const foundStateNonSerializableValue = findNonSerializableValue(
+        state,
+        [],
+        isSerializable,
+        getEntries,
+        ignoredPaths
+      )
 
-    if (foundStateNonSerializableValue) {
-      const { keyPath, value } = foundStateNonSerializableValue
+      if (foundStateNonSerializableValue) {
+        const { keyPath, value } = foundStateNonSerializableValue
 
-      console.error(
-        `A non-serializable value was detected in the state, in the path: \`${keyPath}\`. Value:`,
-        value,
-        `
+        console.error(
+          `A non-serializable value was detected in the state, in the path: \`${keyPath}\`. Value:`,
+          value,
+          `
 Take a look at the reducer(s) handling this action type: ${action.type}.
 (See https://redux.js.org/faq/organizing-state#can-i-put-functions-promises-or-other-non-serializable-items-in-my-store-state)`
-      )
-    }
+        )
+      }
+    })
+
+    measureUtils.warnIfExceeded()
 
     return result
   }
diff --git a/src/utils.ts b/src/utils.ts
new file mode 100644
index 00000000..933c7f4a
--- /dev/null
+++ b/src/utils.ts
@@ -0,0 +1,21 @@
+export function getTimeMeasureUtils(maxDelay: number, fnName: string) {
+  let elapsed = 0
+  return {
+    measureTime<T>(fn: () => T): T {
+      const started = Date.now()
+      try {
+        return fn()
+      } finally {
+        const finished = Date.now()
+        elapsed += finished - started
+      }
+    },
+    warnIfExceeded() {
+      if (elapsed > maxDelay) {
+        console.warn(`${fnName} took ${elapsed}ms, which is more than the warning threshold of ${maxDelay}ms. 
+If your state or actions are very large, you may want to disable the middleware as it might cause too much of a slowdown in development mode. See https://redux-toolkit.js.org/api/getDefaultMiddleware for instructions.
+It is disabled in production builds, so you don't need to worry about that.`)
+      }
+    }
+  }
+}


Test patch:
diff --git a/src/immutableStateInvariantMiddleware.test.ts b/src/immutableStateInvariantMiddleware.test.ts
index 38763d65..104e677d 100644
--- a/src/immutableStateInvariantMiddleware.test.ts
+++ b/src/immutableStateInvariantMiddleware.test.ts
@@ -5,6 +5,7 @@ import {
   trackForMutations,
   ImmutableStateInvariantMiddlewareOptions
 } from './immutableStateInvariantMiddleware'
+import { mockConsole, createConsole, getLog } from 'console-testing-library'
 
 describe('createImmutableStateInvariantMiddleware', () => {
   let state: { foo: { bar: number[]; baz: string } }
@@ -121,6 +122,42 @@ describe('createImmutableStateInvariantMiddleware', () => {
       dispatch({ type: 'SOME_ACTION' })
     }).not.toThrow()
   })
+
+  it('Should print a warning if execution takes too long', () => {
+    state.foo.bar = new Array(10000).fill({ value: 'more' })
+
+    const next: Dispatch = action => action
+
+    const dispatch = middleware({ warnAfter: 4 })(next)
+
+    const restore = mockConsole(createConsole())
+    try {
+      dispatch({ type: 'SOME_ACTION' })
+      expect(getLog().log).toMatch(
+        /^ImmutableStateInvariantMiddleware took \d*ms, which is more than the warning threshold of 4ms./
+      )
+    } finally {
+      restore()
+    }
+  })
+
+  it('Should not print a warning if "next" takes too long', () => {
+    const next: Dispatch = action => {
+      const started = Date.now()
+      while (Date.now() - started < 8) {}
+      return action
+    }
+
+    const dispatch = middleware({ warnAfter: 4 })(next)
+
+    const restore = mockConsole(createConsole())
+    try {
+      dispatch({ type: 'SOME_ACTION' })
+      expect(getLog().log).toEqual('')
+    } finally {
+      restore()
+    }
+  })
 })
 
 describe('trackForMutations', () => {
diff --git a/src/serializableStateInvariantMiddleware.test.ts b/src/serializableStateInvariantMiddleware.test.ts
index 9c3476f8..c435b659 100644
--- a/src/serializableStateInvariantMiddleware.test.ts
+++ b/src/serializableStateInvariantMiddleware.test.ts
@@ -408,4 +408,51 @@ describe('serializableStateInvariantMiddleware', () => {
     expect(log).toBe('')
     const q = 42
   })
+
+  it('Should print a warning if execution takes too long', () => {
+    const reducer: Reducer = (state = 42, action) => {
+      return state
+    }
+
+    const serializableStateInvariantMiddleware = createSerializableStateInvariantMiddleware(
+      { warnAfter: 4 }
+    )
+
+    const store = configureStore({
+      reducer: {
+        testSlice: reducer
+      },
+      middleware: [serializableStateInvariantMiddleware]
+    })
+
+    store.dispatch({
+      type: 'SOME_ACTION',
+      payload: new Array(10000).fill({ value: 'more' })
+    })
+    expect(getLog().log).toMatch(
+      /^SerializableStateInvariantMiddleware took \d*ms, which is more than the warning threshold of 4ms./
+    )
+  })
+
+  it('Should not print a warning if "reducer" takes too long', () => {
+    const reducer: Reducer = (state = 42, action) => {
+      const started = Date.now()
+      while (Date.now() - started < 8) {}
+      return state
+    }
+
+    const serializableStateInvariantMiddleware = createSerializableStateInvariantMiddleware(
+      { warnAfter: 4 }
+    )
+
+    const store = configureStore({
+      reducer: {
+        testSlice: reducer
+      },
+      middleware: [serializableStateInvariantMiddleware]
+    })
+
+    store.dispatch({ type: 'SOME_ACTION' })
+    expect(getLog().log).toMatch('')
+  })
 })
diff --git a/type-tests/files/createAction.typetest.ts b/type-tests/files/createAction.typetest.ts
index 702913a6..e702070b 100644
--- a/type-tests/files/createAction.typetest.ts
+++ b/type-tests/files/createAction.typetest.ts
@@ -73,9 +73,9 @@ function expectType<T>(p: T): T {
     { type: 'action' }
   ) as PayloadActionCreator<number | undefined>
 
-  expectType<PayloadAction<number>>(actionCreator(1))
-  expectType<PayloadAction<undefined>>(actionCreator())
-  expectType<PayloadAction<undefined>>(actionCreator(undefined))
+  expectType<PayloadAction<number | undefined>>(actionCreator(1))
+  expectType<PayloadAction<number | undefined>>(actionCreator())
+  expectType<PayloadAction<number | undefined>>(actionCreator(undefined))
 
   // typings:expect-error
   expectType<PayloadAction<number>>(actionCreator())
@@ -104,9 +104,9 @@ function expectType<T>(p: T): T {
     { type: 'action' }
   ) as PayloadActionCreator<number>
 
-  const actionCreator2: ActionCreator<
-    PayloadAction<number>
-  > = payloadActionCreator2
+  const actionCreator2: ActionCreator<PayloadAction<
+    number
+  >> = payloadActionCreator2
 }
 
 /* createAction() */