From 7c9af6543cbc1cd77967e017a126a7941346579b Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 25 Mar 2025 12:08:45 +0000 Subject: [PATCH 1/5] add cache to injected slices --- packages/toolkit/src/createSlice.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/toolkit/src/createSlice.ts b/packages/toolkit/src/createSlice.ts index 1d4f3e3712..226d6cb8c8 100644 --- a/packages/toolkit/src/createSlice.ts +++ b/packages/toolkit/src/createSlice.ts @@ -732,6 +732,8 @@ export function buildCreateSlice({ creators }: BuildCreateSliceConfig = {}) { > >() + const injectedStateCache = new WeakMap<(rootState: any) => State, State>() + let _reducer: ReducerWithInitialState function reducer(state: State | undefined, action: UnknownAction) { @@ -757,7 +759,11 @@ export function buildCreateSlice({ creators }: BuildCreateSliceConfig = {}) { let sliceState = state[reducerPath] if (typeof sliceState === 'undefined') { if (injected) { - sliceState = getInitialState() + sliceState = getOrInsertComputed( + injectedStateCache, + selectSlice, + getInitialState, + ) } else if (process.env.NODE_ENV !== 'production') { throw new Error( 'selectSlice returned undefined for an uninjected slice reducer', @@ -766,6 +772,7 @@ export function buildCreateSlice({ creators }: BuildCreateSliceConfig = {}) { } return sliceState } + function getSelectors( selectState: (rootState: any) => State = selectSelf, ) { @@ -783,7 +790,12 @@ export function buildCreateSlice({ creators }: BuildCreateSliceConfig = {}) { map[name] = wrapSelector( selector, selectState, - getInitialState, + () => + getOrInsertComputed( + injectedStateCache, + selectState, + getInitialState, + ), injected, ) } From 2c95e3e38012e41ef7333c73fd98ca8914260b08 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 25 Mar 2025 12:24:51 +0000 Subject: [PATCH 2/5] add cache to combineSlices --- packages/toolkit/src/combineSlices.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/toolkit/src/combineSlices.ts b/packages/toolkit/src/combineSlices.ts index a5353af937..6e50a55176 100644 --- a/packages/toolkit/src/combineSlices.ts +++ b/packages/toolkit/src/combineSlices.ts @@ -323,6 +323,7 @@ const stateProxyMap = new WeakMap() const createStateProxy = ( state: State, reducerMap: Partial>, + initialStateCache: Record, ) => getOrInsertComputed( stateProxyMap, @@ -332,20 +333,24 @@ const createStateProxy = ( get: (target, prop, receiver) => { if (prop === ORIGINAL_STATE) return target const result = Reflect.get(target, prop, receiver) + const propString = prop.toString() if (typeof result === 'undefined') { - const reducer = reducerMap[prop.toString()] + const cached = initialStateCache[propString] + if (typeof cached !== 'undefined') return cached + const reducer = reducerMap[propString] if (reducer) { // ensure action type is random, to prevent reducer treating it differently const reducerResult = reducer(undefined, { type: nanoid() }) if (typeof reducerResult === 'undefined') { throw new Error( - `The slice reducer for key "${prop.toString()}" returned undefined when called for selector(). ` + + `The slice reducer for key "${propString}" returned undefined when called for selector(). ` + `If the state passed to the reducer is undefined, you must ` + `explicitly return the initial state. The initial state may ` + `not be undefined. If you don't want to set a value for this reducer, ` + `you can use null instead of undefined.`, ) } + initialStateCache[propString] = reducerResult return reducerResult } } @@ -382,6 +387,8 @@ export function combineSlices>( combinedReducer.withLazyLoadedSlices = () => combinedReducer + const initialStateCache: Record = {} + const inject = ( slice: AnySliceLike, config: InjectConfig = {}, @@ -406,6 +413,10 @@ export function combineSlices>( return combinedReducer } + if (config.overrideExisting && currentReducer !== reducerToInject) { + delete initialStateCache[reducerPath] + } + reducerMap[reducerPath] = reducerToInject reducer = getReducer() @@ -423,6 +434,7 @@ export function combineSlices>( createStateProxy( selectState ? selectState(state as any, ...args) : state, reducerMap, + initialStateCache, ), ...args, ) From e0f91cc6fec41c648da932528a2f6dd0f34a97e6 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 25 Mar 2025 12:31:59 +0000 Subject: [PATCH 3/5] avoid calling prop.toString so much --- packages/toolkit/src/combineSlices.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/toolkit/src/combineSlices.ts b/packages/toolkit/src/combineSlices.ts index 6e50a55176..aedfdac70e 100644 --- a/packages/toolkit/src/combineSlices.ts +++ b/packages/toolkit/src/combineSlices.ts @@ -322,8 +322,8 @@ const stateProxyMap = new WeakMap() const createStateProxy = ( state: State, - reducerMap: Partial>, - initialStateCache: Record, + reducerMap: Partial>, + initialStateCache: Record, ) => getOrInsertComputed( stateProxyMap, @@ -333,24 +333,23 @@ const createStateProxy = ( get: (target, prop, receiver) => { if (prop === ORIGINAL_STATE) return target const result = Reflect.get(target, prop, receiver) - const propString = prop.toString() if (typeof result === 'undefined') { - const cached = initialStateCache[propString] + const cached = initialStateCache[prop] if (typeof cached !== 'undefined') return cached - const reducer = reducerMap[propString] + const reducer = reducerMap[prop] if (reducer) { // ensure action type is random, to prevent reducer treating it differently const reducerResult = reducer(undefined, { type: nanoid() }) if (typeof reducerResult === 'undefined') { throw new Error( - `The slice reducer for key "${propString}" returned undefined when called for selector(). ` + + `The slice reducer for key "${prop.toString()}" returned undefined when called for selector(). ` + `If the state passed to the reducer is undefined, you must ` + `explicitly return the initial state. The initial state may ` + `not be undefined. If you don't want to set a value for this reducer, ` + `you can use null instead of undefined.`, ) } - initialStateCache[propString] = reducerResult + initialStateCache[prop] = reducerResult return reducerResult } } @@ -366,7 +365,8 @@ const original = (state: any) => { return state[ORIGINAL_STATE] } -const noopReducer: Reducer> = (state = {}) => state +const emptyObject = {} +const noopReducer: Reducer> = (state = emptyObject) => state export function combineSlices>( ...slices: Slices @@ -387,7 +387,7 @@ export function combineSlices>( combinedReducer.withLazyLoadedSlices = () => combinedReducer - const initialStateCache: Record = {} + const initialStateCache: Record = {} const inject = ( slice: AnySliceLike, From 493cc77eccfcb68c787d2413b90062605495344b Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 25 Mar 2025 17:28:09 +0000 Subject: [PATCH 4/5] add tests for createSlice cache --- .../toolkit/src/tests/createSlice.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/toolkit/src/tests/createSlice.test.ts b/packages/toolkit/src/tests/createSlice.test.ts index 44312a64c2..248e7c71fe 100644 --- a/packages/toolkit/src/tests/createSlice.test.ts +++ b/packages/toolkit/src/tests/createSlice.test.ts @@ -644,6 +644,38 @@ describe('createSlice', () => { // these should be different expect(injected.selectors).not.toBe(injected2.selectors) }) + it('caches initial states for selectors', () => { + const slice = createSlice({ + name: 'counter', + initialState: () => ({ value: 0 }), + reducers: {}, + selectors: { + selectObj: (state) => state, + }, + }) + // not cached + expect(slice.getInitialState()).not.toBe(slice.getInitialState()) + expect(slice.reducer(undefined, { type: 'dummy' })).not.toBe( + slice.reducer(undefined, { type: 'dummy' }), + ) + + const combinedReducer = combineSlices({ + static: slice.reducer, + }).withLazyLoadedSlices>() + + const injected = slice.injectInto(combinedReducer) + + // still not cached + expect(injected.getInitialState()).not.toBe(injected.getInitialState()) + expect(injected.reducer(undefined, { type: 'dummy' })).not.toBe( + injected.reducer(undefined, { type: 'dummy' }), + ) + // cached + expect(injected.selectSlice({})).toBe(injected.selectSlice({})) + expect(injected.selectors.selectObj({})).toBe( + injected.selectors.selectObj({}), + ) + }) }) describe('reducers definition with asyncThunks', () => { it('is disabled by default', () => { From ed07d4cb5d6e416deab1dc3d4c073eceec4360f2 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Tue, 25 Mar 2025 17:37:42 +0000 Subject: [PATCH 5/5] add test for combineSlices cache --- .../toolkit/src/tests/combineSlices.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/toolkit/src/tests/combineSlices.test.ts b/packages/toolkit/src/tests/combineSlices.test.ts index 8c263a94f5..db21f4a85c 100644 --- a/packages/toolkit/src/tests/combineSlices.test.ts +++ b/packages/toolkit/src/tests/combineSlices.test.ts @@ -22,6 +22,12 @@ const numberSlice = createSlice({ const booleanReducer = createReducer(false, () => {}) +const counterReducer = createSlice({ + name: 'counter', + initialState: () => ({ value: 0 }), + reducers: {}, +}) + // mimic - we can't use RTKQ here directly const api = { reducerPath: 'api' as const, @@ -144,6 +150,7 @@ describe('combineSlices', () => { describe('selector', () => { const combinedReducer = combineSlices(stringSlice).withLazyLoadedSlices<{ boolean: boolean + counter: { value: number } }>() const uninjectedState = combinedReducer(undefined, dummyAction()) @@ -189,5 +196,20 @@ describe('combineSlices', () => { booleanReducer.getInitialState(), ) }) + it('caches initial state', () => { + const beforeInject = combinedReducer(undefined, dummyAction()) + const injectedReducer = combinedReducer.inject(counterReducer) + const selectCounter = injectedReducer.selector((state) => state.counter) + const counter = selectCounter(beforeInject) + expect(counter).toBe(selectCounter(beforeInject)) + + injectedReducer.inject( + { reducerPath: 'counter', reducer: () => ({ value: 0 }) }, + { overrideExisting: true }, + ) + const counter2 = selectCounter(beforeInject) + expect(counter2).not.toBe(counter) + expect(counter2).toBe(selectCounter(beforeInject)) + }) }) })