From fd004e978636f2ea62242467ebc8b77fcf0c5a55 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Wed, 29 Nov 2023 21:17:49 -0500 Subject: [PATCH] Switch createSelector to use weakMapMemoize by default --- src/createSelectorCreator.ts | 16 ++++++++-------- test/defaultMemoize.spec.ts | 7 ++++++- test/inputStabilityCheck.spec.ts | 7 ++++++- test/perfComparisons.spec.ts | 4 ++-- test/reselect.spec.ts | 20 +++++++++++--------- test/selectorUtils.spec.ts | 5 +++-- test/weakmapMemoize.spec.ts | 4 ++-- 7 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/createSelectorCreator.ts b/src/createSelectorCreator.ts index 5d92015c7..4efe8bdc1 100644 --- a/src/createSelectorCreator.ts +++ b/src/createSelectorCreator.ts @@ -1,4 +1,4 @@ -import { defaultMemoize } from './defaultMemoize' +import { weakMapMemoize } from './weakMapMemoize' import type { Combiner, @@ -35,8 +35,8 @@ import { * @public */ export interface CreateSelectorFunction< - MemoizeFunction extends UnknownMemoizer = typeof defaultMemoize, - ArgsMemoizeFunction extends UnknownMemoizer = typeof defaultMemoize + MemoizeFunction extends UnknownMemoizer = typeof weakMapMemoize, + ArgsMemoizeFunction extends UnknownMemoizer = typeof weakMapMemoize > { /** * Creates a memoized selector function. @@ -222,13 +222,13 @@ export function setInputStabilityCheckEnabled( */ export function createSelectorCreator< MemoizeFunction extends UnknownMemoizer, - ArgsMemoizeFunction extends UnknownMemoizer = typeof defaultMemoize + ArgsMemoizeFunction extends UnknownMemoizer = typeof weakMapMemoize >( options: Simplify< SetRequired< CreateSelectorOptions< - typeof defaultMemoize, - typeof defaultMemoize, + typeof weakMapMemoize, + typeof weakMapMemoize, MemoizeFunction, ArgsMemoizeFunction >, @@ -372,7 +372,7 @@ export function createSelectorCreator< const { memoize, memoizeOptions = [], - argsMemoize = defaultMemoize, + argsMemoize = weakMapMemoize, argsMemoizeOptions = [], inputStabilityCheck = globalStabilityCheck } = combinedOptions @@ -477,4 +477,4 @@ export function createSelectorCreator< * @public */ export const createSelector = - /* #__PURE__ */ createSelectorCreator(defaultMemoize) + /* #__PURE__ */ createSelectorCreator(weakMapMemoize) diff --git a/test/defaultMemoize.spec.ts b/test/defaultMemoize.spec.ts index 94e98d97e..7d3237f62 100644 --- a/test/defaultMemoize.spec.ts +++ b/test/defaultMemoize.spec.ts @@ -1,8 +1,13 @@ // TODO: Add test for React Redux connect function -import { createSelector, defaultMemoize } from 'reselect' +import { defaultMemoize, createSelectorCreator } from 'reselect' import { vi } from 'vitest' +const createSelector = createSelectorCreator({ + memoize: defaultMemoize, + argsMemoize: defaultMemoize +}) + describe('defaultMemoize', () => { test('Basic memoization', () => { let called = 0 diff --git a/test/inputStabilityCheck.spec.ts b/test/inputStabilityCheck.spec.ts index be17bc427..3dc0d28e6 100644 --- a/test/inputStabilityCheck.spec.ts +++ b/test/inputStabilityCheck.spec.ts @@ -1,4 +1,8 @@ -import { createSelector, setInputStabilityCheckEnabled } from 'reselect' +import { + createSelector, + defaultMemoize, + setInputStabilityCheckEnabled +} from 'reselect' import { shallowEqual } from 'react-redux' describe('inputStabilityCheck', () => { @@ -107,6 +111,7 @@ describe('inputStabilityCheck', () => { [unstableInput], ({ a, b }) => a + b, { + memoize: defaultMemoize, memoizeOptions: { equalityCheck: shallowEqual } diff --git a/test/perfComparisons.spec.ts b/test/perfComparisons.spec.ts index 2959eca21..ae66c01c0 100644 --- a/test/perfComparisons.spec.ts +++ b/test/perfComparisons.spec.ts @@ -337,11 +337,11 @@ describe('More perf comparisons', () => { cdTodoIdsAndNames(reduxStates[0]) - expect(cdTodoIdsAndNames.recomputations()).toBe(NUM_ITEMS + 1) + expect(cdTodoIdsAndNames.recomputations()).toBe(NUM_ITEMS) cdTodoIdsAndNames(reduxStates[1]) - expect(cdTodoIdsAndNames.recomputations()).toBe(NUM_ITEMS + 2) + expect(cdTodoIdsAndNames.recomputations()).toBe(NUM_ITEMS) // @ts-ignore reduxStates[0] = null diff --git a/test/reselect.spec.ts b/test/reselect.spec.ts index 44e951dfe..adee306f6 100644 --- a/test/reselect.spec.ts +++ b/test/reselect.spec.ts @@ -344,6 +344,7 @@ describe('Customizing selectors', () => { (state: StateAB) => state.b, (a, b) => a + b, { + memoize: defaultMemoize, memoizeOptions: (a, b) => { memoizer1Calls++ return a === b @@ -361,6 +362,7 @@ describe('Customizing selectors', () => { (state: StateAB) => state.b, (a, b) => a + b, { + memoize: defaultMemoize, memoizeOptions: [ (a, b) => { memoizer2Calls++ @@ -404,6 +406,7 @@ describe('Customizing selectors', () => { todos => todos.map(({ id }) => id), { inputStabilityCheck: 'always', + memoize: defaultMemoize, memoizeOptions: { equalityCheck: (a, b) => false, resultEqualityCheck: (a, b) => false @@ -430,8 +433,7 @@ describe('argsMemoize and memoize', () => { const state = store.getState() const selectorDefault = createSelector( (state: RootState) => state.todos, - todos => todos.map(({ id }) => id), - { memoize: defaultMemoize } + todos => todos.map(({ id }) => id) ) const selectorDefaultParametric = createSelector( [(state: RootState, id: number) => id, (state: RootState) => state.todos], @@ -695,11 +697,11 @@ describe('argsMemoize and memoize', () => { expect(selectorDefaultParametric.dependencyRecomputations()).toBe(1) selectorDefaultParametric(store.getState(), 2) selectorDefaultParametric(store.getState(), 1) - expect(selectorDefaultParametric.recomputations()).toBe(3) - expect(selectorDefaultParametric.dependencyRecomputations()).toBe(3) + expect(selectorDefaultParametric.recomputations()).toBe(2) + expect(selectorDefaultParametric.dependencyRecomputations()).toBe(2) selectorDefaultParametric(store.getState(), 2) - expect(selectorDefaultParametric.recomputations()).toBe(4) - expect(selectorDefaultParametric.dependencyRecomputations()).toBe(4) + expect(selectorDefaultParametric.recomputations()).toBe(2) + expect(selectorDefaultParametric.dependencyRecomputations()).toBe(2) const selectorDefaultParametricArgsWeakMap = createSelector( [(state: RootState, id: number) => id, (state: RootState) => state.todos], (id, todos) => todos.filter(todo => todo.id === id), @@ -748,8 +750,8 @@ describe('argsMemoize and memoize', () => { selectorDefaultParametric(store.getState(), 4) selectorDefaultParametric(store.getState(), 5) } - expect(selectorDefaultParametric.recomputations()).toBe(54) - expect(selectorDefaultParametric.dependencyRecomputations()).toBe(54) + expect(selectorDefaultParametric.recomputations()).toBe(5) + expect(selectorDefaultParametric.dependencyRecomputations()).toBe(5) for (let i = 0; i < 10; i++) { selectorDefaultParametricWeakMap(store.getState(), 1) selectorDefaultParametricWeakMap(store.getState(), 2) @@ -758,7 +760,7 @@ describe('argsMemoize and memoize', () => { selectorDefaultParametricWeakMap(store.getState(), 5) } expect(selectorDefaultParametricWeakMap.recomputations()).toBe(5) - expect(selectorDefaultParametricWeakMap.dependencyRecomputations()).toBe(50) + expect(selectorDefaultParametricWeakMap.dependencyRecomputations()).toBe(5) }) localTest('passing argsMemoize to createSelectorCreator', ({ store }) => { diff --git a/test/selectorUtils.spec.ts b/test/selectorUtils.spec.ts index 78daee563..c493cc71e 100644 --- a/test/selectorUtils.spec.ts +++ b/test/selectorUtils.spec.ts @@ -1,11 +1,12 @@ -import { createSelector } from 'reselect' +import { createSelector, defaultMemoize } from 'reselect' import type { StateA, StateAB } from 'testTypes' describe('createSelector exposed utils', () => { test('resetRecomputations', () => { const selector = createSelector( (state: StateA) => state.a, - a => a + a => a, + { memoize: defaultMemoize, argsMemoize: defaultMemoize } ) expect(selector({ a: 1 })).toBe(1) expect(selector({ a: 1 })).toBe(1) diff --git a/test/weakmapMemoize.spec.ts b/test/weakmapMemoize.spec.ts index 999d36248..bf050429a 100644 --- a/test/weakmapMemoize.spec.ts +++ b/test/weakmapMemoize.spec.ts @@ -112,8 +112,8 @@ describe('Basic selector behavior with weakMapMemoize', () => { expect(selector(state1)).toBe(3) expect(selector.recomputations()).toBe(1) - // Expected a million calls to a selector with the same arguments to take less than 1 second - expect(totalTime).toBeLessThan(1000) + // Expected a million calls to a selector with the same arguments to take less than 1.5 seconds + expect(totalTime).toBeLessThan(1500) }) test('basic selector cache hit performance for state changes but shallowly equal selector args', () => {