From f9049cca5cc9e9c938f4d4fb3d4ca7b6a5f56992 Mon Sep 17 00:00:00 2001 From: Georg Wicke-Arndt Date: Mon, 23 Jan 2023 16:54:15 +0100 Subject: [PATCH 1/2] Add caching to serializableStateInvariantMiddleware --- .../serializableStateInvariantMiddleware.ts | 38 ++++++++++++++-- ...rializableStateInvariantMiddleware.test.ts | 44 ++++++++++++++++++- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/packages/toolkit/src/serializableStateInvariantMiddleware.ts b/packages/toolkit/src/serializableStateInvariantMiddleware.ts index 5aeafb26aa..7c6ffb34a2 100644 --- a/packages/toolkit/src/serializableStateInvariantMiddleware.ts +++ b/packages/toolkit/src/serializableStateInvariantMiddleware.ts @@ -38,7 +38,8 @@ export function findNonSerializableValue( path: string = '', isSerializable: (value: unknown) => boolean = isPlain, getEntries?: (value: unknown) => [string, any][], - ignoredPaths: IgnorePaths = [] + ignoredPaths: IgnorePaths = [], + cache?: WeakSet ): NonSerializableValue | false { let foundNestedSerializable: NonSerializableValue | false @@ -53,6 +54,8 @@ export function findNonSerializableValue( return false } + if (cache?.has(value)) return false + const entries = getEntries != null ? getEntries(value) : Object.entries(value) const hasIgnoredPaths = ignoredPaths.length > 0 @@ -85,7 +88,8 @@ export function findNonSerializableValue( nestedPath, isSerializable, getEntries, - ignoredPaths + ignoredPaths, + cache ) if (foundNestedSerializable) { @@ -94,9 +98,23 @@ export function findNonSerializableValue( } } + if (cache && isNestedFrozen(value)) cache.add(value) + return false } +export function isNestedFrozen(value: object) { + if (!Object.isFrozen(value)) return false + + for (const nestedValue of Object.values(value)) { + if (typeof nestedValue !== 'object' || nestedValue === null) continue + + if (!isNestedFrozen(nestedValue)) return false + } + + return true +} + /** * Options for `createSerializableStateInvariantMiddleware()`. * @@ -150,6 +168,12 @@ export interface SerializableStateInvariantMiddlewareOptions { * Opt out of checking actions. When set to `true`, other action-related params will be ignored. */ ignoreActions?: boolean + + /** + * Opt out of caching the results. The cache uses a WeakSet and speeds up repeated checking processes. + * The cache is automatically disabled if no browser support for WeakSet is present. + */ + disableCache?: boolean } /** @@ -176,8 +200,12 @@ export function createSerializableStateInvariantMiddleware( warnAfter = 32, ignoreState = false, ignoreActions = false, + disableCache = false, } = options + const cache: WeakSet | undefined = + !disableCache && WeakSet ? new WeakSet() : undefined + return (storeAPI) => (next) => (action) => { const result = next(action) @@ -196,7 +224,8 @@ export function createSerializableStateInvariantMiddleware( '', isSerializable, getEntries, - ignoredActionPaths + ignoredActionPaths, + cache ) if (foundActionNonSerializableValue) { @@ -223,7 +252,8 @@ export function createSerializableStateInvariantMiddleware( '', isSerializable, getEntries, - ignoredPaths + ignoredPaths, + cache ) if (foundStateNonSerializableValue) { diff --git a/packages/toolkit/src/tests/serializableStateInvariantMiddleware.test.ts b/packages/toolkit/src/tests/serializableStateInvariantMiddleware.test.ts index 2b55ff97b4..e408358e31 100644 --- a/packages/toolkit/src/tests/serializableStateInvariantMiddleware.test.ts +++ b/packages/toolkit/src/tests/serializableStateInvariantMiddleware.test.ts @@ -3,13 +3,15 @@ import { createConsole, getLog, } from 'console-testing-library/pure' -import type { Reducer } from '@reduxjs/toolkit' +import type { AnyAction, Reducer } from '@reduxjs/toolkit' import { + createNextState, configureStore, createSerializableStateInvariantMiddleware, findNonSerializableValue, isPlain, } from '@reduxjs/toolkit' +import { isNestedFrozen } from '@internal/serializableStateInvariantMiddleware' // Mocking console let restore = () => {} @@ -594,4 +596,44 @@ describe('serializableStateInvariantMiddleware', () => { store.dispatch({ type: 'SOME_ACTION' }) expect(getLog().log).toMatch('') }) + + it('Should cache its results', () => { + const reducer: Reducer<[], AnyAction> = (state = [], action) => { + if (action.type === 'SET_STATE') return action.payload + return state + } + + let numPlainChecks = 0 + const countPlainChecks = (x: any) => { + numPlainChecks++ + return isPlain(x) + } + + const serializableStateInvariantMiddleware = + createSerializableStateInvariantMiddleware({ + isSerializable: countPlainChecks, + }) + + const store = configureStore({ + reducer: { + testSlice: reducer, + }, + middleware: [serializableStateInvariantMiddleware], + }) + + const state = createNextState([], () => + new Array(50).fill(0).map((x, i) => ({ i })) + ) + expect(isNestedFrozen(state)).toBe(true) + + store.dispatch({ + type: 'SET_STATE', + payload: state, + }) + expect(numPlainChecks).toBeGreaterThan(state.length) + + numPlainChecks = 0 + store.dispatch({ type: 'NOOP' }) + expect(numPlainChecks).toBeLessThan(10) + }) }) From bcd0615bc0e1545ec7873a523c6f367fd71380a8 Mon Sep 17 00:00:00 2001 From: Georg Wicke-Arndt Date: Mon, 23 Jan 2023 17:15:55 +0100 Subject: [PATCH 2/2] Simplify test --- .../tests/serializableStateInvariantMiddleware.test.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/toolkit/src/tests/serializableStateInvariantMiddleware.test.ts b/packages/toolkit/src/tests/serializableStateInvariantMiddleware.test.ts index e408358e31..adf99bbf7a 100644 --- a/packages/toolkit/src/tests/serializableStateInvariantMiddleware.test.ts +++ b/packages/toolkit/src/tests/serializableStateInvariantMiddleware.test.ts @@ -598,11 +598,6 @@ describe('serializableStateInvariantMiddleware', () => { }) it('Should cache its results', () => { - const reducer: Reducer<[], AnyAction> = (state = [], action) => { - if (action.type === 'SET_STATE') return action.payload - return state - } - let numPlainChecks = 0 const countPlainChecks = (x: any) => { numPlainChecks++ @@ -615,8 +610,9 @@ describe('serializableStateInvariantMiddleware', () => { }) const store = configureStore({ - reducer: { - testSlice: reducer, + reducer: (state = [], action) => { + if (action.type === 'SET_STATE') return action.payload + return state }, middleware: [serializableStateInvariantMiddleware], })