From bf7c37b745d192b7ca0bba6ab867fddee3ebe61f Mon Sep 17 00:00:00 2001 From: Dennis Schaller Date: Thu, 1 Sep 2022 14:24:36 +0200 Subject: [PATCH 1/3] query/endpointDefinitions: add sideEffectForced to QueryExtraOptions query/buildInitiate: check sideEffectForced to conditionally force query --- packages/toolkit/src/query/core/buildInitiate.ts | 16 +++++++++++++--- .../toolkit/src/query/endpointDefinitions.ts | 6 +++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/toolkit/src/query/core/buildInitiate.ts b/packages/toolkit/src/query/core/buildInitiate.ts index cbb5c19d71..7ce8e0f0ca 100644 --- a/packages/toolkit/src/query/core/buildInitiate.ts +++ b/packages/toolkit/src/query/core/buildInitiate.ts @@ -5,11 +5,10 @@ import type { QueryArgFrom, ResultTypeFrom, } from '../endpointDefinitions' -import { DefinitionType } from '../endpointDefinitions' +import { DefinitionType, isQueryDefinition } from '../endpointDefinitions' import type { QueryThunk, MutationThunk, QueryThunkArg } from './buildThunks' import type { AnyAction, ThunkAction, SerializedError } from '@reduxjs/toolkit' import type { SubscriptionOptions, RootState } from './apiState' -import { QueryStatus } from './apiState' import type { InternalSerializeQueryArgs } from '../defaultSerializeQueryArgs' import type { Api, ApiContext } from '../apiTypes' import type { ApiEndpointQuery } from './module' @@ -279,10 +278,21 @@ Features like automatic cache collection, automatic refetching etc. will not be endpointDefinition, endpointName, }) + + const endpointContext = context.endpointDefinitions[endpointName] + const sideEffectForced = + isQueryDefinition(endpointContext) && + endpointContext.sideEffectForced?.({ + getState, + endpointState: ( + api.endpoints[endpointName] as ApiEndpointQuery + ).select(arg)(getState()), + }) + const thunk = queryThunk({ type: 'query', subscribe, - forceRefetch, + forceRefetch: forceRefetch || sideEffectForced, subscriptionOptions, endpointName, originalArgs: arg, diff --git a/packages/toolkit/src/query/endpointDefinitions.ts b/packages/toolkit/src/query/endpointDefinitions.ts index 477c8ddd95..3ce91f0d6e 100644 --- a/packages/toolkit/src/query/endpointDefinitions.ts +++ b/packages/toolkit/src/query/endpointDefinitions.ts @@ -1,6 +1,6 @@ import type { AnyAction, ThunkDispatch } from '@reduxjs/toolkit' import { SerializeQueryArgs } from './defaultSerializeQueryArgs' -import type { RootState } from './core/apiState' +import type { QuerySubState, RootState } from './core/apiState' import type { BaseQueryExtraOptions, BaseQueryFn, @@ -343,6 +343,10 @@ export interface QueryExtraOptions< * All of these are `undefined` at runtime, purely to be used in TypeScript declarations! */ Types?: QueryTypes + sideEffectForced?: (params: { + getState(): RootState + endpointState: QuerySubState + }) => boolean } export type QueryDefinition< From 0dedba0adba04394b0ea15fc5ca868f3824ed520 Mon Sep 17 00:00:00 2001 From: Dennis Schaller Date: Tue, 13 Sep 2022 18:36:17 +0200 Subject: [PATCH 2/3] rename sideEffectForced to forceRefetch make forceRefetch be read from endpoint config instead of passing it into thunk as forced --- packages/toolkit/src/query/core/buildInitiate.ts | 12 +----------- packages/toolkit/src/query/core/buildThunks.ts | 13 ++++++++++++- packages/toolkit/src/query/endpointDefinitions.ts | 9 +++++---- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/packages/toolkit/src/query/core/buildInitiate.ts b/packages/toolkit/src/query/core/buildInitiate.ts index 7ce8e0f0ca..f29514cbdc 100644 --- a/packages/toolkit/src/query/core/buildInitiate.ts +++ b/packages/toolkit/src/query/core/buildInitiate.ts @@ -279,20 +279,10 @@ Features like automatic cache collection, automatic refetching etc. will not be endpointName, }) - const endpointContext = context.endpointDefinitions[endpointName] - const sideEffectForced = - isQueryDefinition(endpointContext) && - endpointContext.sideEffectForced?.({ - getState, - endpointState: ( - api.endpoints[endpointName] as ApiEndpointQuery - ).select(arg)(getState()), - }) - const thunk = queryThunk({ type: 'query', subscribe, - forceRefetch: forceRefetch || sideEffectForced, + forceRefetch: forceRefetch, subscriptionOptions, endpointName, originalArgs: arg, diff --git a/packages/toolkit/src/query/core/buildThunks.ts b/packages/toolkit/src/query/core/buildThunks.ts index d6a111b194..01d76716d7 100644 --- a/packages/toolkit/src/query/core/buildThunks.ts +++ b/packages/toolkit/src/query/core/buildThunks.ts @@ -12,10 +12,11 @@ import type { QueryActionCreatorResult, } from './buildInitiate' import { forceQueryFnSymbol, isUpsertQuery } from './buildInitiate' -import type { +import { AssertTagTypes, EndpointDefinition, EndpointDefinitions, + isQueryDefinition, MutationDefinition, QueryArgFrom, QueryDefinition, @@ -478,6 +479,7 @@ In the case of an unhandled error, no tags will be "provided" or "invalidated".` const state = getState() const requestState = state[reducerPath]?.queries?.[arg.queryCacheKey] const fulfilledVal = requestState?.fulfilledTimeStamp + const endpointDefinition = endpointDefinitions[arg.endpointName] // Order of these checks matters. // In order for `upsertQueryData` to successfully run while an existing request is in flight, @@ -490,6 +492,15 @@ In the case of an unhandled error, no tags will be "provided" or "invalidated".` // if this is forced, continue if (isForcedQuery(arg, state)) return true + if ( + isQueryDefinition(endpointDefinition) && + endpointDefinition?.forceRefetch?.({ + endpointState: requestState, + state, + }) + ) + return true + // Pull from the cache unless we explicitly force refetch or qualify based on time if (fulfilledVal) // Value is cached and we didn't specify to refresh, skip it. diff --git a/packages/toolkit/src/query/endpointDefinitions.ts b/packages/toolkit/src/query/endpointDefinitions.ts index 3ce91f0d6e..59eda3c7fa 100644 --- a/packages/toolkit/src/query/endpointDefinitions.ts +++ b/packages/toolkit/src/query/endpointDefinitions.ts @@ -339,14 +339,15 @@ export interface QueryExtraOptions< responseData: ResultType ): ResultType | void + forceRefetch?(params: { + state: RootState + endpointState?: QuerySubState + }): boolean + /** * All of these are `undefined` at runtime, purely to be used in TypeScript declarations! */ Types?: QueryTypes - sideEffectForced?: (params: { - getState(): RootState - endpointState: QuerySubState - }) => boolean } export type QueryDefinition< From 9723738f50e91e8e997f3ffb57c8593c3da326ab Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 8 Oct 2022 11:54:39 -0400 Subject: [PATCH 3/3] Add args to forceRefetch, add doc block, and test for merging --- .../toolkit/src/query/core/buildThunks.ts | 31 +++++++++---- .../toolkit/src/query/endpointDefinitions.ts | 25 +++++++++++ .../toolkit/src/query/tests/createApi.test.ts | 45 +++++++++++++++++++ 3 files changed, 93 insertions(+), 8 deletions(-) diff --git a/packages/toolkit/src/query/core/buildThunks.ts b/packages/toolkit/src/query/core/buildThunks.ts index 01d76716d7..67291aad07 100644 --- a/packages/toolkit/src/query/core/buildThunks.ts +++ b/packages/toolkit/src/query/core/buildThunks.ts @@ -475,36 +475,51 @@ In the case of an unhandled error, no tags will be "provided" or "invalidated".` getPendingMeta() { return { startedTimeStamp: Date.now() } }, - condition(arg, { getState }) { + condition(queryThunkArgs, { getState }) { const state = getState() - const requestState = state[reducerPath]?.queries?.[arg.queryCacheKey] + + const requestState = + state[reducerPath]?.queries?.[queryThunkArgs.queryCacheKey] const fulfilledVal = requestState?.fulfilledTimeStamp - const endpointDefinition = endpointDefinitions[arg.endpointName] + const currentArg = queryThunkArgs.originalArgs + const previousArg = requestState?.originalArgs + const endpointDefinition = + endpointDefinitions[queryThunkArgs.endpointName] // Order of these checks matters. // In order for `upsertQueryData` to successfully run while an existing request is in flight, /// we have to check for that first, otherwise `queryThunk` will bail out and not run at all. - if (isUpsertQuery(arg)) return true + if (isUpsertQuery(queryThunkArgs)) { + return true + } // Don't retry a request that's currently in-flight - if (requestState?.status === 'pending') return false + if (requestState?.status === 'pending') { + return false + } // if this is forced, continue - if (isForcedQuery(arg, state)) return true + if (isForcedQuery(queryThunkArgs, state)) { + return true + } if ( isQueryDefinition(endpointDefinition) && endpointDefinition?.forceRefetch?.({ + currentArg, + previousArg, endpointState: requestState, state, }) - ) + ) { return true + } // Pull from the cache unless we explicitly force refetch or qualify based on time - if (fulfilledVal) + if (fulfilledVal) { // Value is cached and we didn't specify to refresh, skip it. return false + } return true }, diff --git a/packages/toolkit/src/query/endpointDefinitions.ts b/packages/toolkit/src/query/endpointDefinitions.ts index 59eda3c7fa..f0bc6c7534 100644 --- a/packages/toolkit/src/query/endpointDefinitions.ts +++ b/packages/toolkit/src/query/endpointDefinitions.ts @@ -339,7 +339,32 @@ export interface QueryExtraOptions< responseData: ResultType ): ResultType | void + /** + * Check to see if the endpoint should force a refetch in cases where it normally wouldn't. + * This is primarily useful for "infinite scroll" / pagination use cases where + * RTKQ is keeping a single cache entry that is added to over time, in combination + * with `serializeQueryArgs` returning a fixed cache key and a `merge` callback + * set to add incoming data to the cache entry each time. + * + * Example: + * + * ```ts + * forceRefetch({currentArg, previousArg}) { + * // Assume these are page numbers + * return currentArg !== previousArg + * }, + * serializeQueryArgs({endpointName}) { + * return endpointName + * }, + * merge(currentCacheData, responseData) { + * currentCacheData.push(...responseData) + * } + * + * ``` + */ forceRefetch?(params: { + currentArg: QueryArg | undefined + previousArg: QueryArg | undefined state: RootState endpointState?: QuerySubState }): boolean diff --git a/packages/toolkit/src/query/tests/createApi.test.ts b/packages/toolkit/src/query/tests/createApi.test.ts index ed0c0e9cb7..e21546c8ad 100644 --- a/packages/toolkit/src/query/tests/createApi.test.ts +++ b/packages/toolkit/src/query/tests/createApi.test.ts @@ -866,6 +866,18 @@ describe('custom serializeQueryArgs per endpoint', () => { query: (arg) => `${arg}`, serializeQueryArgs: serializer1, }), + listItems: build.query({ + query: (pageNumber) => `/listItems?page=${pageNumber}`, + serializeQueryArgs: ({ endpointName }) => { + return endpointName + }, + merge: (currentCache, newItems) => { + currentCache.push(...newItems) + }, + forceRefetch({ currentArg, previousArg }) { + return currentArg !== previousArg + }, + }), }), }) @@ -918,4 +930,37 @@ describe('custom serializeQueryArgs per endpoint', () => { ] ).toBeTruthy() }) + + test('serializeQueryArgs + merge allows refetching as args change with same cache key', async () => { + const allItems = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'i'] + const PAGE_SIZE = 3 + + function paginate(array: T[], page_size: number, page_number: number) { + // human-readable page numbers usually start with 1, so we reduce 1 in the first argument + return array.slice((page_number - 1) * page_size, page_number * page_size) + } + + server.use( + rest.get('https://example.com/listItems', (req, res, ctx) => { + const pageString = req.url.searchParams.get('page') + const pageNum = parseInt(pageString || '0') + + const results = paginate(allItems, PAGE_SIZE, pageNum) + return res(ctx.json(results)) + }) + ) + + // Page number shouldn't matter here, because the cache key ignores that. + // We just need to select the only cache entry. + const selectListItems = api.endpoints.listItems.select(0) + + await storeRef.store.dispatch(api.endpoints.listItems.initiate(1)) + + const initialEntry = selectListItems(storeRef.store.getState()) + expect(initialEntry.data).toEqual(['a', 'b', 'c']) + + await storeRef.store.dispatch(api.endpoints.listItems.initiate(2)) + const updatedEntry = selectListItems(storeRef.store.getState()) + expect(updatedEntry.data).toEqual(['a', 'b', 'c', 'd', 'e', 'f']) + }) })