From 573f4e17af395727123f1b4eebfdaaca5e607878 Mon Sep 17 00:00:00 2001 From: Alexander Kireev Date: Mon, 15 Jun 2026 17:36:48 +0700 Subject: [PATCH] fix: don't pass resultEqualityCheck to inputStabilityCheck probe `runInputStabilityCheck` memoizes an internal probe function that returns a fresh empty object on every call, in order to detect whether the configured memoizer treats two sets of input selector results as equal. It built that probe with the *full* `memoizeOptions`, including a user-provided `resultEqualityCheck`. When the input selectors are unstable (the exact case this check exists to catch), the probe is invoked twice with differing arguments, which causes the memoizer to run `resultEqualityCheck` against the empty probe objects (`{}`) instead of the real selector results. Any `resultEqualityCheck` that inspects the shape of the result (e.g. comparing a `Set` or reading properties) then throws a development-only runtime error. PR #699 only addressed the stable-input path. This strips `resultEqualityCheck` from the options used to build the probe so it is never called with the probe objects, fixing the remaining unstable-input case. Adds regression tests for `inputStabilityCheck` set to `once` and `always` with unstable inputs. --- src/devModeChecks/inputStabilityCheck.ts | 24 ++++++++- test/inputStabilityCheck.spec.ts | 68 ++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/src/devModeChecks/inputStabilityCheck.ts b/src/devModeChecks/inputStabilityCheck.ts index 30705693..65b72332 100644 --- a/src/devModeChecks/inputStabilityCheck.ts +++ b/src/devModeChecks/inputStabilityCheck.ts @@ -30,7 +30,29 @@ export const runInputStabilityCheck = ( const { memoize, memoizeOptions } = options const { inputSelectorResults, inputSelectorResultsCopy } = inputSelectorResultsObject - const createAnEmptyObject = memoize(() => ({}), ...memoizeOptions) + // The probe function below intentionally returns a new object reference on + // every call so we can detect whether `memoize` considers the two sets of + // input selector results to be equal. A user-provided `resultEqualityCheck` + // is irrelevant to that comparison (it compares *results*, not arguments) and + // must not run here, otherwise it would be invoked with these empty probe + // objects rather than the actual selector results. See #693. + const memoizeOptionsWithoutResultEqualityCheck = ( + memoizeOptions as unknown[] + ).map(option => { + if ( + option != null && + typeof option === 'object' && + 'resultEqualityCheck' in option + ) { + const { resultEqualityCheck, ...rest } = option as Record + return rest + } + return option + }) + const createAnEmptyObject = memoize( + () => ({}), + ...memoizeOptionsWithoutResultEqualityCheck + ) // if the memoize method thinks the parameters are equal, these *should* be the same reference const areInputSelectorResultsEqual = createAnEmptyObject.apply(null, inputSelectorResults) === diff --git a/test/inputStabilityCheck.spec.ts b/test/inputStabilityCheck.spec.ts index 2f7b8b95..ecde833e 100644 --- a/test/inputStabilityCheck.spec.ts +++ b/test/inputStabilityCheck.spec.ts @@ -272,4 +272,72 @@ describe('the effects of inputStabilityCheck with resultEqualityCheck', () => { expect(resultEqualityCheck).not.toHaveBeenCalled() } ) + + localTest( + 'resultEqualityCheck should not be called with empty objects when inputStabilityCheck is set to once and input selectors are unstable', + ({ store }) => { + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const selectTodoIds = createAppSelector( + [state => [...state.todos]], + todos => todos.map(({ id }) => id), + { + memoizeOptions: { resultEqualityCheck }, + devModeChecks: { inputStabilityCheck: 'once' } + } + ) + + // The first call runs `inputStabilityCheck`, which internally memoizes a + // probe function returning empty objects. Because the input selector is + // unstable, the probe runs twice with differing arguments. The user's + // `resultEqualityCheck` must not be invoked with those empty probe objects. + const firstResult = selectTodoIds(store.getState()) + + for (const call of resultEqualityCheck.mock.calls) { + expect(call[0]).toBeInstanceOf(Array) + expect(call[1]).toBeInstanceOf(Array) + } + + const secondResult = selectTodoIds(store.getState()) + + for (const call of resultEqualityCheck.mock.calls) { + expect(call[0]).toBeInstanceOf(Array) + expect(call[1]).toBeInstanceOf(Array) + } + + // Sanity check: the unstable input still triggers the stability warning. + expect(consoleSpy).toHaveBeenCalledTimes(1) + + expect(firstResult).toBe(secondResult) + + consoleSpy.mockRestore() + } + ) + + localTest( + 'resultEqualityCheck should not be called with empty objects when inputStabilityCheck is set to always and input selectors are unstable', + ({ store }) => { + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const selectTodoIds = createAppSelector( + [state => [...state.todos]], + todos => todos.map(({ id }) => id), + { + memoizeOptions: { resultEqualityCheck }, + devModeChecks: { inputStabilityCheck: 'always' } + } + ) + + selectTodoIds(store.getState()) + selectTodoIds(store.getState()) + selectTodoIds(store.getState()) + + for (const call of resultEqualityCheck.mock.calls) { + expect(call[0]).toBeInstanceOf(Array) + expect(call[1]).toBeInstanceOf(Array) + } + + consoleSpy.mockRestore() + } + ) })