From cdc1cc233ef698832454bb415630bed266063ae5 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Sun, 21 Aug 2022 11:24:21 +0200 Subject: [PATCH 1/9] add autoBatchEnhancer --- packages/toolkit/src/autoBatchEnhancer.ts | 47 ++++++++++ packages/toolkit/src/configureStore.ts | 6 +- packages/toolkit/src/index.ts | 1 + .../src/query/core/buildMiddleware/index.ts | 6 +- packages/toolkit/src/query/core/buildSlice.ts | 89 +++++++++++-------- .../toolkit/src/query/core/buildThunks.ts | 16 ++-- packages/toolkit/src/query/react/module.ts | 3 +- packages/toolkit/src/query/tests/helpers.tsx | 11 ++- 8 files changed, 128 insertions(+), 51 deletions(-) create mode 100644 packages/toolkit/src/autoBatchEnhancer.ts diff --git a/packages/toolkit/src/autoBatchEnhancer.ts b/packages/toolkit/src/autoBatchEnhancer.ts new file mode 100644 index 0000000000..ee2a27f86b --- /dev/null +++ b/packages/toolkit/src/autoBatchEnhancer.ts @@ -0,0 +1,47 @@ +import type { StoreEnhancer } from 'redux' + +export const autoBatch = 'ReduxToolkit_autoBatch' + +export const autoBatchEnhancer = + (batchTimeout: number = 0): StoreEnhancer => + (next) => + (...args) => { + const store = next(...args) + + let notifying = true + let nextNotification: NodeJS.Timeout | undefined = undefined + const listeners = new Set<() => void>() + const notifyListeners = () => { + nextNotification = void listeners.forEach((l) => l()) + } + + return Object.assign({}, store, { + subscribe(listener: () => void) { + const wrappedListener: typeof listener = () => notifying && listener() + const unsubscribe = store.subscribe(wrappedListener) + listeners.add(listener) + return () => { + unsubscribe() + listeners.delete(listener) + } + }, + dispatch(action: any) { + try { + notifying = !action?.meta?.[autoBatch] + if (notifying) { + if (nextNotification) { + nextNotification = void clearTimeout(nextNotification) + } + } else { + nextNotification ||= setTimeout( + notifyListeners, + batchTimeout + ) as any + } + return store.dispatch(action) + } finally { + notifying = true + } + }, + }) + } diff --git a/packages/toolkit/src/configureStore.ts b/packages/toolkit/src/configureStore.ts index 7b4c0b3534..aea39e660e 100644 --- a/packages/toolkit/src/configureStore.ts +++ b/packages/toolkit/src/configureStore.ts @@ -34,7 +34,7 @@ const IS_PRODUCTION = process.env.NODE_ENV === 'production' * @public */ export type ConfigureEnhancersCallback = ( - defaultEnhancers: readonly StoreEnhancer[] + defaultEnhancers: readonly StoreEnhancer[] ) => [...E] /** @@ -104,10 +104,10 @@ type Middlewares = ReadonlyArray> type Enhancers = ReadonlyArray -interface ToolkitStore< +export interface ToolkitStore< S = any, A extends Action = AnyAction, - M extends Middlewares = Middlewares, + M extends Middlewares = Middlewares > extends Store { /** * The `dispatch` method of your store, enhanced by all its middlewares. diff --git a/packages/toolkit/src/index.ts b/packages/toolkit/src/index.ts index 580a3412d4..85f9a35032 100644 --- a/packages/toolkit/src/index.ts +++ b/packages/toolkit/src/index.ts @@ -17,6 +17,7 @@ export type { } from 'reselect' export { createDraftSafeSelector } from './createDraftSafeSelector' export type { ThunkAction, ThunkDispatch, ThunkMiddleware } from 'redux-thunk' +export { autoBatch, autoBatchEnhancer } from './autoBatchEnhancer' // We deliberately enable Immer's ES5 support, on the grounds that // we assume RTK will be used with React Native and other Proxy-less diff --git a/packages/toolkit/src/query/core/buildMiddleware/index.ts b/packages/toolkit/src/query/core/buildMiddleware/index.ts index 09f2412e12..2729bf77f6 100644 --- a/packages/toolkit/src/query/core/buildMiddleware/index.ts +++ b/packages/toolkit/src/query/core/buildMiddleware/index.ts @@ -79,9 +79,9 @@ export function buildMiddleware< const stateBefore = mwApi.getState() - if (!batchedActionsHandler(action, mwApi, stateBefore)) { - return - } + // if (!batchedActionsHandler(action, mwApi, stateBefore)) { + // return + // } const res = next(action) diff --git a/packages/toolkit/src/query/core/buildSlice.ts b/packages/toolkit/src/query/core/buildSlice.ts index 195c0863e3..f28ec77a11 100644 --- a/packages/toolkit/src/query/core/buildSlice.ts +++ b/packages/toolkit/src/query/core/buildSlice.ts @@ -1,5 +1,6 @@ import type { AnyAction, PayloadAction } from '@reduxjs/toolkit' import { + autoBatch, combineReducers, createAction, createSlice, @@ -85,6 +86,12 @@ function updateMutationSubstateIfExists( } const initialState = {} as any +const prepareAutoBatched = + () => + (payload: T): { payload: T; meta: unknown } => ({ + payload, + meta: { [autoBatch]: true }, + }) export function buildSlice({ reducerPath, @@ -114,11 +121,14 @@ export function buildSlice({ name: `${reducerPath}/queries`, initialState: initialState as QueryState, reducers: { - removeQueryResult( - draft, - { payload: { queryCacheKey } }: PayloadAction - ) { - delete draft[queryCacheKey] + removeQueryResult: { + reducer( + draft, + { payload: { queryCacheKey } }: PayloadAction + ) { + delete draft[queryCacheKey] + }, + prepare: prepareAutoBatched(), }, queryResultPatched( draft, @@ -243,14 +253,14 @@ export function buildSlice({ name: `${reducerPath}/mutations`, initialState: initialState as MutationState, reducers: { - removeMutationResult( - draft, - { payload }: PayloadAction - ) { - const cacheKey = getMutationCacheKey(payload) - if (cacheKey in draft) { - delete draft[cacheKey] - } + removeMutationResult: { + reducer(draft, { payload }: PayloadAction) { + const cacheKey = getMutationCacheKey(payload) + if (cacheKey in draft) { + delete draft[cacheKey] + } + }, + prepare: prepareAutoBatched(), }, }, extraReducers(builder) { @@ -369,35 +379,42 @@ export function buildSlice({ }, }) + type SubscriptionUpdate = { + endpointName: string + requestId: string + options: Subscribers[number] + } & QuerySubstateIdentifier const subscriptionSlice = createSlice({ name: `${reducerPath}/subscriptions`, initialState: initialState as SubscriptionState, reducers: { - updateSubscriptionOptions( - draft, - { - payload: { queryCacheKey, requestId, options }, - }: PayloadAction< + updateSubscriptionOptions: { + reducer( + draft, { - endpointName: string - requestId: string - options: Subscribers[number] - } & QuerySubstateIdentifier - > - ) { - if (draft?.[queryCacheKey]?.[requestId]) { - draft[queryCacheKey]![requestId] = options - } + payload: { queryCacheKey, requestId, options }, + }: PayloadAction + ) { + if (draft?.[queryCacheKey]?.[requestId]) { + draft[queryCacheKey]![requestId] = options + } + }, + prepare: prepareAutoBatched(), }, - unsubscribeQueryResult( - draft, - { - payload: { queryCacheKey, requestId }, - }: PayloadAction<{ requestId: string } & QuerySubstateIdentifier> - ) { - if (draft[queryCacheKey]) { - delete draft[queryCacheKey]![requestId] - } + unsubscribeQueryResult: { + reducer( + draft, + { + payload: { queryCacheKey, requestId }, + }: PayloadAction<{ requestId: string } & QuerySubstateIdentifier> + ) { + if (draft[queryCacheKey]) { + delete draft[queryCacheKey]![requestId] + } + }, + prepare: prepareAutoBatched< + { requestId: string } & QuerySubstateIdentifier + >(), }, subscriptionRequestsRejected( draft, diff --git a/packages/toolkit/src/query/core/buildThunks.ts b/packages/toolkit/src/query/core/buildThunks.ts index d6a111b194..d78be76ce7 100644 --- a/packages/toolkit/src/query/core/buildThunks.ts +++ b/packages/toolkit/src/query/core/buildThunks.ts @@ -38,7 +38,7 @@ import type { ThunkDispatch, AsyncThunk, } from '@reduxjs/toolkit' -import { createAsyncThunk } from '@reduxjs/toolkit' +import { createAsyncThunk, autoBatch } from '@reduxjs/toolkit' import { HandledError } from '../HandledError' @@ -122,13 +122,18 @@ export interface MutationThunkArg { export type ThunkResult = unknown export type ThunkApiMetaConfig = { - pendingMeta: { startedTimeStamp: number } + pendingMeta: { + startedTimeStamp: number + [autoBatch]: true + } fulfilledMeta: { fulfilledTimeStamp: number baseQueryMeta: unknown + [autoBatch]: true } rejectedMeta: { baseQueryMeta: unknown + [autoBatch]: true } } export type QueryThunk = AsyncThunk< @@ -398,6 +403,7 @@ export function buildThunks< { fulfilledTimeStamp: Date.now(), baseQueryMeta: result.meta, + [autoBatch]: true, } ) } catch (error) { @@ -422,7 +428,7 @@ export function buildThunks< catchedError.meta, arg.originalArgs ), - { baseQueryMeta: catchedError.meta } + { baseQueryMeta: catchedError.meta, [autoBatch]: true } ) } catch (e) { catchedError = e @@ -472,7 +478,7 @@ In the case of an unhandled error, no tags will be "provided" or "invalidated".` ThunkApiMetaConfig & { state: RootState } >(`${reducerPath}/executeQuery`, executeEndpoint, { getPendingMeta() { - return { startedTimeStamp: Date.now() } + return { startedTimeStamp: Date.now(), [autoBatch]: true } }, condition(arg, { getState }) { const state = getState() @@ -506,7 +512,7 @@ In the case of an unhandled error, no tags will be "provided" or "invalidated".` ThunkApiMetaConfig & { state: RootState } >(`${reducerPath}/executeMutation`, executeEndpoint, { getPendingMeta() { - return { startedTimeStamp: Date.now() } + return { startedTimeStamp: Date.now(), [autoBatch]: true } }, }) diff --git a/packages/toolkit/src/query/react/module.ts b/packages/toolkit/src/query/react/module.ts index 5028c64fe7..ec14050677 100644 --- a/packages/toolkit/src/query/react/module.ts +++ b/packages/toolkit/src/query/react/module.ts @@ -150,7 +150,8 @@ export const reactHooksModule = ({ context, }) safeAssign(anyApi, { usePrefetch }) - safeAssign(context, { batch }) + // even with React batching completely out of the picture, we should get similar results now + // safeAssign(context, { batch }) return { injectEndpoint(endpointName, definition) { diff --git a/packages/toolkit/src/query/tests/helpers.tsx b/packages/toolkit/src/query/tests/helpers.tsx index 61581e91df..843d86ffb8 100644 --- a/packages/toolkit/src/query/tests/helpers.tsx +++ b/packages/toolkit/src/query/tests/helpers.tsx @@ -4,7 +4,7 @@ import type { Middleware, Store, } from '@reduxjs/toolkit' -import { configureStore } from '@reduxjs/toolkit' +import { configureStore, autoBatchEnhancer } from '@reduxjs/toolkit' import { setupListeners } from '@reduxjs/toolkit/query' import type { Reducer } from 'react' import React, { useCallback } from 'react' @@ -176,9 +176,13 @@ export function setupApiStore< >( api: A, extraReducers?: R, - options: { withoutListeners?: boolean; withoutTestLifecycles?: boolean, middleware?: Middleware[] } = {} + options: { + withoutListeners?: boolean + withoutTestLifecycles?: boolean + middleware?: Middleware[] + } = {} ) { - const { middleware = [] } = options; + const { middleware = [] } = options const getStore = () => configureStore({ reducer: { api: api.reducer, ...extraReducers }, @@ -187,6 +191,7 @@ export function setupApiStore< api.middleware, ...middleware ), + enhancers: (e) => e.concat(autoBatchEnhancer(0)), }) type StoreType = EnhancedStore< From 1f803f9426e0532ad80a75aaf06fca7cd27aa7bd Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 23 Aug 2022 05:05:21 +0200 Subject: [PATCH 2/9] also apply to condition rejection action (hacky) --- packages/toolkit/src/createAsyncThunk.ts | 4 ++++ packages/toolkit/src/query/core/buildSlice.ts | 12 ++++++++++ .../toolkit/src/query/tests/cleanup.test.tsx | 22 +++++++++++++++++-- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/toolkit/src/createAsyncThunk.ts b/packages/toolkit/src/createAsyncThunk.ts index 017d740e5e..e6d1398b95 100644 --- a/packages/toolkit/src/createAsyncThunk.ts +++ b/packages/toolkit/src/createAsyncThunk.ts @@ -547,6 +547,10 @@ export const createAsyncThunk = (() => { requestStatus: 'rejected' as const, aborted: error?.name === 'AbortError', condition: error?.name === 'ConditionError', + // TODO: this is a hack to showcase the autobatching behaviour + // currently there is no way to add `meta` to a "condition rejected" + // action - that would need to be added + ReduxToolkit_autoBatch: true, }, }) ) diff --git a/packages/toolkit/src/query/core/buildSlice.ts b/packages/toolkit/src/query/core/buildSlice.ts index f28ec77a11..9379771e1c 100644 --- a/packages/toolkit/src/query/core/buildSlice.ts +++ b/packages/toolkit/src/query/core/buildSlice.ts @@ -452,6 +452,18 @@ export function buildSlice({ arg.subscriptionOptions ?? substate[requestId] ?? {} } }) + // original case added back in as otherwise nothing would happen without the batching middleware + .addCase( + queryThunk.rejected, + (draft, { meta: { condition, arg, requestId }, error, payload }) => { + // request was aborted due to condition (another query already running) + if (condition && arg.subscribe) { + const substate = (draft[arg.queryCacheKey] ??= {}) + substate[requestId] = + arg.subscriptionOptions ?? substate[requestId] ?? {} + } + } + ) // update the state to be a new object to be picked up as a "state change" // by redux-persist's `autoMergeLevel2` .addMatcher(hasRehydrationInfo, (draft) => ({ ...draft })) diff --git a/packages/toolkit/src/query/tests/cleanup.test.tsx b/packages/toolkit/src/query/tests/cleanup.test.tsx index e9b9b95992..96770067a6 100644 --- a/packages/toolkit/src/query/tests/cleanup.test.tsx +++ b/packages/toolkit/src/query/tests/cleanup.test.tsx @@ -155,6 +155,12 @@ test('Minimizes the number of subscription dispatches when multiple components a withoutTestLifecycles: true, }) + let subscribersTriggered = 0 + let totalComponentRenders = 0 + storeRef.store.subscribe(() => { + subscribersTriggered += 1 + }) + let getSubscriptionsA = () => storeRef.store.getState().api.subscriptions['a(undefined)'] @@ -169,7 +175,15 @@ test('Minimizes the number of subscription dispatches when multiple components a const NUM_LIST_ITEMS = 1000 + function UsingA() { + totalComponentRenders++ + const { data } = api.endpoints.a.useQuery() + + return <>Result: {data} + } + function ParentComponent() { + totalComponentRenders++ const listItems = Array.from({ length: NUM_LIST_ITEMS }).map((_, i) => ( )) @@ -181,6 +195,9 @@ test('Minimizes the number of subscription dispatches when multiple components a wrapper: storeRef.wrapper, }) + expect(totalComponentRenders).toBe(2001) + expect(subscribersTriggered).toBe(1) // 1001 without batching + jest.advanceTimersByTime(10) await waitFor(() => { @@ -188,7 +205,6 @@ test('Minimizes the number of subscription dispatches when multiple components a }) const subscriptions = getSubscriptionsA() - expect(Object.keys(subscriptions!).length).toBe(NUM_LIST_ITEMS) // Expected: [ // 'api/config/middlewareRegistered', @@ -196,5 +212,7 @@ test('Minimizes the number of subscription dispatches when multiple components a // 'api/subscriptions/subscriptionRequestsRejected', // 'api/executeQuery/fulfilled' // ] - expect(actionTypes.length).toBe(4) + expect(actionTypes.length).toBe(1002) + expect(subscribersTriggered).toBe(3) // 1002 without batching + expect(totalComponentRenders).toBe(3001) }, 25000) From 02d5d32c7b5a9c561cd3b43820da88181eb2b9af Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sun, 4 Sep 2022 10:52:13 -0400 Subject: [PATCH 3/9] WIP Temporarily revert back to `batchActions` behavior --- .../src/query/core/buildMiddleware/index.ts | 6 ++-- packages/toolkit/src/query/core/buildSlice.ts | 22 +++++++-------- packages/toolkit/src/query/react/module.ts | 2 +- .../toolkit/src/query/tests/cleanup.test.tsx | 28 ++++++++++++------- packages/toolkit/src/query/tests/helpers.tsx | 2 +- 5 files changed, 34 insertions(+), 26 deletions(-) diff --git a/packages/toolkit/src/query/core/buildMiddleware/index.ts b/packages/toolkit/src/query/core/buildMiddleware/index.ts index 2729bf77f6..09f2412e12 100644 --- a/packages/toolkit/src/query/core/buildMiddleware/index.ts +++ b/packages/toolkit/src/query/core/buildMiddleware/index.ts @@ -79,9 +79,9 @@ export function buildMiddleware< const stateBefore = mwApi.getState() - // if (!batchedActionsHandler(action, mwApi, stateBefore)) { - // return - // } + if (!batchedActionsHandler(action, mwApi, stateBefore)) { + return + } const res = next(action) diff --git a/packages/toolkit/src/query/core/buildSlice.ts b/packages/toolkit/src/query/core/buildSlice.ts index 9379771e1c..6dc5de7c87 100644 --- a/packages/toolkit/src/query/core/buildSlice.ts +++ b/packages/toolkit/src/query/core/buildSlice.ts @@ -453,17 +453,17 @@ export function buildSlice({ } }) // original case added back in as otherwise nothing would happen without the batching middleware - .addCase( - queryThunk.rejected, - (draft, { meta: { condition, arg, requestId }, error, payload }) => { - // request was aborted due to condition (another query already running) - if (condition && arg.subscribe) { - const substate = (draft[arg.queryCacheKey] ??= {}) - substate[requestId] = - arg.subscriptionOptions ?? substate[requestId] ?? {} - } - } - ) + // .addCase( + // queryThunk.rejected, + // (draft, { meta: { condition, arg, requestId }, error, payload }) => { + // // request was aborted due to condition (another query already running) + // if (condition && arg.subscribe) { + // const substate = (draft[arg.queryCacheKey] ??= {}) + // substate[requestId] = + // arg.subscriptionOptions ?? substate[requestId] ?? {} + // } + // } + // ) // update the state to be a new object to be picked up as a "state change" // by redux-persist's `autoMergeLevel2` .addMatcher(hasRehydrationInfo, (draft) => ({ ...draft })) diff --git a/packages/toolkit/src/query/react/module.ts b/packages/toolkit/src/query/react/module.ts index ec14050677..f2864840c1 100644 --- a/packages/toolkit/src/query/react/module.ts +++ b/packages/toolkit/src/query/react/module.ts @@ -151,7 +151,7 @@ export const reactHooksModule = ({ }) safeAssign(anyApi, { usePrefetch }) // even with React batching completely out of the picture, we should get similar results now - // safeAssign(context, { batch }) + safeAssign(context, { batch }) return { injectEndpoint(endpointName, definition) { diff --git a/packages/toolkit/src/query/tests/cleanup.test.tsx b/packages/toolkit/src/query/tests/cleanup.test.tsx index 96770067a6..4d0d2f4038 100644 --- a/packages/toolkit/src/query/tests/cleanup.test.tsx +++ b/packages/toolkit/src/query/tests/cleanup.test.tsx @@ -148,7 +148,7 @@ test('data stays in store when one component requiring the data stays in the sto expect(getSubStateB()).toEqual(statusB) }) -test('Minimizes the number of subscription dispatches when multiple components ask for the same data', async () => { +test.only('Minimizes the number of subscription dispatches when multiple components ask for the same data', async () => { const listenerMiddleware = createListenerMiddleware() const storeRef = setupApiStore(api, undefined, { middleware: [listenerMiddleware.middleware], @@ -156,7 +156,8 @@ test('Minimizes the number of subscription dispatches when multiple components a }) let subscribersTriggered = 0 - let totalComponentRenders = 0 + let totalListItemRenders = 0 + let totalParentComponentRenders = 0 storeRef.store.subscribe(() => { subscribersTriggered += 1 }) @@ -176,14 +177,14 @@ test('Minimizes the number of subscription dispatches when multiple components a const NUM_LIST_ITEMS = 1000 function UsingA() { - totalComponentRenders++ + totalListItemRenders++ const { data } = api.endpoints.a.useQuery() return <>Result: {data} } function ParentComponent() { - totalComponentRenders++ + totalParentComponentRenders++ const listItems = Array.from({ length: NUM_LIST_ITEMS }).map((_, i) => ( )) @@ -195,10 +196,15 @@ test('Minimizes the number of subscription dispatches when multiple components a wrapper: storeRef.wrapper, }) - expect(totalComponentRenders).toBe(2001) - expect(subscribersTriggered).toBe(1) // 1001 without batching + expect(totalListItemRenders).toBe(NUM_LIST_ITEMS * 2) + expect(totalParentComponentRenders).toBe(1) + expect(actionTypes.length).toBe(2) + expect(subscribersTriggered).toBe(2) + // expect(subscribersTriggered).toBe(1) // 1001 without batching - jest.advanceTimersByTime(10) + await act(async () => { + jest.advanceTimersByTime(10) + }) await waitFor(() => { return screen.getAllByText(/42/).length > 0 @@ -212,7 +218,9 @@ test('Minimizes the number of subscription dispatches when multiple components a // 'api/subscriptions/subscriptionRequestsRejected', // 'api/executeQuery/fulfilled' // ] - expect(actionTypes.length).toBe(1002) - expect(subscribersTriggered).toBe(3) // 1002 without batching - expect(totalComponentRenders).toBe(3001) + expect(actionTypes.length).toBe(4) + expect(subscribersTriggered).toBe(4) + // expect(actionTypes.length).toBe(1002) + // expect(subscribersTriggered).toBe(3) // 1002 without batching + // expect(totalComponentRenders).toBe(3001) }, 25000) diff --git a/packages/toolkit/src/query/tests/helpers.tsx b/packages/toolkit/src/query/tests/helpers.tsx index 843d86ffb8..6e0138aec2 100644 --- a/packages/toolkit/src/query/tests/helpers.tsx +++ b/packages/toolkit/src/query/tests/helpers.tsx @@ -191,7 +191,7 @@ export function setupApiStore< api.middleware, ...middleware ), - enhancers: (e) => e.concat(autoBatchEnhancer(0)), + // enhancers: (e) => e.concat(autoBatchEnhancer(0)), }) type StoreType = EnhancedStore< From 420baff3b4a2b2f043a9403b104e889a92eda71a Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sun, 4 Sep 2022 11:14:27 -0400 Subject: [PATCH 4/9] WIP batch enhancer turned back on --- .../src/query/core/buildMiddleware/index.ts | 6 +-- packages/toolkit/src/query/core/buildSlice.ts | 22 +++++------ .../toolkit/src/query/tests/cleanup.test.tsx | 39 +++++++++++++++---- packages/toolkit/src/query/tests/helpers.tsx | 2 +- 4 files changed, 46 insertions(+), 23 deletions(-) diff --git a/packages/toolkit/src/query/core/buildMiddleware/index.ts b/packages/toolkit/src/query/core/buildMiddleware/index.ts index 09f2412e12..2729bf77f6 100644 --- a/packages/toolkit/src/query/core/buildMiddleware/index.ts +++ b/packages/toolkit/src/query/core/buildMiddleware/index.ts @@ -79,9 +79,9 @@ export function buildMiddleware< const stateBefore = mwApi.getState() - if (!batchedActionsHandler(action, mwApi, stateBefore)) { - return - } + // if (!batchedActionsHandler(action, mwApi, stateBefore)) { + // return + // } const res = next(action) diff --git a/packages/toolkit/src/query/core/buildSlice.ts b/packages/toolkit/src/query/core/buildSlice.ts index 6dc5de7c87..9379771e1c 100644 --- a/packages/toolkit/src/query/core/buildSlice.ts +++ b/packages/toolkit/src/query/core/buildSlice.ts @@ -453,17 +453,17 @@ export function buildSlice({ } }) // original case added back in as otherwise nothing would happen without the batching middleware - // .addCase( - // queryThunk.rejected, - // (draft, { meta: { condition, arg, requestId }, error, payload }) => { - // // request was aborted due to condition (another query already running) - // if (condition && arg.subscribe) { - // const substate = (draft[arg.queryCacheKey] ??= {}) - // substate[requestId] = - // arg.subscriptionOptions ?? substate[requestId] ?? {} - // } - // } - // ) + .addCase( + queryThunk.rejected, + (draft, { meta: { condition, arg, requestId }, error, payload }) => { + // request was aborted due to condition (another query already running) + if (condition && arg.subscribe) { + const substate = (draft[arg.queryCacheKey] ??= {}) + substate[requestId] = + arg.subscriptionOptions ?? substate[requestId] ?? {} + } + } + ) // update the state to be a new object to be picked up as a "state change" // by redux-persist's `autoMergeLevel2` .addMatcher(hasRehydrationInfo, (draft) => ({ ...draft })) diff --git a/packages/toolkit/src/query/tests/cleanup.test.tsx b/packages/toolkit/src/query/tests/cleanup.test.tsx index 4d0d2f4038..f6d153360b 100644 --- a/packages/toolkit/src/query/tests/cleanup.test.tsx +++ b/packages/toolkit/src/query/tests/cleanup.test.tsx @@ -19,6 +19,10 @@ const storeRef = setupApiStore(api) let getSubStateA = () => storeRef.store.getState().api.queries['a(undefined)'] let getSubStateB = () => storeRef.store.getState().api.queries['b(undefined)'] +function delay(ms: number) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + function UsingA() { const { data } = api.endpoints.a.useQuery() @@ -174,7 +178,7 @@ test.only('Minimizes the number of subscription dispatches when multiple compone }, }) - const NUM_LIST_ITEMS = 1000 + const NUM_LIST_ITEMS = 5 function UsingA() { totalListItemRenders++ @@ -198,9 +202,26 @@ test.only('Minimizes the number of subscription dispatches when multiple compone expect(totalListItemRenders).toBe(NUM_LIST_ITEMS * 2) expect(totalParentComponentRenders).toBe(1) - expect(actionTypes.length).toBe(2) - expect(subscribersTriggered).toBe(2) - // expect(subscribersTriggered).toBe(1) // 1001 without batching + + const firstActions = actionTypes.slice(0, 2) + const remainingActions = actionTypes.slice(2) + expect(firstActions).toEqual([ + // First action is always registration + 'api/config/middlewareRegistered', + // First list item always initiates the request + 'api/executeQuery/pending', + ]) + expect(remainingActions.length).toBe(NUM_LIST_ITEMS - 1) + // All remaining list items end up triggering a "rejected" action that adds a subscription + expect( + remainingActions.every( + (actionType) => actionType === 'api/executeQuery/rejected' + ) + ).toBe(true) + + // expect(actionTypes.length).toBe(2) + // expect(subscribersTriggered).toBe(2) + expect(subscribersTriggered).toBe(1) // N + 1 without batching await act(async () => { jest.advanceTimersByTime(10) @@ -218,9 +239,11 @@ test.only('Minimizes the number of subscription dispatches when multiple compone // 'api/subscriptions/subscriptionRequestsRejected', // 'api/executeQuery/fulfilled' // ] - expect(actionTypes.length).toBe(4) - expect(subscribersTriggered).toBe(4) + // "registered", "pending", N-1 "rejected", "fulfilled" + expect(actionTypes.length).toBe(NUM_LIST_ITEMS + 2) + // expect(subscribersTriggered).toBe(4) // expect(actionTypes.length).toBe(1002) - // expect(subscribersTriggered).toBe(3) // 1002 without batching - // expect(totalComponentRenders).toBe(3001) + expect(subscribersTriggered).toBe(3) // N + 2 without batching + expect(totalListItemRenders).toBe(NUM_LIST_ITEMS * 3) + expect(totalParentComponentRenders).toBe(1) }, 25000) diff --git a/packages/toolkit/src/query/tests/helpers.tsx b/packages/toolkit/src/query/tests/helpers.tsx index 6e0138aec2..843d86ffb8 100644 --- a/packages/toolkit/src/query/tests/helpers.tsx +++ b/packages/toolkit/src/query/tests/helpers.tsx @@ -191,7 +191,7 @@ export function setupApiStore< api.middleware, ...middleware ), - // enhancers: (e) => e.concat(autoBatchEnhancer(0)), + enhancers: (e) => e.concat(autoBatchEnhancer(0)), }) type StoreType = EnhancedStore< From 66a657a6fdc12b53e481571d863005d0b15ab967 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sun, 4 Sep 2022 11:26:18 -0400 Subject: [PATCH 5/9] WIP Use queueMicrotask in autoBatchEnhancer --- packages/toolkit/src/autoBatchEnhancer.ts | 41 ++++++++++++++----- .../toolkit/src/query/tests/cleanup.test.tsx | 2 +- packages/toolkit/src/query/tests/helpers.tsx | 2 +- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/packages/toolkit/src/autoBatchEnhancer.ts b/packages/toolkit/src/autoBatchEnhancer.ts index ee2a27f86b..0082dbfe7d 100644 --- a/packages/toolkit/src/autoBatchEnhancer.ts +++ b/packages/toolkit/src/autoBatchEnhancer.ts @@ -2,17 +2,33 @@ import type { StoreEnhancer } from 'redux' export const autoBatch = 'ReduxToolkit_autoBatch' +// Copied from https://github.com/feross/queue-microtask +let promise: Promise +const queueMicrotaskShim = + typeof queueMicrotask === 'function' + ? queueMicrotask.bind(typeof window !== 'undefined' ? window : global) + : // reuse resolved promise, and allocate it lazily + (cb: () => void) => + (promise || (promise = Promise.resolve())).then(cb).catch((err: any) => + setTimeout(() => { + throw err + }, 0) + ) + export const autoBatchEnhancer = - (batchTimeout: number = 0): StoreEnhancer => + (): StoreEnhancer => (next) => (...args) => { const store = next(...args) let notifying = true - let nextNotification: NodeJS.Timeout | undefined = undefined + let notificationQueued = false + // let nextNotification: NodeJS.Timeout | undefined = undefined const listeners = new Set<() => void>() const notifyListeners = () => { - nextNotification = void listeners.forEach((l) => l()) + //nextNotification = void + notificationQueued = false + listeners.forEach((l) => l()) } return Object.assign({}, store, { @@ -29,14 +45,19 @@ export const autoBatchEnhancer = try { notifying = !action?.meta?.[autoBatch] if (notifying) { - if (nextNotification) { - nextNotification = void clearTimeout(nextNotification) - } + notificationQueued = false + // if (nextNotification) { + // nextNotification = void clearTimeout(nextNotification) + // } } else { - nextNotification ||= setTimeout( - notifyListeners, - batchTimeout - ) as any + if (!notificationQueued) { + notificationQueued = true + queueMicrotaskShim(notifyListeners) + } + // nextNotification ||= setTimeout( + // notifyListeners, + // batchTimeout + // ) as any } return store.dispatch(action) } finally { diff --git a/packages/toolkit/src/query/tests/cleanup.test.tsx b/packages/toolkit/src/query/tests/cleanup.test.tsx index f6d153360b..0f1faacb5b 100644 --- a/packages/toolkit/src/query/tests/cleanup.test.tsx +++ b/packages/toolkit/src/query/tests/cleanup.test.tsx @@ -152,7 +152,7 @@ test('data stays in store when one component requiring the data stays in the sto expect(getSubStateB()).toEqual(statusB) }) -test.only('Minimizes the number of subscription dispatches when multiple components ask for the same data', async () => { +test('Minimizes the number of subscription dispatches when multiple components ask for the same data', async () => { const listenerMiddleware = createListenerMiddleware() const storeRef = setupApiStore(api, undefined, { middleware: [listenerMiddleware.middleware], diff --git a/packages/toolkit/src/query/tests/helpers.tsx b/packages/toolkit/src/query/tests/helpers.tsx index 843d86ffb8..3d9b198921 100644 --- a/packages/toolkit/src/query/tests/helpers.tsx +++ b/packages/toolkit/src/query/tests/helpers.tsx @@ -191,7 +191,7 @@ export function setupApiStore< api.middleware, ...middleware ), - enhancers: (e) => e.concat(autoBatchEnhancer(0)), + enhancers: (e) => e.concat(autoBatchEnhancer()), }) type StoreType = EnhancedStore< From cde4b0bfdffa64c0f2fb66adf06fe48e24f675aa Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sun, 4 Sep 2022 13:05:50 -0400 Subject: [PATCH 6/9] Rename flag to `shouldAutoBatch` --- packages/toolkit/src/autoBatchEnhancer.ts | 4 ++-- packages/toolkit/src/createAsyncThunk.ts | 3 ++- packages/toolkit/src/index.ts | 2 +- packages/toolkit/src/query/core/buildSlice.ts | 4 ++-- packages/toolkit/src/query/core/buildThunks.ts | 16 ++++++++-------- .../toolkit/src/tests/createAsyncThunk.test.ts | 3 +++ 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/toolkit/src/autoBatchEnhancer.ts b/packages/toolkit/src/autoBatchEnhancer.ts index 0082dbfe7d..4fa3b3e46e 100644 --- a/packages/toolkit/src/autoBatchEnhancer.ts +++ b/packages/toolkit/src/autoBatchEnhancer.ts @@ -1,6 +1,6 @@ import type { StoreEnhancer } from 'redux' -export const autoBatch = 'ReduxToolkit_autoBatch' +export const shouldAutoBatch = 'RTK_autoBatch' // Copied from https://github.com/feross/queue-microtask let promise: Promise @@ -43,7 +43,7 @@ export const autoBatchEnhancer = }, dispatch(action: any) { try { - notifying = !action?.meta?.[autoBatch] + notifying = !action?.meta?.[shouldAutoBatch] if (notifying) { notificationQueued = false // if (nextNotification) { diff --git a/packages/toolkit/src/createAsyncThunk.ts b/packages/toolkit/src/createAsyncThunk.ts index e6d1398b95..3c5b76ac68 100644 --- a/packages/toolkit/src/createAsyncThunk.ts +++ b/packages/toolkit/src/createAsyncThunk.ts @@ -7,6 +7,7 @@ import { createAction } from './createAction' import type { ThunkDispatch } from 'redux-thunk' import type { FallbackIfUnknown, Id, IsAny, IsUnknown } from './tsHelpers' import { nanoid } from './nanoid' +import { shouldAutoBatch } from './autoBatchEnhancer' // @ts-ignore we need the import of these types due to a bundling issue. type _Keep = PayloadAction | ActionCreatorWithPreparedPayload @@ -550,7 +551,7 @@ export const createAsyncThunk = (() => { // TODO: this is a hack to showcase the autobatching behaviour // currently there is no way to add `meta` to a "condition rejected" // action - that would need to be added - ReduxToolkit_autoBatch: true, + [shouldAutoBatch]: true, }, }) ) diff --git a/packages/toolkit/src/index.ts b/packages/toolkit/src/index.ts index 85f9a35032..58adf1d445 100644 --- a/packages/toolkit/src/index.ts +++ b/packages/toolkit/src/index.ts @@ -17,7 +17,7 @@ export type { } from 'reselect' export { createDraftSafeSelector } from './createDraftSafeSelector' export type { ThunkAction, ThunkDispatch, ThunkMiddleware } from 'redux-thunk' -export { autoBatch, autoBatchEnhancer } from './autoBatchEnhancer' +export { shouldAutoBatch, autoBatchEnhancer } from './autoBatchEnhancer' // We deliberately enable Immer's ES5 support, on the grounds that // we assume RTK will be used with React Native and other Proxy-less diff --git a/packages/toolkit/src/query/core/buildSlice.ts b/packages/toolkit/src/query/core/buildSlice.ts index 9379771e1c..ec6e373e54 100644 --- a/packages/toolkit/src/query/core/buildSlice.ts +++ b/packages/toolkit/src/query/core/buildSlice.ts @@ -1,6 +1,6 @@ import type { AnyAction, PayloadAction } from '@reduxjs/toolkit' import { - autoBatch, + shouldAutoBatch, combineReducers, createAction, createSlice, @@ -90,7 +90,7 @@ const prepareAutoBatched = () => (payload: T): { payload: T; meta: unknown } => ({ payload, - meta: { [autoBatch]: true }, + meta: { [shouldAutoBatch]: true }, }) export function buildSlice({ diff --git a/packages/toolkit/src/query/core/buildThunks.ts b/packages/toolkit/src/query/core/buildThunks.ts index d78be76ce7..dc6318bb5d 100644 --- a/packages/toolkit/src/query/core/buildThunks.ts +++ b/packages/toolkit/src/query/core/buildThunks.ts @@ -38,7 +38,7 @@ import type { ThunkDispatch, AsyncThunk, } from '@reduxjs/toolkit' -import { createAsyncThunk, autoBatch } from '@reduxjs/toolkit' +import { createAsyncThunk, shouldAutoBatch } from '@reduxjs/toolkit' import { HandledError } from '../HandledError' @@ -124,16 +124,16 @@ export type ThunkResult = unknown export type ThunkApiMetaConfig = { pendingMeta: { startedTimeStamp: number - [autoBatch]: true + [shouldAutoBatch]: true } fulfilledMeta: { fulfilledTimeStamp: number baseQueryMeta: unknown - [autoBatch]: true + [shouldAutoBatch]: true } rejectedMeta: { baseQueryMeta: unknown - [autoBatch]: true + [shouldAutoBatch]: true } } export type QueryThunk = AsyncThunk< @@ -403,7 +403,7 @@ export function buildThunks< { fulfilledTimeStamp: Date.now(), baseQueryMeta: result.meta, - [autoBatch]: true, + [shouldAutoBatch]: true, } ) } catch (error) { @@ -428,7 +428,7 @@ export function buildThunks< catchedError.meta, arg.originalArgs ), - { baseQueryMeta: catchedError.meta, [autoBatch]: true } + { baseQueryMeta: catchedError.meta, [shouldAutoBatch]: true } ) } catch (e) { catchedError = e @@ -478,7 +478,7 @@ In the case of an unhandled error, no tags will be "provided" or "invalidated".` ThunkApiMetaConfig & { state: RootState } >(`${reducerPath}/executeQuery`, executeEndpoint, { getPendingMeta() { - return { startedTimeStamp: Date.now(), [autoBatch]: true } + return { startedTimeStamp: Date.now(), [shouldAutoBatch]: true } }, condition(arg, { getState }) { const state = getState() @@ -512,7 +512,7 @@ In the case of an unhandled error, no tags will be "provided" or "invalidated".` ThunkApiMetaConfig & { state: RootState } >(`${reducerPath}/executeMutation`, executeEndpoint, { getPendingMeta() { - return { startedTimeStamp: Date.now(), [autoBatch]: true } + return { startedTimeStamp: Date.now(), [shouldAutoBatch]: true } }, }) diff --git a/packages/toolkit/src/tests/createAsyncThunk.test.ts b/packages/toolkit/src/tests/createAsyncThunk.test.ts index 6539d73232..9f9a9a4d39 100644 --- a/packages/toolkit/src/tests/createAsyncThunk.test.ts +++ b/packages/toolkit/src/tests/createAsyncThunk.test.ts @@ -6,6 +6,7 @@ import { createReducer, } from '@reduxjs/toolkit' import { miniSerializeError } from '@internal/createAsyncThunk' +import { shouldAutoBatch } from '@internal/autoBatchEnhancer' import { mockConsole, @@ -664,6 +665,7 @@ describe('conditional skipping of asyncThunks', () => { condition: true, requestId: expect.stringContaining(''), requestStatus: 'rejected', + [shouldAutoBatch]: true, }, payload: undefined, type: 'test/rejected', @@ -939,6 +941,7 @@ describe('meta', () => { rejectedWithValue: true, aborted: false, condition: false, + [shouldAutoBatch]: true, }, error: { message: 'Rejected' }, payload: 'damn!', From 20368bfeaa449410bb6d2144cfdd7254f42e9a62 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sun, 4 Sep 2022 13:06:23 -0400 Subject: [PATCH 7/9] Make autobatching optional in store setup test util --- packages/toolkit/src/query/tests/cleanup.test.tsx | 1 + packages/toolkit/src/query/tests/helpers.tsx | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/toolkit/src/query/tests/cleanup.test.tsx b/packages/toolkit/src/query/tests/cleanup.test.tsx index 0f1faacb5b..6627dba601 100644 --- a/packages/toolkit/src/query/tests/cleanup.test.tsx +++ b/packages/toolkit/src/query/tests/cleanup.test.tsx @@ -157,6 +157,7 @@ test('Minimizes the number of subscription dispatches when multiple components a const storeRef = setupApiStore(api, undefined, { middleware: [listenerMiddleware.middleware], withoutTestLifecycles: true, + addAutoBatchEnhancer: true, }) let subscribersTriggered = 0 diff --git a/packages/toolkit/src/query/tests/helpers.tsx b/packages/toolkit/src/query/tests/helpers.tsx index 3d9b198921..a1a6fb3cb7 100644 --- a/packages/toolkit/src/query/tests/helpers.tsx +++ b/packages/toolkit/src/query/tests/helpers.tsx @@ -179,10 +179,11 @@ export function setupApiStore< options: { withoutListeners?: boolean withoutTestLifecycles?: boolean + addAutoBatchEnhancer?: boolean middleware?: Middleware[] } = {} ) { - const { middleware = [] } = options + const { middleware = [], addAutoBatchEnhancer = true } = options const getStore = () => configureStore({ reducer: { api: api.reducer, ...extraReducers }, @@ -191,7 +192,13 @@ export function setupApiStore< api.middleware, ...middleware ), - enhancers: (e) => e.concat(autoBatchEnhancer()), + enhancers: (e) => { + if (addAutoBatchEnhancer) { + return e.concat(autoBatchEnhancer()) + } + // stupid TS `readonly` error if we `return e` + return e.concat() + }, }) type StoreType = EnhancedStore< From 14da09282e607527b79693deee560a3101d43752 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sun, 4 Sep 2022 13:25:43 -0400 Subject: [PATCH 8/9] Remove stray `.only` calls in tests --- packages/toolkit/src/query/tests/buildSlice.test.ts | 2 +- packages/toolkit/src/tests/createSlice.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/toolkit/src/query/tests/buildSlice.test.ts b/packages/toolkit/src/query/tests/buildSlice.test.ts index 1918863739..086ed4d1a1 100644 --- a/packages/toolkit/src/query/tests/buildSlice.test.ts +++ b/packages/toolkit/src/query/tests/buildSlice.test.ts @@ -82,7 +82,7 @@ it('only resets the api state when resetApiState is dispatched', async () => { expect(storeRef.store.getState()).toEqual(initialState) }) -describe.only('`merge` callback', () => { +describe('`merge` callback', () => { const baseQuery = (args?: any) => ({ data: args }) interface Todo { diff --git a/packages/toolkit/src/tests/createSlice.test.ts b/packages/toolkit/src/tests/createSlice.test.ts index bea220ba6a..3355cf80a9 100644 --- a/packages/toolkit/src/tests/createSlice.test.ts +++ b/packages/toolkit/src/tests/createSlice.test.ts @@ -372,7 +372,7 @@ describe('createSlice', () => { }) }) - describe.only('Deprecation warnings', () => { + describe('Deprecation warnings', () => { let originalNodeEnv = process.env.NODE_ENV beforeEach(() => { From 81e6c1b7698e47596f0478947270e77c3888cf9b Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sun, 4 Sep 2022 13:26:04 -0400 Subject: [PATCH 9/9] Fix timing issues and `act()` warnings in tests --- .../src/query/tests/buildHooks.test.tsx | 54 +++++++++++++------ .../src/query/tests/errorHandling.test.tsx | 21 +++++--- 2 files changed, 53 insertions(+), 22 deletions(-) diff --git a/packages/toolkit/src/query/tests/buildHooks.test.tsx b/packages/toolkit/src/query/tests/buildHooks.test.tsx index 5806252c75..d626f81633 100644 --- a/packages/toolkit/src/query/tests/buildHooks.test.tsx +++ b/packages/toolkit/src/query/tests/buildHooks.test.tsx @@ -1277,8 +1277,13 @@ describe('hooks tests', () => { ) userEvent.hover(screen.getByTestId('highPriority')) + + await waitFor(() => { + expect(screen.getByTestId('isFetching').textContent).toBe('true') + }) + expect( - api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any) + api.endpoints.getUser.select(USER_ID)(storeRef.store.getState()) ).toEqual({ data: { name: 'Timmy' }, endpointName: 'getUser', @@ -1299,7 +1304,7 @@ describe('hooks tests', () => { ) expect( - api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any) + api.endpoints.getUser.select(USER_ID)(storeRef.store.getState()) ).toEqual({ data: { name: 'Timmy' }, endpointName: 'getUser', @@ -1347,7 +1352,7 @@ describe('hooks tests', () => { userEvent.hover(screen.getByTestId('lowPriority')) expect( - api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any) + api.endpoints.getUser.select(USER_ID)(storeRef.store.getState()) ).toEqual({ data: { name: 'Timmy' }, endpointName: 'getUser', @@ -1365,7 +1370,7 @@ describe('hooks tests', () => { await waitMs() expect( - api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any) + api.endpoints.getUser.select(USER_ID)(storeRef.store.getState()) ).toEqual({ data: { name: 'Timmy' }, endpointName: 'getUser', @@ -1414,8 +1419,13 @@ describe('hooks tests', () => { // This should run the query being that we're past the threshold userEvent.hover(screen.getByTestId('lowPriority')) + + await waitFor(() => + expect(screen.getByTestId('isFetching').textContent).toBe('true') + ) + expect( - api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any) + api.endpoints.getUser.select(USER_ID)(storeRef.store.getState()) ).toEqual({ data: { name: 'Timmy' }, endpointName: 'getUser', @@ -1435,7 +1445,7 @@ describe('hooks tests', () => { ) expect( - api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any) + api.endpoints.getUser.select(USER_ID)(storeRef.store.getState()) ).toEqual({ data: { name: 'Timmy' }, endpointName: 'getUser', @@ -1482,13 +1492,13 @@ describe('hooks tests', () => { // Get a snapshot of the last result const latestQueryData = api.endpoints.getUser.select(USER_ID)( - storeRef.store.getState() as any + storeRef.store.getState() ) userEvent.hover(screen.getByTestId('lowPriority')) // Serve up the result from the cache being that the condition wasn't met expect( - api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any) + api.endpoints.getUser.select(USER_ID)(storeRef.store.getState()) ).toEqual(latestQueryData) }) @@ -1516,7 +1526,7 @@ describe('hooks tests', () => { userEvent.hover(screen.getByTestId('lowPriority')) expect( - api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any) + api.endpoints.getUser.select(USER_ID)(storeRef.store.getState()) ).toEqual({ endpointName: 'getUser', isError: false, @@ -1984,13 +1994,13 @@ describe('hooks with createApi defaults set', () => { const addBtn = screen.getByTestId('addPost') - await waitFor(() => expect(getRenderCount()).toBe(4)) + await waitFor(() => expect(getRenderCount()).toBe(3)) fireEvent.click(addBtn) - await waitFor(() => expect(getRenderCount()).toBe(6)) + await waitFor(() => expect(getRenderCount()).toBe(5)) fireEvent.click(addBtn) fireEvent.click(addBtn) - await waitFor(() => expect(getRenderCount()).toBe(8)) + await waitFor(() => expect(getRenderCount()).toBe(7)) }) test('useQuery with selectFromResult option serves a deeply memoized value and does not rerender unnecessarily', async () => { @@ -2355,13 +2365,19 @@ describe('skip behaviour', () => { expect(result.current).toEqual(uninitialized) expect(subscriptionCount('getUser(1)')).toBe(0) - rerender([1]) + act(() => { + rerender([1]) + }) expect(result.current).toMatchObject({ status: QueryStatus.pending }) expect(subscriptionCount('getUser(1)')).toBe(1) - rerender([1, { skip: true }]) + act(() => { + rerender([1, { skip: true }]) + }) expect(result.current).toEqual(uninitialized) expect(subscriptionCount('getUser(1)')).toBe(0) + + await act(async () => {}) }) test('skipToken', async () => { @@ -2379,14 +2395,20 @@ describe('skip behaviour', () => { // also no subscription on `getUser(skipToken)` or similar: expect(storeRef.store.getState().api.subscriptions).toEqual({}) - rerender([1]) + act(() => { + rerender([1]) + }) expect(result.current).toMatchObject({ status: QueryStatus.pending }) expect(subscriptionCount('getUser(1)')).toBe(1) expect(storeRef.store.getState().api.subscriptions).not.toEqual({}) - rerender([skipToken]) + act(() => { + rerender([skipToken]) + }) expect(result.current).toEqual(uninitialized) expect(subscriptionCount('getUser(1)')).toBe(0) + + await act(async () => {}) }) }) diff --git a/packages/toolkit/src/query/tests/errorHandling.test.tsx b/packages/toolkit/src/query/tests/errorHandling.test.tsx index b6a258e1a8..55f5f1d886 100644 --- a/packages/toolkit/src/query/tests/errorHandling.test.tsx +++ b/packages/toolkit/src/query/tests/errorHandling.test.tsx @@ -6,7 +6,14 @@ import type { AxiosError, AxiosRequestConfig, AxiosResponse } from 'axios' import axios from 'axios' import { expectExactType, hookWaitFor, setupApiStore } from './helpers' import { server } from './mocks/server' -import { fireEvent, render, waitFor, screen, act, renderHook } from '@testing-library/react' +import { + fireEvent, + render, + waitFor, + screen, + act, + renderHook, +} from '@testing-library/react' import { useDispatch } from 'react-redux' import type { AnyAction, ThunkDispatch } from '@reduxjs/toolkit' import type { BaseQueryApi } from '../baseQueryTypes' @@ -131,7 +138,7 @@ describe('query error handling', () => { wrapper: storeRef.wrapper, }) - await hookWaitFor(() => expect(result.current.isFetching).toBeFalsy()) + await hookWaitFor(() => expect(result.current.isFetching).toBe(false)) expect(result.current).toEqual( expect.objectContaining({ isLoading: false, @@ -147,9 +154,10 @@ describe('query error handling', () => { ) ) - act(() => void result.current.refetch()) + await act(async () => { + await result.current.refetch() + }) - await hookWaitFor(() => expect(result.current.isFetching).toBeFalsy()) expect(result.current).toEqual( expect.objectContaining({ isLoading: false, @@ -193,9 +201,10 @@ describe('query error handling', () => { }) ) - act(() => void result.current.refetch()) + await act(async () => { + await result.current.refetch() + }) - await hookWaitFor(() => expect(result.current.isFetching).toBeFalsy()) expect(result.current).toEqual( expect.objectContaining({ isLoading: false,