From 42309689c558b3b46e4c7a31e237485bfc475f50 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 6 Feb 2024 11:36:52 +0000 Subject: [PATCH 1/6] try wrapping selector factories in weakmapmemoize --- .../toolkit/src/query/core/buildSelectors.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/toolkit/src/query/core/buildSelectors.ts b/packages/toolkit/src/query/core/buildSelectors.ts index 4ca49fc3e..69e010d01 100644 --- a/packages/toolkit/src/query/core/buildSelectors.ts +++ b/packages/toolkit/src/query/core/buildSelectors.ts @@ -23,6 +23,7 @@ import { expandTagDescription } from '../endpointDefinitions' import type { InternalSerializeQueryArgs } from '../defaultSerializeQueryArgs' import { getMutationCacheKey } from './buildSlice' import { flatten } from '../utils' +import { weakMapMemoize } from 'reselect' export type SkipToken = typeof skipToken /** @@ -79,12 +80,15 @@ declare module './module' { } } +type WeakMapMemoizeKeys = Omit, ''> + type QueryResultSelectorFactory< Definition extends QueryDefinition, RootState, -> = ( +> = (( queryArg: QueryArgFrom | SkipToken, -) => (state: RootState) => QueryResultSelectorResult +) => (state: RootState) => QueryResultSelectorResult) & + WeakMapMemoizeKeys export type QueryResultSelectorResult< Definition extends QueryDefinition, @@ -93,12 +97,13 @@ export type QueryResultSelectorResult< type MutationResultSelectorFactory< Definition extends MutationDefinition, RootState, -> = ( +> = (( requestId: | string | { requestId: string | undefined; fixedCacheKey: string | undefined } | SkipToken, -) => (state: RootState) => MutationResultSelectorResult +) => (state: RootState) => MutationResultSelectorResult) & + WeakMapMemoizeKeys export type MutationResultSelectorResult< Definition extends MutationDefinition, @@ -169,7 +174,7 @@ export function buildSelectors< endpointName: string, endpointDefinition: QueryDefinition, ) { - return ((queryArgs: any) => { + return weakMapMemoize((queryArgs: any) => { const serializedArgs = serializeQueryArgs({ queryArgs, endpointDefinition, @@ -186,7 +191,7 @@ export function buildSelectors< } function buildMutationSelector() { - return ((id) => { + return weakMapMemoize((id) => { let mutationId: string | typeof skipToken if (typeof id === 'object') { mutationId = getMutationCacheKey(id) ?? skipToken From f92e8c9fd92a0338a36df237b9d2e115156e1444 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 6 Feb 2024 11:44:33 +0000 Subject: [PATCH 2/6] add test --- .../src/query/tests/buildCreateApi.test.tsx | 4 +- .../src/query/tests/buildSelector.test.ts | 40 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 packages/toolkit/src/query/tests/buildSelector.test.ts diff --git a/packages/toolkit/src/query/tests/buildCreateApi.test.tsx b/packages/toolkit/src/query/tests/buildCreateApi.test.tsx index 9a443c935..4ba1a6ba6 100644 --- a/packages/toolkit/src/query/tests/buildCreateApi.test.tsx +++ b/packages/toolkit/src/query/tests/buildCreateApi.test.tsx @@ -138,7 +138,7 @@ describe('buildCreateApi', () => { name: 'Timmy', }) - expect(memoize).toHaveBeenCalledTimes(4) + expect(memoize).toHaveBeenCalledTimes(2) memoize.mockClear() @@ -163,6 +163,6 @@ describe('buildCreateApi', () => { ) // select() + selectFromResult - expect(memoize).toHaveBeenCalledTimes(8) + expect(memoize).toHaveBeenCalledTimes(4) }) }) diff --git a/packages/toolkit/src/query/tests/buildSelector.test.ts b/packages/toolkit/src/query/tests/buildSelector.test.ts new file mode 100644 index 000000000..8291babce --- /dev/null +++ b/packages/toolkit/src/query/tests/buildSelector.test.ts @@ -0,0 +1,40 @@ +import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query/react' +import { setupApiStore } from '../../tests/utils/helpers' + +describe('buildSelector', () => { + interface Todo { + userId: number + id: number + title: string + completed: boolean + } + + type Todos = Array + + const exampleApi = createApi({ + reducerPath: 'api', + baseQuery: fetchBaseQuery({ + baseUrl: 'https://jsonplaceholder.typicode.com', + }), + endpoints: (build) => ({ + getTodos: build.query({ + query: () => '/todos', + }), + }), + }) + + const store = setupApiStore(exampleApi) + it('should memoize selector creation', () => { + expect(exampleApi.endpoints.getTodos.select('1')).toBe( + exampleApi.endpoints.getTodos.select('1'), + ) + + expect(exampleApi.endpoints.getTodos.select('1')).not.toBe( + exampleApi.endpoints.getTodos.select('2'), + ) + + expect( + exampleApi.endpoints.getTodos.select('1')(store.store.getState()), + ).toBe(exampleApi.endpoints.getTodos.select('1')(store.store.getState())) + }) +}) From 77f040b59eb79b1e9b6b52f91fc489533050296f Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 6 Feb 2024 11:49:17 +0000 Subject: [PATCH 3/6] test memoize methods --- .../src/query/tests/buildSelector.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/toolkit/src/query/tests/buildSelector.test.ts b/packages/toolkit/src/query/tests/buildSelector.test.ts index 8291babce..c43d122dc 100644 --- a/packages/toolkit/src/query/tests/buildSelector.test.ts +++ b/packages/toolkit/src/query/tests/buildSelector.test.ts @@ -37,4 +37,27 @@ describe('buildSelector', () => { exampleApi.endpoints.getTodos.select('1')(store.store.getState()), ).toBe(exampleApi.endpoints.getTodos.select('1')(store.store.getState())) }) + it('exposes memoize methods on select', () => { + expect(exampleApi.endpoints.getTodos.select.resultsCount).toBeTypeOf( + 'function', + ) + expect(exampleApi.endpoints.getTodos.select.resetResultsCount).toBeTypeOf( + 'function', + ) + expect(exampleApi.endpoints.getTodos.select.clearCache).toBeTypeOf( + 'function', + ) + + const firstResult = exampleApi.endpoints.getTodos.select('1') + + exampleApi.endpoints.getTodos.select.clearCache() + + const secondResult = exampleApi.endpoints.getTodos.select('1') + + expect(firstResult).not.toBe(secondResult) + + expect(firstResult(store.store.getState())).not.toBe( + secondResult(store.store.getState()), + ) + }) }) From 916cc76683ef66a650393d704bbd93c07d19bd8e Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 6 Feb 2024 11:55:03 +0000 Subject: [PATCH 4/6] make sure select() === select(undefined) --- .../toolkit/src/query/core/buildSelectors.ts | 16 ++++++--- .../src/query/tests/buildSelector.test.ts | 33 +++++++++++-------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/packages/toolkit/src/query/core/buildSelectors.ts b/packages/toolkit/src/query/core/buildSelectors.ts index 69e010d01..e4e2f9e7c 100644 --- a/packages/toolkit/src/query/core/buildSelectors.ts +++ b/packages/toolkit/src/query/core/buildSelectors.ts @@ -174,7 +174,7 @@ export function buildSelectors< endpointName: string, endpointDefinition: QueryDefinition, ) { - return weakMapMemoize((queryArgs: any) => { + const memoized = weakMapMemoize((queryArgs: any) => { const serializedArgs = serializeQueryArgs({ queryArgs, endpointDefinition, @@ -187,11 +187,15 @@ export function buildSelectors< queryArgs === skipToken ? selectSkippedQuery : selectQuerySubstate return createSelector(finalSelectQuerySubState, withRequestFlags) - }) as QueryResultSelectorFactory + }) + return Object.assign( + (arg: any) => memoized(arg), + memoized, + ) as QueryResultSelectorFactory } function buildMutationSelector() { - return weakMapMemoize((id) => { + const memoized = weakMapMemoize((id) => { let mutationId: string | typeof skipToken if (typeof id === 'object') { mutationId = getMutationCacheKey(id) ?? skipToken @@ -207,7 +211,11 @@ export function buildSelectors< : selectMutationSubstate return createSelector(finalSelectMutationSubstate, withRequestFlags) - }) as MutationResultSelectorFactory + }) + return Object.assign( + (arg: any) => memoized(arg), + memoized, + ) as MutationResultSelectorFactory } function selectInvalidatedBy( diff --git a/packages/toolkit/src/query/tests/buildSelector.test.ts b/packages/toolkit/src/query/tests/buildSelector.test.ts index c43d122dc..2152e9960 100644 --- a/packages/toolkit/src/query/tests/buildSelector.test.ts +++ b/packages/toolkit/src/query/tests/buildSelector.test.ts @@ -17,42 +17,49 @@ describe('buildSelector', () => { baseUrl: 'https://jsonplaceholder.typicode.com', }), endpoints: (build) => ({ - getTodos: build.query({ + getTodos: build.query({ query: () => '/todos', }), + getTodo: build.query({ + query: (id) => `/todos/${id}`, + }), }), }) const store = setupApiStore(exampleApi) it('should memoize selector creation', () => { - expect(exampleApi.endpoints.getTodos.select('1')).toBe( - exampleApi.endpoints.getTodos.select('1'), + expect(exampleApi.endpoints.getTodo.select('1')).toBe( + exampleApi.endpoints.getTodo.select('1'), ) - expect(exampleApi.endpoints.getTodos.select('1')).not.toBe( - exampleApi.endpoints.getTodos.select('2'), + expect(exampleApi.endpoints.getTodo.select('1')).not.toBe( + exampleApi.endpoints.getTodo.select('2'), ) expect( - exampleApi.endpoints.getTodos.select('1')(store.store.getState()), - ).toBe(exampleApi.endpoints.getTodos.select('1')(store.store.getState())) + exampleApi.endpoints.getTodo.select('1')(store.store.getState()), + ).toBe(exampleApi.endpoints.getTodo.select('1')(store.store.getState())) + + expect(exampleApi.endpoints.getTodos.select()).toBe( + exampleApi.endpoints.getTodos.select(undefined), + ) }) it('exposes memoize methods on select', () => { - expect(exampleApi.endpoints.getTodos.select.resultsCount).toBeTypeOf( + expect(exampleApi.endpoints.getTodo.select.resultsCount).toBeTypeOf( 'function', ) - expect(exampleApi.endpoints.getTodos.select.resetResultsCount).toBeTypeOf( + expect(exampleApi.endpoints.getTodo.select.resetResultsCount).toBeTypeOf( 'function', ) - expect(exampleApi.endpoints.getTodos.select.clearCache).toBeTypeOf( + expect(exampleApi.endpoints.getTodo.select.clearCache).toBeTypeOf( 'function', ) - const firstResult = exampleApi.endpoints.getTodos.select('1') + const firstResult = exampleApi.endpoints.getTodo.select('1') - exampleApi.endpoints.getTodos.select.clearCache() + exampleApi.endpoints.getTodo.select.clearCache() - const secondResult = exampleApi.endpoints.getTodos.select('1') + const secondResult = exampleApi.endpoints.getTodo.select('1') expect(firstResult).not.toBe(secondResult) From 978a067520befcdf6671c504719207423abf4dd8 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 6 Feb 2024 12:15:15 +0000 Subject: [PATCH 5/6] use memoize function from createSelector instance --- .../toolkit/src/query/core/buildSelectors.ts | 22 +++---- .../src/query/tests/buildCreateApi.test.tsx | 2 +- .../src/query/tests/buildSelector.test.ts | 60 ++++++++++++++++++- 3 files changed, 69 insertions(+), 15 deletions(-) diff --git a/packages/toolkit/src/query/core/buildSelectors.ts b/packages/toolkit/src/query/core/buildSelectors.ts index e4e2f9e7c..73cb73ad9 100644 --- a/packages/toolkit/src/query/core/buildSelectors.ts +++ b/packages/toolkit/src/query/core/buildSelectors.ts @@ -23,7 +23,6 @@ import { expandTagDescription } from '../endpointDefinitions' import type { InternalSerializeQueryArgs } from '../defaultSerializeQueryArgs' import { getMutationCacheKey } from './buildSlice' import { flatten } from '../utils' -import { weakMapMemoize } from 'reselect' export type SkipToken = typeof skipToken /** @@ -80,15 +79,12 @@ declare module './module' { } } -type WeakMapMemoizeKeys = Omit, ''> - type QueryResultSelectorFactory< Definition extends QueryDefinition, RootState, -> = (( +> = ( queryArg: QueryArgFrom | SkipToken, -) => (state: RootState) => QueryResultSelectorResult) & - WeakMapMemoizeKeys +) => (state: RootState) => QueryResultSelectorResult export type QueryResultSelectorResult< Definition extends QueryDefinition, @@ -97,13 +93,12 @@ export type QueryResultSelectorResult< type MutationResultSelectorFactory< Definition extends MutationDefinition, RootState, -> = (( +> = ( requestId: | string | { requestId: string | undefined; fixedCacheKey: string | undefined } | SkipToken, -) => (state: RootState) => MutationResultSelectorResult) & - WeakMapMemoizeKeys +) => (state: RootState) => MutationResultSelectorResult export type MutationResultSelectorResult< Definition extends MutationDefinition, @@ -140,6 +135,11 @@ export function buildSelectors< const selectSkippedQuery = (state: RootState) => defaultQuerySubState const selectSkippedMutation = (state: RootState) => defaultMutationSubState + const aMemoizedSelector = createSelector( + () => {}, + () => ({}), + ) + return { buildQuerySelector, buildMutationSelector, @@ -174,7 +174,7 @@ export function buildSelectors< endpointName: string, endpointDefinition: QueryDefinition, ) { - const memoized = weakMapMemoize((queryArgs: any) => { + const memoized = aMemoizedSelector.memoize((queryArgs: any) => { const serializedArgs = serializeQueryArgs({ queryArgs, endpointDefinition, @@ -195,7 +195,7 @@ export function buildSelectors< } function buildMutationSelector() { - const memoized = weakMapMemoize((id) => { + const memoized = aMemoizedSelector.memoize((id) => { let mutationId: string | typeof skipToken if (typeof id === 'object') { mutationId = getMutationCacheKey(id) ?? skipToken diff --git a/packages/toolkit/src/query/tests/buildCreateApi.test.tsx b/packages/toolkit/src/query/tests/buildCreateApi.test.tsx index 4ba1a6ba6..8b9e978b3 100644 --- a/packages/toolkit/src/query/tests/buildCreateApi.test.tsx +++ b/packages/toolkit/src/query/tests/buildCreateApi.test.tsx @@ -138,7 +138,7 @@ describe('buildCreateApi', () => { name: 'Timmy', }) - expect(memoize).toHaveBeenCalledTimes(2) + expect(memoize).toHaveBeenCalledTimes(3) memoize.mockClear() diff --git a/packages/toolkit/src/query/tests/buildSelector.test.ts b/packages/toolkit/src/query/tests/buildSelector.test.ts index 2152e9960..ea6f12db0 100644 --- a/packages/toolkit/src/query/tests/buildSelector.test.ts +++ b/packages/toolkit/src/query/tests/buildSelector.test.ts @@ -1,5 +1,17 @@ -import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query/react' +import { + buildCreateApi, + coreModule, + createApi, + fetchBaseQuery, + reactHooksModule, +} from '@reduxjs/toolkit/query/react' import { setupApiStore } from '../../tests/utils/helpers' +import { + createSelectorCreator, + lruMemoize, + type weakMapMemoize, +} from 'reselect' +import { assertCast } from '../tsHelpers' describe('buildSelector', () => { interface Todo { @@ -12,7 +24,6 @@ describe('buildSelector', () => { type Todos = Array const exampleApi = createApi({ - reducerPath: 'api', baseQuery: fetchBaseQuery({ baseUrl: 'https://jsonplaceholder.typicode.com', }), @@ -44,7 +55,12 @@ describe('buildSelector', () => { exampleApi.endpoints.getTodos.select(undefined), ) }) - it('exposes memoize methods on select', () => { + it('exposes memoize methods on select (untyped)', () => { + assertCast< + typeof exampleApi.endpoints.getTodo.select & + Omit, ''> + >(exampleApi.endpoints.getTodo.select) + expect(exampleApi.endpoints.getTodo.select.resultsCount).toBeTypeOf( 'function', ) @@ -67,4 +83,42 @@ describe('buildSelector', () => { secondResult(store.store.getState()), ) }) + it('uses createSelector instance memoize', () => { + const createLruSelector = createSelectorCreator(lruMemoize) + const createApi = buildCreateApi( + coreModule({ createSelector: createLruSelector }), + reactHooksModule({ createSelector: createLruSelector }), + ) + + const exampleLruApi = createApi({ + baseQuery: fetchBaseQuery({ + baseUrl: 'https://jsonplaceholder.typicode.com', + }), + endpoints: (build) => ({ + getTodos: build.query({ + query: () => '/todos', + }), + getTodo: build.query({ + query: (id) => `/todos/${id}`, + }), + }), + }) + + expect(exampleLruApi.endpoints.getTodo.select('1')).toBe( + exampleLruApi.endpoints.getTodo.select('1'), + ) + + expect(exampleLruApi.endpoints.getTodo.select('1')).not.toBe( + exampleLruApi.endpoints.getTodo.select('2'), + ) + + const firstResult = exampleLruApi.endpoints.getTodo.select('1') + + const secondResult = exampleLruApi.endpoints.getTodo.select('2') + + const thirdResult = exampleLruApi.endpoints.getTodo.select('1') + + // cache size of 1 + expect(firstResult).not.toBe(thirdResult) + }) }) From 7abfe3268f7ded46a04354695887d9d384c26b6a Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 6 Feb 2024 12:19:46 +0000 Subject: [PATCH 6/6] tweak the number again --- packages/toolkit/src/query/tests/buildCreateApi.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/toolkit/src/query/tests/buildCreateApi.test.tsx b/packages/toolkit/src/query/tests/buildCreateApi.test.tsx index 8b9e978b3..aebd6d1a1 100644 --- a/packages/toolkit/src/query/tests/buildCreateApi.test.tsx +++ b/packages/toolkit/src/query/tests/buildCreateApi.test.tsx @@ -138,7 +138,7 @@ describe('buildCreateApi', () => { name: 'Timmy', }) - expect(memoize).toHaveBeenCalledTimes(3) + expect(memoize).toHaveBeenCalledTimes(4) memoize.mockClear()