Skip to content

Commit

Permalink
Switch createSelector to use weakMapMemoize by default
Browse files Browse the repository at this point in the history
  • Loading branch information
markerikson committed Nov 30, 2023
1 parent 9ee488b commit fd004e9
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 25 deletions.
16 changes: 8 additions & 8 deletions src/createSelectorCreator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { defaultMemoize } from './defaultMemoize'
import { weakMapMemoize } from './weakMapMemoize'

import type {
Combiner,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
>,
Expand Down Expand Up @@ -372,7 +372,7 @@ export function createSelectorCreator<
const {
memoize,
memoizeOptions = [],
argsMemoize = defaultMemoize,
argsMemoize = weakMapMemoize,
argsMemoizeOptions = [],
inputStabilityCheck = globalStabilityCheck
} = combinedOptions
Expand Down Expand Up @@ -477,4 +477,4 @@ export function createSelectorCreator<
* @public
*/
export const createSelector =
/* #__PURE__ */ createSelectorCreator(defaultMemoize)
/* #__PURE__ */ createSelectorCreator(weakMapMemoize)
7 changes: 6 additions & 1 deletion test/defaultMemoize.spec.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
7 changes: 6 additions & 1 deletion test/inputStabilityCheck.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { createSelector, setInputStabilityCheckEnabled } from 'reselect'
import {
createSelector,
defaultMemoize,
setInputStabilityCheckEnabled
} from 'reselect'
import { shallowEqual } from 'react-redux'

describe('inputStabilityCheck', () => {
Expand Down Expand Up @@ -107,6 +111,7 @@ describe('inputStabilityCheck', () => {
[unstableInput],
({ a, b }) => a + b,
{
memoize: defaultMemoize,
memoizeOptions: {
equalityCheck: shallowEqual
}
Expand Down
4 changes: 2 additions & 2 deletions test/perfComparisons.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 11 additions & 9 deletions test/reselect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ describe('Customizing selectors', () => {
(state: StateAB) => state.b,
(a, b) => a + b,
{
memoize: defaultMemoize,
memoizeOptions: (a, b) => {
memoizer1Calls++
return a === b
Expand All @@ -361,6 +362,7 @@ describe('Customizing selectors', () => {
(state: StateAB) => state.b,
(a, b) => a + b,
{
memoize: defaultMemoize,
memoizeOptions: [
(a, b) => {
memoizer2Calls++
Expand Down Expand Up @@ -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
Expand All @@ -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],
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand All @@ -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 }) => {
Expand Down
5 changes: 3 additions & 2 deletions test/selectorUtils.spec.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
4 changes: 2 additions & 2 deletions test/weakmapMemoize.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit fd004e9

Please sign in to comment.