From 6ae000c20847cc0e730c77a4379e705205a9226a Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sat, 1 May 2021 14:01:32 +0200 Subject: [PATCH 01/18] split up middleware into multiple files --- .eslintrc.js | 3 +- src/query/core/buildMiddleware.ts | 358 ------------------ .../core/buildMiddleware/cacheCollection.ts | 51 +++ src/query/core/buildMiddleware/index.ts | 78 ++++ .../buildMiddleware/invalidationByTags.ts | 100 +++++ src/query/core/buildMiddleware/polling.ts | 132 +++++++ src/query/core/buildMiddleware/types.ts | 54 +++ .../buildMiddleware/windowEventHandling.ts | 58 +++ 8 files changed, 475 insertions(+), 359 deletions(-) delete mode 100644 src/query/core/buildMiddleware.ts create mode 100644 src/query/core/buildMiddleware/cacheCollection.ts create mode 100644 src/query/core/buildMiddleware/index.ts create mode 100644 src/query/core/buildMiddleware/invalidationByTags.ts create mode 100644 src/query/core/buildMiddleware/polling.ts create mode 100644 src/query/core/buildMiddleware/types.ts create mode 100644 src/query/core/buildMiddleware/windowEventHandling.ts diff --git a/.eslintrc.js b/.eslintrc.js index fde5ac996e..bba3244a98 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -13,7 +13,8 @@ module.exports = { // Silence some bizarre "rule not found" TSLint error '@typescript-eslint/no-angle-bracket-type-assertion': 'off', 'no-redeclare': 'off', - '@typescript-eslint/no-redeclare': ['error'], + // Silence some bizarre "rule not found" TSLint error + '@typescript-eslint/no-redeclare': 'off', 'no-use-before-define': 'off', '@typescript-eslint/no-use-before-define': ['error', { functions: false }], }, diff --git a/src/query/core/buildMiddleware.ts b/src/query/core/buildMiddleware.ts deleted file mode 100644 index ef4fc2dfe5..0000000000 --- a/src/query/core/buildMiddleware.ts +++ /dev/null @@ -1,358 +0,0 @@ -import { - AnyAction, - AsyncThunk, - createAction, - isAnyOf, - isFulfilled, - isRejectedWithValue, - Middleware, - MiddlewareAPI, - ThunkDispatch, - ActionCreatorWithPayload, -} from '@reduxjs/toolkit' -import { - QueryCacheKey, - QueryStatus, - QuerySubState, - QuerySubstateIdentifier, - RootState, - Subscribers, -} from './apiState' -import { Api, ApiContext } from '../apiTypes' -import { - calculateProvidedByThunk, - MutationThunkArg, - QueryThunkArg, - ThunkResult, -} from './buildThunks' -import { - AssertTagTypes, - calculateProvidedBy, - EndpointDefinitions, - FullTagDescription, -} from '../endpointDefinitions' -import { onFocus, onOnline } from './setupListeners' -import { flatten } from '../utils' - -type QueryStateMeta = Record -type TimeoutId = ReturnType - -export function buildMiddleware< - Definitions extends EndpointDefinitions, - ReducerPath extends string, - TagTypes extends string ->({ - reducerPath, - context, - context: { endpointDefinitions }, - queryThunk, - mutationThunk, - api, - assertTagType, -}: { - reducerPath: ReducerPath - context: ApiContext - queryThunk: AsyncThunk - mutationThunk: AsyncThunk - api: Api - assertTagType: AssertTagTypes -}) { - type MWApi = MiddlewareAPI< - ThunkDispatch, - RootState - > - const { - removeQueryResult, - unsubscribeQueryResult, - updateSubscriptionOptions, - resetApiState, - } = api.internalActions - - const currentRemovalTimeouts: QueryStateMeta = {} - const currentPolls: QueryStateMeta<{ - nextPollTimestamp: number - timeout?: TimeoutId - pollingInterval: number - }> = {} - - const actions = { - invalidateTags: createAction< - Array> - >(`${reducerPath}/invalidateTags`), - } - - const middleware: Middleware< - {}, - RootState, - ThunkDispatch - > = (mwApi) => (next) => (action) => { - const result = next(action) - - if ( - isAnyOf( - isFulfilled(mutationThunk), - isRejectedWithValue(mutationThunk) - )(action) - ) { - invalidateTags( - calculateProvidedByThunk( - action, - 'invalidatesTags', - endpointDefinitions, - assertTagType - ), - mwApi - ) - } - - if (actions.invalidateTags.match(action)) { - invalidateTags( - calculateProvidedBy( - action.payload, - undefined, - undefined, - undefined, - assertTagType - ), - mwApi - ) - } - - if (unsubscribeQueryResult.match(action)) { - handleUnsubscribe(action.payload, mwApi) - } - - if (updateSubscriptionOptions.match(action)) { - updatePollingInterval(action.payload, mwApi) - } - if ( - queryThunk.pending.match(action) || - (queryThunk.rejected.match(action) && action.meta.condition) - ) { - updatePollingInterval(action.meta.arg, mwApi) - } - if ( - queryThunk.fulfilled.match(action) || - (queryThunk.rejected.match(action) && !action.meta.condition) - ) { - startNextPoll(action.meta.arg, mwApi) - } - - if (onFocus.match(action)) { - refetchValidQueries(mwApi, 'refetchOnFocus') - } - if (onOnline.match(action)) { - refetchValidQueries(mwApi, 'refetchOnReconnect') - } - - if (resetApiState.match(action)) { - for (const [key, poll] of Object.entries(currentPolls)) { - if (poll?.timeout) clearTimeout(poll.timeout) - delete currentPolls[key] - } - for (const [key, timeout] of Object.entries(currentRemovalTimeouts)) { - if (timeout) clearTimeout(timeout) - delete currentRemovalTimeouts[key] - } - } - - return result - } - - return { middleware, actions } - - function refetchQuery( - querySubState: Exclude< - QuerySubState, - { status: QueryStatus.uninitialized } - >, - queryCacheKey: string, - override: Partial = {} - ) { - return queryThunk({ - endpointName: querySubState.endpointName, - originalArgs: querySubState.originalArgs, - subscribe: false, - forceRefetch: true, - startedTimeStamp: Date.now(), - queryCacheKey: queryCacheKey as any, - ...override, - }) - } - - function refetchValidQueries( - api: MWApi, - type: 'refetchOnFocus' | 'refetchOnReconnect' - ) { - const state = api.getState()[reducerPath] - const queries = state.queries - const subscriptions = state.subscriptions - - context.batch(() => { - for (const queryCacheKey of Object.keys(subscriptions)) { - const querySubState = queries[queryCacheKey] - const subscriptionSubState = subscriptions[queryCacheKey] - - if ( - !subscriptionSubState || - !querySubState || - querySubState.status === QueryStatus.uninitialized - ) - return - - const shouldRefetch = - Object.values(subscriptionSubState).some( - (sub) => sub[type] === true - ) || - (Object.values(subscriptionSubState).every( - (sub) => sub[type] === undefined - ) && - state.config[type]) - - if (shouldRefetch) { - api.dispatch(refetchQuery(querySubState, queryCacheKey)) - } - } - }) - } - - function invalidateTags( - tags: readonly FullTagDescription[], - api: MWApi - ) { - const state = api.getState()[reducerPath] - - const toInvalidate = new Set() - for (const tag of tags) { - const provided = state.provided[tag.type] - if (!provided) { - continue - } - - let invalidateSubscriptions = - (tag.id !== undefined - ? // id given: invalidate all queries that provide this type & id - provided[tag.id] - : // no id: invalidate all queries that provide this type - flatten(Object.values(provided))) ?? [] - - for (const invalidate of invalidateSubscriptions) { - toInvalidate.add(invalidate) - } - } - - context.batch(() => { - const valuesArray = Array.from(toInvalidate.values()) - for (const queryCacheKey of valuesArray) { - const querySubState = state.queries[queryCacheKey] - const subscriptionSubState = state.subscriptions[queryCacheKey] - if (querySubState && subscriptionSubState) { - if (Object.keys(subscriptionSubState).length === 0) { - api.dispatch(removeQueryResult({ queryCacheKey })) - } else if (querySubState.status !== QueryStatus.uninitialized) { - api.dispatch(refetchQuery(querySubState, queryCacheKey)) - } else { - } - } - } - }) - } - - function handleUnsubscribe( - { queryCacheKey }: QuerySubstateIdentifier, - api: MWApi - ) { - const keepUnusedDataFor = api.getState()[reducerPath].config - .keepUnusedDataFor - const currentTimeout = currentRemovalTimeouts[queryCacheKey] - if (currentTimeout) { - clearTimeout(currentTimeout) - } - currentRemovalTimeouts[queryCacheKey] = setTimeout(() => { - const subscriptions = api.getState()[reducerPath].subscriptions[ - queryCacheKey - ] - if (!subscriptions || Object.keys(subscriptions).length === 0) { - api.dispatch(removeQueryResult({ queryCacheKey })) - } - delete currentRemovalTimeouts![queryCacheKey] - }, keepUnusedDataFor * 1000) - } - - function startNextPoll( - { queryCacheKey }: QuerySubstateIdentifier, - api: MWApi - ) { - const state = api.getState()[reducerPath] - const querySubState = state.queries[queryCacheKey] - const subscriptions = state.subscriptions[queryCacheKey] - - if (!querySubState || querySubState.status === QueryStatus.uninitialized) - return - - const lowestPollingInterval = findLowestPollingInterval(subscriptions) - if (!Number.isFinite(lowestPollingInterval)) return - - const currentPoll = currentPolls[queryCacheKey] - - if (currentPoll?.timeout) { - clearTimeout(currentPoll.timeout) - currentPoll.timeout = undefined - } - - const nextPollTimestamp = Date.now() + lowestPollingInterval - - const currentInterval: typeof currentPolls[number] = (currentPolls[ - queryCacheKey - ] = { - nextPollTimestamp, - pollingInterval: lowestPollingInterval, - timeout: setTimeout(() => { - currentInterval!.timeout = undefined - api.dispatch(refetchQuery(querySubState, queryCacheKey)) - }, lowestPollingInterval), - }) - } - - function updatePollingInterval( - { queryCacheKey }: QuerySubstateIdentifier, - api: MWApi - ) { - const state = api.getState()[reducerPath] - const querySubState = state.queries[queryCacheKey] - const subscriptions = state.subscriptions[queryCacheKey] - - if (!querySubState || querySubState.status === QueryStatus.uninitialized) { - return - } - - const lowestPollingInterval = findLowestPollingInterval(subscriptions) - const currentPoll = currentPolls[queryCacheKey] - - if (!Number.isFinite(lowestPollingInterval)) { - if (currentPoll?.timeout) { - clearTimeout(currentPoll.timeout) - } - delete currentPolls[queryCacheKey] - return - } - - const nextPollTimestamp = Date.now() + lowestPollingInterval - - if (!currentPoll || nextPollTimestamp < currentPoll.nextPollTimestamp) { - startNextPoll({ queryCacheKey }, api) - } - } -} - -function findLowestPollingInterval(subscribers: Subscribers = {}) { - let lowestPollingInterval = Number.POSITIVE_INFINITY - for (const subscription of Object.values(subscribers)) { - if (!!subscription.pollingInterval) - lowestPollingInterval = Math.min( - subscription.pollingInterval, - lowestPollingInterval - ) - } - return lowestPollingInterval -} diff --git a/src/query/core/buildMiddleware/cacheCollection.ts b/src/query/core/buildMiddleware/cacheCollection.ts new file mode 100644 index 0000000000..1224c43334 --- /dev/null +++ b/src/query/core/buildMiddleware/cacheCollection.ts @@ -0,0 +1,51 @@ +import { QuerySubstateIdentifier } from '../apiState' +import { + QueryStateMeta, + SubMiddlewareApi, + SubMiddlewareBuilder, + TimeoutId, +} from './types' + +export const build: SubMiddlewareBuilder = ({ reducerPath, api }) => { + const currentRemovalTimeouts: QueryStateMeta = {} + + const { removeQueryResult, unsubscribeQueryResult } = api.internalActions + + return (mwApi) => (next) => (action): any => { + const result = next(action) + + if (unsubscribeQueryResult.match(action)) { + handleUnsubscribe(action.payload, mwApi) + } + + if (api.util.resetApiState.match(action)) { + for (const [key, timeout] of Object.entries(currentRemovalTimeouts)) { + if (timeout) clearTimeout(timeout) + delete currentRemovalTimeouts[key] + } + } + + return result + } + + function handleUnsubscribe( + { queryCacheKey }: QuerySubstateIdentifier, + api: SubMiddlewareApi + ) { + const keepUnusedDataFor = api.getState()[reducerPath].config + .keepUnusedDataFor + const currentTimeout = currentRemovalTimeouts[queryCacheKey] + if (currentTimeout) { + clearTimeout(currentTimeout) + } + currentRemovalTimeouts[queryCacheKey] = setTimeout(() => { + const subscriptions = api.getState()[reducerPath].subscriptions[ + queryCacheKey + ] + if (!subscriptions || Object.keys(subscriptions).length === 0) { + api.dispatch(removeQueryResult({ queryCacheKey })) + } + delete currentRemovalTimeouts![queryCacheKey] + }, keepUnusedDataFor * 1000) + } +} diff --git a/src/query/core/buildMiddleware/index.ts b/src/query/core/buildMiddleware/index.ts new file mode 100644 index 0000000000..921edfd264 --- /dev/null +++ b/src/query/core/buildMiddleware/index.ts @@ -0,0 +1,78 @@ +import { compose } from 'redux' + +import { + AnyAction, + createAction, + Middleware, + ThunkDispatch, +} from '@reduxjs/toolkit' + +import { + EndpointDefinitions, + FullTagDescription, +} from '../../endpointDefinitions' +import { QueryStatus, QuerySubState, RootState } from '../apiState' +import { QueryThunkArg } from '../buildThunks' +import { build as buildCacheCollection } from './cacheCollection' +import { build as buildInvalidationByTags } from './invalidationByTags' +import { build as buildPolling } from './polling' +import { BuildMiddlewareInput } from './types' +import { build as buildWindowEventHandling } from './windowEventHandling' + +export function buildMiddleware< + Definitions extends EndpointDefinitions, + ReducerPath extends string, + TagTypes extends string +>(input: BuildMiddlewareInput) { + const { reducerPath, queryThunk } = input + const actions = { + invalidateTags: createAction< + Array> + >(`${reducerPath}/invalidateTags`), + } + + const middlewares = [ + buildCacheCollection, + buildInvalidationByTags, + buildPolling, + buildWindowEventHandling, + ].map((build) => + build({ + ...((input as any) as BuildMiddlewareInput< + EndpointDefinitions, + string, + string + >), + refetchQuery, + }) + ) + const middleware: Middleware< + {}, + RootState, + ThunkDispatch + > = (mwApi) => (next) => { + const chain = middlewares.map((middleware) => middleware(mwApi)) + return compose(...chain)(next) + } + + return { middleware, actions } + + function refetchQuery( + querySubState: Exclude< + QuerySubState, + { status: QueryStatus.uninitialized } + >, + queryCacheKey: string, + override: Partial = {} + ) { + return queryThunk({ + endpointName: querySubState.endpointName, + originalArgs: querySubState.originalArgs, + subscribe: false, + forceRefetch: true, + startedTimeStamp: Date.now(), + queryCacheKey: queryCacheKey as any, + ...override, + }) + } +} diff --git a/src/query/core/buildMiddleware/invalidationByTags.ts b/src/query/core/buildMiddleware/invalidationByTags.ts new file mode 100644 index 0000000000..664c75d331 --- /dev/null +++ b/src/query/core/buildMiddleware/invalidationByTags.ts @@ -0,0 +1,100 @@ +import { isAnyOf, isFulfilled, isRejectedWithValue } from '@reduxjs/toolkit' + +import { + calculateProvidedBy, + FullTagDescription, +} from '../../endpointDefinitions' +import { flatten } from '../../utils' +import { QueryCacheKey, QueryStatus } from '../apiState' +import { calculateProvidedByThunk } from '../buildThunks' +import { SubMiddlewareApi, SubMiddlewareBuilder } from './types' + +export const build: SubMiddlewareBuilder = ({ + reducerPath, + context, + context: { endpointDefinitions }, + mutationThunk, + api, + assertTagType, + refetchQuery, +}) => { + const { removeQueryResult } = api.internalActions + + return (mwApi) => (next) => (action): any => { + const result = next(action) + + if ( + isAnyOf( + isFulfilled(mutationThunk), + isRejectedWithValue(mutationThunk) + )(action) + ) { + invalidateTags( + calculateProvidedByThunk( + action, + 'invalidatesTags', + endpointDefinitions, + assertTagType + ), + mwApi + ) + } + + if (api.util.invalidateTags.match(action)) { + invalidateTags( + calculateProvidedBy( + action.payload, + undefined, + undefined, + undefined, + assertTagType + ), + mwApi + ) + } + + return result + } + + function invalidateTags( + tags: readonly FullTagDescription[], + api: SubMiddlewareApi + ) { + const state = api.getState()[reducerPath] + + const toInvalidate = new Set() + for (const tag of tags) { + const provided = state.provided[tag.type] + if (!provided) { + continue + } + + let invalidateSubscriptions = + (tag.id !== undefined + ? // id given: invalidate all queries that provide this type & id + provided[tag.id] + : // no id: invalidate all queries that provide this type + flatten(Object.values(provided))) ?? [] + + for (const invalidate of invalidateSubscriptions) { + toInvalidate.add(invalidate) + } + } + + context.batch(() => { + const valuesArray = Array.from(toInvalidate.values()) + for (const queryCacheKey of valuesArray) { + const querySubState = state.queries[queryCacheKey] + const subscriptionSubState = state.subscriptions[queryCacheKey] + if (querySubState && subscriptionSubState) { + if (Object.keys(subscriptionSubState).length === 0) { + api.dispatch(removeQueryResult({ queryCacheKey })) + } else if (querySubState.status !== QueryStatus.uninitialized) { + api.dispatch(refetchQuery(querySubState, queryCacheKey)) + } else { + } + } + } + }) + } +} diff --git a/src/query/core/buildMiddleware/polling.ts b/src/query/core/buildMiddleware/polling.ts new file mode 100644 index 0000000000..a8878f19c8 --- /dev/null +++ b/src/query/core/buildMiddleware/polling.ts @@ -0,0 +1,132 @@ +import { QueryStatus, QuerySubstateIdentifier, Subscribers } from '../apiState' +import { + QueryStateMeta, + SubMiddlewareApi, + SubMiddlewareBuilder, + TimeoutId, +} from './types' + +export const build: SubMiddlewareBuilder = ({ + reducerPath, + queryThunk, + api, + refetchQuery, +}) => { + const currentPolls: QueryStateMeta<{ + nextPollTimestamp: number + timeout?: TimeoutId + pollingInterval: number + }> = {} + + return (mwApi) => (next) => (action): any => { + const result = next(action) + + if (api.internalActions.updateSubscriptionOptions.match(action)) { + updatePollingInterval(action.payload, mwApi) + } + + if ( + queryThunk.pending.match(action) || + (queryThunk.rejected.match(action) && action.meta.condition) + ) { + updatePollingInterval(action.meta.arg, mwApi) + } + + if ( + queryThunk.fulfilled.match(action) || + (queryThunk.rejected.match(action) && !action.meta.condition) + ) { + startNextPoll(action.meta.arg, mwApi) + } + + if (api.util.resetApiState.match(action)) { + clearPolls() + } + + return result + } + + function startNextPoll( + { queryCacheKey }: QuerySubstateIdentifier, + api: SubMiddlewareApi + ) { + const state = api.getState()[reducerPath] + const querySubState = state.queries[queryCacheKey] + const subscriptions = state.subscriptions[queryCacheKey] + + if (!querySubState || querySubState.status === QueryStatus.uninitialized) + return + + const lowestPollingInterval = findLowestPollingInterval(subscriptions) + if (!Number.isFinite(lowestPollingInterval)) return + + const currentPoll = currentPolls[queryCacheKey] + + if (currentPoll?.timeout) { + clearTimeout(currentPoll.timeout) + currentPoll.timeout = undefined + } + + const nextPollTimestamp = Date.now() + lowestPollingInterval + + const currentInterval: typeof currentPolls[number] = (currentPolls[ + queryCacheKey + ] = { + nextPollTimestamp, + pollingInterval: lowestPollingInterval, + timeout: setTimeout(() => { + currentInterval!.timeout = undefined + api.dispatch(refetchQuery(querySubState, queryCacheKey)) + }, lowestPollingInterval), + }) + } + + function updatePollingInterval( + { queryCacheKey }: QuerySubstateIdentifier, + api: SubMiddlewareApi + ) { + const state = api.getState()[reducerPath] + const querySubState = state.queries[queryCacheKey] + const subscriptions = state.subscriptions[queryCacheKey] + + if (!querySubState || querySubState.status === QueryStatus.uninitialized) { + return + } + + const lowestPollingInterval = findLowestPollingInterval(subscriptions) + const currentPoll = currentPolls[queryCacheKey] + + if (!Number.isFinite(lowestPollingInterval)) { + if (currentPoll?.timeout) { + clearTimeout(currentPoll.timeout) + } + delete currentPolls[queryCacheKey] + return + } + + const nextPollTimestamp = Date.now() + lowestPollingInterval + + if (!currentPoll || nextPollTimestamp < currentPoll.nextPollTimestamp) { + startNextPoll({ queryCacheKey }, api) + } + } + + function clearPolls() { + for (const [key, poll] of Object.entries(currentPolls)) { + if (poll?.timeout) clearTimeout(poll.timeout) + delete currentPolls[key] + } + } +} + +function findLowestPollingInterval(subscribers: Subscribers = {}) { + let lowestPollingInterval = Number.POSITIVE_INFINITY + for (const subscription of Object.values(subscribers)) { + if (!!subscription.pollingInterval) + lowestPollingInterval = Math.min( + subscription.pollingInterval, + lowestPollingInterval + ) + } + return lowestPollingInterval +} diff --git a/src/query/core/buildMiddleware/types.ts b/src/query/core/buildMiddleware/types.ts new file mode 100644 index 0000000000..d7a6ac1ef7 --- /dev/null +++ b/src/query/core/buildMiddleware/types.ts @@ -0,0 +1,54 @@ +import { + AnyAction, + AsyncThunk, + AsyncThunkAction, + Middleware, + MiddlewareAPI, + ThunkDispatch, +} from '@reduxjs/toolkit' + +import { Api, ApiContext } from '../../apiTypes' +import { AssertTagTypes, EndpointDefinitions } from '../../endpointDefinitions' +import { QueryStatus, QuerySubState, RootState } from '../apiState' +import { MutationThunkArg, QueryThunkArg, ThunkResult } from '../buildThunks' + +export type QueryStateMeta = Record +export type TimeoutId = ReturnType + +export interface BuildMiddlewareInput< + Definitions extends EndpointDefinitions, + ReducerPath extends string, + TagTypes extends string +> { + reducerPath: ReducerPath + context: ApiContext + queryThunk: AsyncThunk + mutationThunk: AsyncThunk + api: Api + assertTagType: AssertTagTypes +} + +export type SubMiddlewareApi = MiddlewareAPI< + ThunkDispatch, + RootState +> + +export interface BuildSubMiddlewareInput + extends BuildMiddlewareInput { + refetchQuery( + querySubState: Exclude< + QuerySubState, + { status: QueryStatus.uninitialized } + >, + queryCacheKey: string, + override?: Partial + ): AsyncThunkAction +} + +export type SubMiddlewareBuilder = ( + input: BuildSubMiddlewareInput +) => Middleware< + {}, + RootState, + ThunkDispatch +> diff --git a/src/query/core/buildMiddleware/windowEventHandling.ts b/src/query/core/buildMiddleware/windowEventHandling.ts new file mode 100644 index 0000000000..39f56e21e5 --- /dev/null +++ b/src/query/core/buildMiddleware/windowEventHandling.ts @@ -0,0 +1,58 @@ +import { QueryStatus } from '../apiState' +import { onFocus, onOnline } from '../setupListeners' +import { SubMiddlewareApi, SubMiddlewareBuilder } from './types' + +export const build: SubMiddlewareBuilder = ({ + reducerPath, + context, + refetchQuery, +}) => { + return (mwApi) => (next) => (action): any => { + const result = next(action) + + if (onFocus.match(action)) { + refetchValidQueries(mwApi, 'refetchOnFocus') + } + if (onOnline.match(action)) { + refetchValidQueries(mwApi, 'refetchOnReconnect') + } + + return result + } + + function refetchValidQueries( + api: SubMiddlewareApi, + type: 'refetchOnFocus' | 'refetchOnReconnect' + ) { + const state = api.getState()[reducerPath] + const queries = state.queries + const subscriptions = state.subscriptions + + context.batch(() => { + for (const queryCacheKey of Object.keys(subscriptions)) { + const querySubState = queries[queryCacheKey] + const subscriptionSubState = subscriptions[queryCacheKey] + + if ( + !subscriptionSubState || + !querySubState || + querySubState.status === QueryStatus.uninitialized + ) + return + + const shouldRefetch = + Object.values(subscriptionSubState).some( + (sub) => sub[type] === true + ) || + (Object.values(subscriptionSubState).every( + (sub) => sub[type] === undefined + ) && + state.config[type]) + + if (shouldRefetch) { + api.dispatch(refetchQuery(querySubState, queryCacheKey)) + } + } + }) + } +} From d3e3f2380fe5ce443187875566e6bbd311a14578 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sat, 1 May 2021 17:17:04 +0200 Subject: [PATCH 02/18] add cache lifecycle --- .../core/buildMiddleware/cacheLifecycle.ts | 118 ++++++++++++++++++ src/query/core/buildMiddleware/index.ts | 2 + src/query/endpointDefinitions.ts | 30 +++++ src/query/tests/optionalPromise.test.ts | 22 ++++ src/query/utils/toOptionalPromise.ts | 25 ++++ 5 files changed, 197 insertions(+) create mode 100644 src/query/core/buildMiddleware/cacheLifecycle.ts create mode 100644 src/query/tests/optionalPromise.test.ts create mode 100644 src/query/utils/toOptionalPromise.ts diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts new file mode 100644 index 0000000000..aaa91a507c --- /dev/null +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -0,0 +1,118 @@ +import { isAsyncThunkAction, isFulfilled } from '@reduxjs/toolkit' +import { toOptionalPromise } from '../../utils/toOptionalPromise' +import { SubMiddlewareApi, SubMiddlewareBuilder } from './types' + +export const build: SubMiddlewareBuilder = ({ + api, + reducerPath, + context, + queryThunk, + mutationThunk, +}) => { + type CacheLifecycle = { + valueResolved?(value: unknown): unknown + cleanup(): void + } + const lifecycleMap: Record = {} + + const isQueryThunk = isAsyncThunkAction(queryThunk) + const isMutationThunk = isAsyncThunkAction(mutationThunk) + const isFullfilledThunk = isFulfilled(queryThunk, mutationThunk) + + return (mwApi) => (next) => (action): any => { + const result = next(action) + + const cacheKey = getCacheKey(action) + + if (queryThunk.pending.match(action)) { + const state = mwApi.getState()[reducerPath].queries[cacheKey] + if (state?.requestId === action.meta.requestId) { + handleNewKey( + action.meta.arg.endpointName, + action.meta.arg.originalArgs, + cacheKey, + mwApi + ) + } + } else if (mutationThunk.pending.match(action)) { + const state = mwApi.getState()[reducerPath].mutations[cacheKey] + if (state) { + handleNewKey( + action.meta.arg.endpointName, + action.meta.arg.originalArgs, + cacheKey, + mwApi + ) + } + } else if (isFullfilledThunk(action)) { + const lifecycle = lifecycleMap[cacheKey] + if (lifecycle?.valueResolved) { + lifecycle.valueResolved(action.payload) + delete lifecycle.valueResolved + } + } else if ( + api.internalActions.removeQueryResult.match(action) || + api.internalActions.unsubscribeMutationResult.match(action) + ) { + const lifecycle = lifecycleMap[cacheKey] + if (lifecycle) { + delete lifecycleMap[cacheKey] + lifecycle.cleanup() + } + } else if (api.util.resetApiState.match(action)) { + for (const [cacheKey, lifecycle] of Object.entries(lifecycleMap)) { + delete lifecycleMap[cacheKey] + lifecycle.cleanup() + } + } + + return result + } + + function getCacheKey(action: any) { + if (isQueryThunk(action)) return action.meta.arg.queryCacheKey + if (isMutationThunk(action)) return action.meta.requestId + if (api.internalActions.removeQueryResult.match(action)) + return action.payload.queryCacheKey + return '' + } + + function handleNewKey( + endpointName: string, + originalArgs: any, + queryCacheKey: string, + mwApi: SubMiddlewareApi + ) { + const { onCacheEntryAdded } = context.endpointDefinitions[endpointName] + if (!onCacheEntryAdded) return + + const neverResolvedError = new Error( + 'Promise never resolved before cleanup.' + ) + let lifecycle = {} as CacheLifecycle + + const cleanup = new Promise((resolve) => { + lifecycle.cleanup = resolve + }) + const firstValueResolved = toOptionalPromise( + Promise.race([ + new Promise((resolve) => { + lifecycle.valueResolved = resolve + }), + cleanup.then(() => { + throw neverResolvedError + }), + ]) + ) + lifecycleMap[queryCacheKey] = lifecycle + const runningHandler = onCacheEntryAdded(originalArgs, mwApi, { + firstValueResolved, + cleanup, + }) + // if a `neverResolvedError` was thrown, but not handled in the running handler, do not let it leak out further + Promise.resolve(runningHandler).catch((e) => { + if (e === neverResolvedError) return + throw e + }) + } +} diff --git a/src/query/core/buildMiddleware/index.ts b/src/query/core/buildMiddleware/index.ts index 921edfd264..fbc44a47b2 100644 --- a/src/query/core/buildMiddleware/index.ts +++ b/src/query/core/buildMiddleware/index.ts @@ -18,6 +18,7 @@ import { build as buildInvalidationByTags } from './invalidationByTags' import { build as buildPolling } from './polling' import { BuildMiddlewareInput } from './types' import { build as buildWindowEventHandling } from './windowEventHandling' +import { build as buildCacheLifecycle } from './cacheLifecycle' export function buildMiddleware< Definitions extends EndpointDefinitions, @@ -36,6 +37,7 @@ export function buildMiddleware< buildInvalidationByTags, buildPolling, buildWindowEventHandling, + buildCacheLifecycle, ].map((build) => build({ ...((input as any) as BuildMiddlewareInput< diff --git a/src/query/endpointDefinitions.ts b/src/query/endpointDefinitions.ts index 5fe2218269..65c16c2844 100644 --- a/src/query/endpointDefinitions.ts +++ b/src/query/endpointDefinitions.ts @@ -17,6 +17,7 @@ import { CastAny, } from './tsHelpers' import { NEVER } from './fakeBaseQuery' +import { OptionalPromise } from './utils/toOptionalPromise' const resultType = Symbol() const baseQuery = Symbol() @@ -55,6 +56,11 @@ interface EndpointDefinitionWithQueryFn< transformResponse?: never } +export type LifecycleApi = { + dispatch: ThunkDispatch + getState: () => unknown +} + export type BaseEndpointDefinition< QueryArg, BaseQuery extends BaseQueryFn, @@ -69,6 +75,30 @@ export type BaseEndpointDefinition< [resultType]?: ResultType /* phantom type */ [baseQuery]?: BaseQuery + onCacheEntryAdded?( + arg: QueryArg, + api: LifecycleApi, + promises: { + /** + * Promise that will resolve with the first value for this cache key. + * This allows you to `await` until an actual value is in cache. + * + * If the cache entry is removed from the cache before any value has ever + * been resolved, this Promise will reject with + * `new Error('Promise never resolved before cleanup.')` + * to prevent memory leaks. + * You can just re-throw that error (or not handle it at all) - + * it will be caught outside of `cacheEntryAdded`. + */ + firstValueResolved: OptionalPromise + /** + * Promise that allows you to wait for the point in time when the cache entry + * has been removed from the cache, by not being used/subscribed to any more + * in the application for too long or by dispatching `api.util.resetApiState`. + */ + cleanup: Promise + } + ): Promise | void } & HasRequiredProps< BaseQueryExtraOptions, { extraOptions: BaseQueryExtraOptions }, diff --git a/src/query/tests/optionalPromise.test.ts b/src/query/tests/optionalPromise.test.ts new file mode 100644 index 0000000000..91e3bd5d9e --- /dev/null +++ b/src/query/tests/optionalPromise.test.ts @@ -0,0 +1,22 @@ +import { toOptionalPromise } from '../utils/toOptionalPromise' + +const id = (x: T) => x +test('should .then normally', async () => { + await expect(toOptionalPromise(Promise.resolve(5)).then(id)).resolves.toBe(5) +}) + +test('should .catch normally', async () => { + await expect(toOptionalPromise(Promise.reject(6)).catch(id)).resolves.toBe(6) +}) + +test('should .finally normally', async () => { + const finale = jest.fn() + await expect( + toOptionalPromise(Promise.reject(6)).finally(finale).catch(id) + ).resolves.toBe(6) + expect(finale).toHaveBeenCalled() +}) + +test('not interacting should not make jest freak out', () => { + toOptionalPromise(Promise.reject(6)) +}) diff --git a/src/query/utils/toOptionalPromise.ts b/src/query/utils/toOptionalPromise.ts new file mode 100644 index 0000000000..8dba4ae3d6 --- /dev/null +++ b/src/query/utils/toOptionalPromise.ts @@ -0,0 +1,25 @@ +/** + * A promise that will only throw if it has been interacted with, by calling `.then` or `.catch` on it, + * or by `await`ing it. + * + * This deals with unhandled promise rejections if the promise was never interacted with in any way. + * + * No interaction with the Promise => no error + */ +export interface OptionalPromise extends Promise {} + +/** + * Wraps a Promise in a new Promise that will only throw, if either `.then` or `.catch` have been accessed on it prior to throwing. + */ +export function toOptionalPromise(promise: Promise): OptionalPromise { + let interacted = false + const wrapped = promise.catch((e) => { + if (interacted) throw e + }) + const { then } = wrapped + wrapped.then = (...args) => { + interacted = true + return then.apply(wrapped, args) as any + } + return wrapped as Promise +} From 4e062a81a14f67f059e219792d2256ce77af9321 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sat, 1 May 2021 17:57:32 +0200 Subject: [PATCH 03/18] add `onQuery` lifecycle --- src/query/core/buildMiddleware/index.ts | 2 + .../core/buildMiddleware/queryLifecycle.ts | 52 +++++++++++++++++++ src/query/endpointDefinitions.ts | 14 +++++ 3 files changed, 68 insertions(+) create mode 100644 src/query/core/buildMiddleware/queryLifecycle.ts diff --git a/src/query/core/buildMiddleware/index.ts b/src/query/core/buildMiddleware/index.ts index fbc44a47b2..069f880f91 100644 --- a/src/query/core/buildMiddleware/index.ts +++ b/src/query/core/buildMiddleware/index.ts @@ -19,6 +19,7 @@ import { build as buildPolling } from './polling' import { BuildMiddlewareInput } from './types' import { build as buildWindowEventHandling } from './windowEventHandling' import { build as buildCacheLifecycle } from './cacheLifecycle' +import { build as buildQueryLifecycle } from './queryLifecycle' export function buildMiddleware< Definitions extends EndpointDefinitions, @@ -38,6 +39,7 @@ export function buildMiddleware< buildPolling, buildWindowEventHandling, buildCacheLifecycle, + buildQueryLifecycle, ].map((build) => build({ ...((input as any) as BuildMiddlewareInput< diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts new file mode 100644 index 0000000000..914e1f94db --- /dev/null +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -0,0 +1,52 @@ +import { isPending, isRejected, isFulfilled } from '@reduxjs/toolkit' +import { toOptionalPromise } from '../../utils/toOptionalPromise' +import { SubMiddlewareBuilder } from './types' + +export const build: SubMiddlewareBuilder = ({ + context, + queryThunk, + mutationThunk, +}) => { + type CacheLifecycle = { + resolve(value: unknown): unknown + reject(value: unknown): unknown + } + const lifecycleMap: Record = {} + + const isPendingThunk = isPending(queryThunk, mutationThunk) + const isRejectedThunk = isRejected(queryThunk, mutationThunk) + const isFullfilledThunk = isFulfilled(queryThunk, mutationThunk) + + return (mwApi) => (next) => (action): any => { + const result = next(action) + + if (isPendingThunk(action)) { + const { + requestId, + arg: { endpointName, originalArgs }, + } = action.meta + const { onQuery } = context.endpointDefinitions[endpointName] + if (onQuery) { + const lifecycle = {} as CacheLifecycle + const resultPromise = toOptionalPromise( + new Promise((resolve, reject) => { + lifecycle.resolve = resolve + lifecycle.reject = reject + }) + ) + lifecycleMap[requestId] = lifecycle + onQuery(originalArgs, mwApi, { resultPromise }) + } + } else if (isFullfilledThunk(action)) { + const { requestId } = action.meta + lifecycleMap[requestId].resolve(action.payload) + delete lifecycleMap[requestId] + } else if (isRejectedThunk(action)) { + const { requestId } = action.meta + lifecycleMap[requestId].reject(action.payload ?? action.error) + delete lifecycleMap[requestId] + } + + return result + } +} diff --git a/src/query/endpointDefinitions.ts b/src/query/endpointDefinitions.ts index 65c16c2844..9b7fe59b4c 100644 --- a/src/query/endpointDefinitions.ts +++ b/src/query/endpointDefinitions.ts @@ -99,6 +99,20 @@ export type BaseEndpointDefinition< cleanup: Promise } ): Promise | void + onQuery?( + arg: QueryArg, + api: LifecycleApi, + promises: { + /** + * Promise that will resolve with the (transformed) query result. + + * If the query fails, this promise will reject with the error. + * + * This allows you to `await` for the query to finish. + */ + resultPromise: OptionalPromise + } + ): Promise | void } & HasRequiredProps< BaseQueryExtraOptions, { extraOptions: BaseQueryExtraOptions }, From ee1cc388a01fd0f06c283769bcec62ffa656a46b Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Tue, 4 May 2021 21:48:00 +0200 Subject: [PATCH 04/18] tweaks, tests --- .../core/buildMiddleware/queryLifecycle.ts | 2 +- src/query/createApi.ts | 11 ++- src/query/tests/queryLifecycle.test.tsx | 98 +++++++++++++++++++ src/query/utils/toOptionalPromise.ts | 17 ++-- 4 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 src/query/tests/queryLifecycle.test.tsx diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index 914e1f94db..78ac900944 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -39,7 +39,7 @@ export const build: SubMiddlewareBuilder = ({ } } else if (isFullfilledThunk(action)) { const { requestId } = action.meta - lifecycleMap[requestId].resolve(action.payload) + lifecycleMap[requestId].resolve(action.payload.result) delete lifecycleMap[requestId] } else if (isRejectedThunk(action)) { const { requestId } = action.meta diff --git a/src/query/createApi.ts b/src/query/createApi.ts index 68953883a6..ce1bd575ff 100644 --- a/src/query/createApi.ts +++ b/src/query/createApi.ts @@ -300,18 +300,19 @@ export function buildCreateApi, ...Module[]]>( evaluatedEndpoints )) { if ( - typeof process !== 'undefined' && - process.env.NODE_ENV === 'development' + !inject.overrideExisting && + endpointName in context.endpointDefinitions ) { if ( - !inject.overrideExisting && - endpointName in context.endpointDefinitions + typeof process !== 'undefined' && + process.env.NODE_ENV === 'development' ) { console.error( `called \`injectEndpoints\` to override already-existing endpointName ${endpointName} without specifying \`overrideExisting: true\`` ) - continue } + + continue } context.endpointDefinitions[endpointName] = definition for (const m of initializedModules) { diff --git a/src/query/tests/queryLifecycle.test.tsx b/src/query/tests/queryLifecycle.test.tsx new file mode 100644 index 0000000000..2b5b7a5ec7 --- /dev/null +++ b/src/query/tests/queryLifecycle.test.tsx @@ -0,0 +1,98 @@ +import { createApi } from '@reduxjs/toolkit/query' +import { waitFor } from '@testing-library/react' +import { fetchBaseQuery } from '../fetchBaseQuery' +import { setupApiStore } from './helpers' + +const api = createApi({ + baseQuery: fetchBaseQuery({ baseUrl: 'http://example.com' }), + endpoints: () => ({}), +}) + +const onStart = jest.fn() +const onSuccess = jest.fn() +const onError = jest.fn() + +beforeEach(() => { + onStart.mockClear() + onSuccess.mockClear() + onError.mockClear() +}) + +const storeRef = setupApiStore(api) + +test('query: onStart only', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/success', + onQuery(arg) { + onStart(arg) + }, + }), + }), + }) + storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) + expect(onStart).toHaveBeenCalledWith('arg') +}) + +test('query: onStart and onSuccess', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/success', + async onQuery(arg, {}, { resultPromise }) { + onStart(arg) + // awaiting without catching like this would result in an `unhandledRejection` exception if there was an error + // unfortunately we cannot test for that in jest. + const result = await resultPromise + onSuccess(result) + }, + }), + }), + }) + storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) + expect(onStart).toHaveBeenCalledWith('arg') + await waitFor(() => { + expect(onSuccess).toHaveBeenCalledWith({ value: 'success' }) + }) +}) + +test('query: onStart, onSuccess and onError', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/error', + async onQuery(arg, {}, { resultPromise }) { + onStart(arg) + try { + const result = await resultPromise + onSuccess(result) + } catch (e) { + onError(e) + } + }, + }), + }), + }) + storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) + expect(onStart).toHaveBeenCalledWith('arg') + await waitFor(() => { + expect(onError).toHaveBeenCalledWith({ + status: 500, + data: { value: 'error' }, + }) + }) + expect(onSuccess).not.toHaveBeenCalled() +}) + +/* + other test scenarios: + + + cleanup happens before the query resolves -> should reject the promise + + cleanup happens before the query resolves -> should reject the promise, but the promise should not cause an unhandledRejection if not caught +*/ diff --git a/src/query/utils/toOptionalPromise.ts b/src/query/utils/toOptionalPromise.ts index 8dba4ae3d6..a7d92a70f0 100644 --- a/src/query/utils/toOptionalPromise.ts +++ b/src/query/utils/toOptionalPromise.ts @@ -6,7 +6,8 @@ * * No interaction with the Promise => no error */ -export interface OptionalPromise extends Promise {} +export interface OptionalPromise + extends Pick, 'then' | 'catch'> {} /** * Wraps a Promise in a new Promise that will only throw, if either `.then` or `.catch` have been accessed on it prior to throwing. @@ -16,10 +17,14 @@ export function toOptionalPromise(promise: Promise): OptionalPromise { const wrapped = promise.catch((e) => { if (interacted) throw e }) - const { then } = wrapped - wrapped.then = (...args) => { - interacted = true - return then.apply(wrapped, args) as any + return { + then(onresolve: any, onreject) { + interacted = true + return wrapped.then(onresolve, onreject) as any + }, + catch(onreject) { + interacted = true + return wrapped.catch(onreject) as any + }, } - return wrapped as Promise } From 925c4550547b9ef1993b6f5fae6ec75645b5e20f Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Tue, 4 May 2021 22:23:38 +0200 Subject: [PATCH 05/18] describe scenarios for testing cacheLifecycle --- .../core/buildMiddleware/cacheLifecycle.ts | 2 +- src/query/tests/cacheLifecycle.test.ts | 61 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 src/query/tests/cacheLifecycle.test.ts diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index aaa91a507c..d02427931c 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -47,7 +47,7 @@ export const build: SubMiddlewareBuilder = ({ } else if (isFullfilledThunk(action)) { const lifecycle = lifecycleMap[cacheKey] if (lifecycle?.valueResolved) { - lifecycle.valueResolved(action.payload) + lifecycle.valueResolved(action.payload.result) delete lifecycle.valueResolved } } else if ( diff --git a/src/query/tests/cacheLifecycle.test.ts b/src/query/tests/cacheLifecycle.test.ts new file mode 100644 index 0000000000..7414dc584f --- /dev/null +++ b/src/query/tests/cacheLifecycle.test.ts @@ -0,0 +1,61 @@ +const onNewCacheEntry = jest.fn() +const gotFirstValue = jest.fn() +const onCleanup = jest.fn() + +// scenarios: +/* +onCacheEntryAdded((arg) => { + onNewCacheEntry(arg) +}) +*/ + +/* +onCacheEntryAdded((arg) => { + onNewCacheEntry(arg) + await cleanupPromise + onCleanup() +}) +*/ + +/* +onCacheEntryAdded((arg) => { + onNewCacheEntry(arg) + const value = await valueResolvedPromise + gotFirstValue(value) + await cleanupPromise + onCleanup() +}) +// extra scenario to test & document: +// if cleanup happens before `valueResolvedPromise` resolves, `valueResolvedPromise` will throw, *also* skipping `onCleanup` +*/ + +/* +// recommended if some cleanup is always necessary: + +onCacheEntryAdded((arg) => { + onNewCacheEntry(arg) + try { + const value = await valueResolvedPromise + gotFirstValue(value) + } catch { + neverGotAValue() + } + await cleanupPromise + onCleanup() +}) +*/ + +/* +// recommended if cleanup is only necessary when something was started after first value in cache: + +onCacheEntryAdded((arg) => { + onNewCacheEntry(arg) + try { + const value = await valueResolvedPromise + gotFirstValue(value) + await cleanupPromise + onCleanup() + } catch { + } +}) +*/ From 0e9571e82547708095e598e9f793e4db8a87b6a1 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Wed, 5 May 2021 19:31:52 +0200 Subject: [PATCH 06/18] basic lifecycle tests --- .../core/buildMiddleware/queryLifecycle.ts | 4 +- src/query/tests/cacheLifecycle.test.ts | 72 ++++++++++++++++--- src/query/tests/queryLifecycle.test.tsx | 3 +- 3 files changed, 64 insertions(+), 15 deletions(-) diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index 78ac900944..8dd9f475d5 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -39,11 +39,11 @@ export const build: SubMiddlewareBuilder = ({ } } else if (isFullfilledThunk(action)) { const { requestId } = action.meta - lifecycleMap[requestId].resolve(action.payload.result) + lifecycleMap[requestId]?.resolve(action.payload.result) delete lifecycleMap[requestId] } else if (isRejectedThunk(action)) { const { requestId } = action.meta - lifecycleMap[requestId].reject(action.payload ?? action.error) + lifecycleMap[requestId]?.reject(action.payload ?? action.error) delete lifecycleMap[requestId] } diff --git a/src/query/tests/cacheLifecycle.test.ts b/src/query/tests/cacheLifecycle.test.ts index 7414dc584f..1a7c9a0041 100644 --- a/src/query/tests/cacheLifecycle.test.ts +++ b/src/query/tests/cacheLifecycle.test.ts @@ -1,21 +1,71 @@ +import { createApi } from '@reduxjs/toolkit/query' +import { waitFor, waitForDomChange } from '@testing-library/react' +import { fetchBaseQuery } from '../fetchBaseQuery' +import { setupApiStore, waitMs } from './helpers' + +beforeAll(() => { + jest.useFakeTimers() +}) + +const api = createApi({ + baseQuery: fetchBaseQuery({ baseUrl: 'http://example.com' }), + endpoints: () => ({}), +}) +const storeRef = setupApiStore(api) + const onNewCacheEntry = jest.fn() const gotFirstValue = jest.fn() const onCleanup = jest.fn() -// scenarios: -/* -onCacheEntryAdded((arg) => { - onNewCacheEntry(arg) +beforeEach(() => { + onNewCacheEntry.mockClear() + gotFirstValue.mockClear() + onCleanup.mockClear() }) -*/ -/* -onCacheEntryAdded((arg) => { - onNewCacheEntry(arg) - await cleanupPromise - onCleanup() +test('query: new cache entry only', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/success', + onCacheEntryAdded(arg) { + onNewCacheEntry(arg) + }, + }), + }), + }) + storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') +}) + +test('query: cleaning up', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/success', + async onCacheEntryAdded(arg, {}, { cleanup }) { + onNewCacheEntry(arg) + await cleanup + onCleanup() + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + promise.unsubscribe() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(59000) + await waitMs() // wait a few ticks + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000) + await waitMs() // wait a few ticks + expect(onCleanup).toHaveBeenCalled() }) -*/ /* onCacheEntryAdded((arg) => { diff --git a/src/query/tests/queryLifecycle.test.tsx b/src/query/tests/queryLifecycle.test.tsx index 2b5b7a5ec7..7e8ad269b7 100644 --- a/src/query/tests/queryLifecycle.test.tsx +++ b/src/query/tests/queryLifecycle.test.tsx @@ -7,6 +7,7 @@ const api = createApi({ baseQuery: fetchBaseQuery({ baseUrl: 'http://example.com' }), endpoints: () => ({}), }) +const storeRef = setupApiStore(api) const onStart = jest.fn() const onSuccess = jest.fn() @@ -18,8 +19,6 @@ beforeEach(() => { onError.mockClear() }) -const storeRef = setupApiStore(api) - test('query: onStart only', async () => { const extended = api.injectEndpoints({ overrideExisting: true, From c2f894e9a8b9667e7896b1296724900d87ab2e03 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Wed, 5 May 2021 22:41:48 +0200 Subject: [PATCH 07/18] add `getCacheEntry`, add more tests --- .../core/buildMiddleware/cacheLifecycle.ts | 16 +- .../core/buildMiddleware/queryLifecycle.ts | 14 +- src/query/endpointDefinitions.ts | 129 ++++--- src/query/tests/cacheLifecycle.test.ts | 323 +++++++++++++++--- src/query/tests/helpers.tsx | 3 +- src/query/tests/queryLifecycle.test.tsx | 123 +++++++ src/query/utils/toOptionalPromise.ts | 6 +- 7 files changed, 518 insertions(+), 96 deletions(-) diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index d02427931c..2b6b752dd5 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -105,10 +105,18 @@ export const build: SubMiddlewareBuilder = ({ ]) ) lifecycleMap[queryCacheKey] = lifecycle - const runningHandler = onCacheEntryAdded(originalArgs, mwApi, { - firstValueResolved, - cleanup, - }) + const selector = (api.endpoints[endpointName] as any).select(originalArgs) + const runningHandler = onCacheEntryAdded( + originalArgs, + { + ...mwApi, + getCacheEntry: () => selector(mwApi.getState()), + }, + { + firstValueResolved, + cleanup, + } + ) // if a `neverResolvedError` was thrown, but not handled in the running handler, do not let it leak out further Promise.resolve(runningHandler).catch((e) => { if (e === neverResolvedError) return diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index 8dd9f475d5..5725f29e24 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -3,6 +3,7 @@ import { toOptionalPromise } from '../../utils/toOptionalPromise' import { SubMiddlewareBuilder } from './types' export const build: SubMiddlewareBuilder = ({ + api, context, queryThunk, mutationThunk, @@ -35,7 +36,18 @@ export const build: SubMiddlewareBuilder = ({ }) ) lifecycleMap[requestId] = lifecycle - onQuery(originalArgs, mwApi, { resultPromise }) + const selector = (api.endpoints[endpointName] as any).select( + originalArgs + ) + + onQuery( + originalArgs, + { + ...mwApi, + getCacheEntry: () => selector(mwApi.getState()), + }, + { resultPromise } + ) } } else if (isFullfilledThunk(action)) { const { requestId } = action.meta diff --git a/src/query/endpointDefinitions.ts b/src/query/endpointDefinitions.ts index 9b7fe59b4c..22eeb8293f 100644 --- a/src/query/endpointDefinitions.ts +++ b/src/query/endpointDefinitions.ts @@ -1,5 +1,9 @@ import { AnyAction, ThunkDispatch } from '@reduxjs/toolkit' import { RootState } from './core/apiState' +import { + QueryResultSelectorResult, + MutationResultSelectorResult, +} from './core/buildSelectors' import { BaseQueryExtraOptions, BaseQueryFn, @@ -56,9 +60,42 @@ interface EndpointDefinitionWithQueryFn< transformResponse?: never } -export type LifecycleApi = { +export type LifecycleApi = { dispatch: ThunkDispatch getState: () => unknown + getCacheEntry: () => CacheEntry +} + +export interface CacheLifecyclePromises { + /** + * Promise that will resolve with the first value for this cache key. + * This allows you to `await` until an actual value is in cache. + * + * If the cache entry is removed from the cache before any value has ever + * been resolved, this Promise will reject with + * `new Error('Promise never resolved before cleanup.')` + * to prevent memory leaks. + * You can just re-throw that error (or not handle it at all) - + * it will be caught outside of `cacheEntryAdded`. + */ + firstValueResolved: OptionalPromise + /** + * Promise that allows you to wait for the point in time when the cache entry + * has been removed from the cache, by not being used/subscribed to any more + * in the application for too long or by dispatching `api.util.resetApiState`. + */ + cleanup: Promise +} + +interface QueryLifecyclePromises { + /** + * Promise that will resolve with the (transformed) query result. + + * If the query fails, this promise will reject with the error. + * + * This allows you to `await` for the query to finish. + */ + resultPromise: OptionalPromise } export type BaseEndpointDefinition< @@ -75,44 +112,6 @@ export type BaseEndpointDefinition< [resultType]?: ResultType /* phantom type */ [baseQuery]?: BaseQuery - onCacheEntryAdded?( - arg: QueryArg, - api: LifecycleApi, - promises: { - /** - * Promise that will resolve with the first value for this cache key. - * This allows you to `await` until an actual value is in cache. - * - * If the cache entry is removed from the cache before any value has ever - * been resolved, this Promise will reject with - * `new Error('Promise never resolved before cleanup.')` - * to prevent memory leaks. - * You can just re-throw that error (or not handle it at all) - - * it will be caught outside of `cacheEntryAdded`. - */ - firstValueResolved: OptionalPromise - /** - * Promise that allows you to wait for the point in time when the cache entry - * has been removed from the cache, by not being used/subscribed to any more - * in the application for too long or by dispatching `api.util.resetApiState`. - */ - cleanup: Promise - } - ): Promise | void - onQuery?( - arg: QueryArg, - api: LifecycleApi, - promises: { - /** - * Promise that will resolve with the (transformed) query result. - - * If the query fails, this promise will reject with the error. - * - * This allows you to `await` for the query to finish. - */ - resultPromise: OptionalPromise - } - ): Promise | void } & HasRequiredProps< BaseQueryExtraOptions, { extraOptions: BaseQueryExtraOptions }, @@ -239,6 +238,32 @@ interface QueryExtraOptions< result: ResultType, meta: BaseQueryMeta | undefined ): void + onCacheEntryAdded?( + arg: QueryArg, + api: LifecycleApi< + QueryResultSelectorResult< + { type: DefinitionType.query } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + >, + promises: CacheLifecyclePromises + ): Promise | void + onQuery?( + arg: QueryArg, + api: LifecycleApi< + QueryResultSelectorResult< + { type: DefinitionType.query } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + >, + promises: QueryLifecyclePromises + ): Promise | void } export type QueryDefinition< @@ -345,6 +370,32 @@ interface MutationExtraOptions< result: ResultType, meta: BaseQueryMeta | undefined ): void + onCacheEntryAdded?( + arg: QueryArg, + api: LifecycleApi< + MutationResultSelectorResult< + { type: DefinitionType.mutation } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + >, + promises: CacheLifecyclePromises + ): Promise | void + onQuery?( + arg: QueryArg, + api: LifecycleApi< + MutationResultSelectorResult< + { type: DefinitionType.mutation } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + >, + promises: QueryLifecyclePromises + ): Promise | void } export type MutationDefinition< diff --git a/src/query/tests/cacheLifecycle.test.ts b/src/query/tests/cacheLifecycle.test.ts index 1a7c9a0041..b067ac474b 100644 --- a/src/query/tests/cacheLifecycle.test.ts +++ b/src/query/tests/cacheLifecycle.test.ts @@ -16,11 +16,13 @@ const storeRef = setupApiStore(api) const onNewCacheEntry = jest.fn() const gotFirstValue = jest.fn() const onCleanup = jest.fn() +const onCatch = jest.fn() beforeEach(() => { onNewCacheEntry.mockClear() gotFirstValue.mockClear() onCleanup.mockClear() + onCatch.mockClear() }) test('query: new cache entry only', async () => { @@ -29,7 +31,7 @@ test('query: new cache entry only', async () => { endpoints: (build) => ({ injected: build.query({ query: () => '/success', - onCacheEntryAdded(arg) { + onCacheEntryAdded(arg, { dispatch, getState }) { onNewCacheEntry(arg) }, }), @@ -39,13 +41,13 @@ test('query: new cache entry only', async () => { expect(onNewCacheEntry).toHaveBeenCalledWith('arg') }) -test('query: cleaning up', async () => { +test('query: await cleanup', async () => { const extended = api.injectEndpoints({ overrideExisting: true, endpoints: (build) => ({ injected: build.query({ query: () => '/success', - async onCacheEntryAdded(arg, {}, { cleanup }) { + async onCacheEntryAdded(arg, { dispatch, getState }, { cleanup }) { onNewCacheEntry(arg) await cleanup onCleanup() @@ -53,59 +55,280 @@ test('query: cleaning up', async () => { }), }), }) - const promise = storeRef.store.dispatch( - extended.endpoints.injected.initiate('arg') - ) + storeRef.store + .dispatch(extended.endpoints.injected.initiate('arg')) + .unsubscribe() + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') - promise.unsubscribe() expect(onCleanup).not.toHaveBeenCalled() - jest.advanceTimersByTime(59000) - await waitMs() // wait a few ticks + jest.advanceTimersByTime(59000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000), await waitMs() + expect(onCleanup).toHaveBeenCalled() +}) + +test('query: await firstValueResolved, await cleanup (success)', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/success', + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + await cleanup + onCleanup() + }, + }), + }), + }) + storeRef.store + .dispatch(extended.endpoints.injected.initiate('arg')) + .unsubscribe() + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCleanup).not.toHaveBeenCalled() + + await waitFor(() => { + expect(gotFirstValue).toHaveBeenCalled() + }) + expect(gotFirstValue).toHaveBeenCalledWith({ value: 'success' }) + expect(onCleanup).not.toHaveBeenCalled() + + jest.advanceTimersByTime(59000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000), await waitMs() + expect(onCleanup).toHaveBeenCalled() +}) + +test('query: await firstValueResolved, await cleanup (firstValueResolved never resolves)', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + // this will wait until cleanup, then reject => nothing past that line will execute + // but since this special "cleanup" rejection is handled outside, there will be no + // uncaught rejection error + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + await cleanup + onCleanup() + }, + }), + }), + }) + storeRef.store + .dispatch(extended.endpoints.injected.initiate('arg')) + .unsubscribe() + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + + jest.advanceTimersByTime(120000), await waitMs() + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCleanup).not.toHaveBeenCalled() +}) + +test('query: try { await firstValueResolved }, await cleanup (firstValueResolved never resolves)', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + + try { + // this will wait until cleanup, then reject => nothing else in this try..catch block will execute + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + } catch (e) { + onCatch(e) + } + await cleanup + onCleanup() + }, + }), + }), + }) + storeRef.store + .dispatch(extended.endpoints.injected.initiate('arg')) + .unsubscribe() + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + + jest.advanceTimersByTime(59000), await waitMs() expect(onCleanup).not.toHaveBeenCalled() - jest.advanceTimersByTime(2000) - await waitMs() // wait a few ticks + jest.advanceTimersByTime(2000), await waitMs() expect(onCleanup).toHaveBeenCalled() + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCatch.mock.calls[0][0]).toMatchObject({ + message: 'Promise never resolved before cleanup.', + }) }) -/* -onCacheEntryAdded((arg) => { - onNewCacheEntry(arg) - const value = await valueResolvedPromise - gotFirstValue(value) - await cleanupPromise - onCleanup() +test('query: try { await firstValueResolved, await cleanup } (firstValueResolved never resolves)', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + + try { + // this will wait until cleanup, then reject => nothing else in this try..catch block will execute + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + // cleanup in this scenario only needs to be done for stuff within this try..catch block - totally valid scenario + await cleanup + onCleanup() + } catch (e) { + onCatch(e) + } + }, + }), + }), + }) + storeRef.store + .dispatch(extended.endpoints.injected.initiate('arg')) + .unsubscribe() + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + + jest.advanceTimersByTime(59000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCatch.mock.calls[0][0]).toMatchObject({ + message: 'Promise never resolved before cleanup.', + }) }) -// extra scenario to test & document: -// if cleanup happens before `valueResolvedPromise` resolves, `valueResolvedPromise` will throw, *also* skipping `onCleanup` -*/ - -/* -// recommended if some cleanup is always necessary: - -onCacheEntryAdded((arg) => { - onNewCacheEntry(arg) - try { - const value = await valueResolvedPromise - gotFirstValue(value) - } catch { - neverGotAValue() - } - await cleanupPromise - onCleanup() + +test('query: try { await firstValueResolved } finally { await cleanup } (firstValueResolved never resolves)', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + + try { + // this will wait until cleanup, then reject => nothing else in this try..catch block will execute + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + } catch (e) { + onCatch(e) + } finally { + await cleanup + onCleanup() + } + }, + }), + }), + }) + storeRef.store + .dispatch(extended.endpoints.injected.initiate('arg')) + .unsubscribe() + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + + jest.advanceTimersByTime(59000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000), await waitMs() + expect(onCleanup).toHaveBeenCalled() + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCatch.mock.calls[0][0]).toMatchObject({ + message: 'Promise never resolved before cleanup.', + }) }) -*/ - -/* -// recommended if cleanup is only necessary when something was started after first value in cache: - -onCacheEntryAdded((arg) => { - onNewCacheEntry(arg) - try { - const value = await valueResolvedPromise - gotFirstValue(value) - await cleanupPromise - onCleanup() - } catch { - } + +test('getCacheEntry', async () => { + const snapshot = jest.fn() + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/success', + async onCacheEntryAdded( + arg, + { dispatch, getState, getCacheEntry }, + { cleanup, firstValueResolved } + ) { + snapshot(getCacheEntry()) + gotFirstValue(await firstValueResolved) + snapshot(getCacheEntry()) + await cleanup + snapshot(getCacheEntry()) + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + promise.unsubscribe() + + await waitFor(() => { + expect(gotFirstValue).toHaveBeenCalled() + }) + + jest.advanceTimersByTime(120000), await waitMs() + + expect(snapshot).toHaveBeenCalledTimes(3) + expect(snapshot.mock.calls[0][0]).toMatchObject({ + endpointName: 'injected', + isError: false, + isLoading: true, + isSuccess: false, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'pending', + }) + expect(snapshot.mock.calls[1][0]).toMatchObject({ + data: { + value: 'success', + }, + endpointName: 'injected', + fulfilledTimeStamp: expect.any(Number), + isError: false, + isLoading: false, + isSuccess: true, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'fulfilled', + }) + expect(snapshot.mock.calls[2][0]).toMatchObject({ + isError: false, + isLoading: false, + isSuccess: false, + isUninitialized: true, + status: 'uninitialized', + }) }) -*/ diff --git a/src/query/tests/helpers.tsx b/src/query/tests/helpers.tsx index 5330e36f5a..951b84d35e 100644 --- a/src/query/tests/helpers.tsx +++ b/src/query/tests/helpers.tsx @@ -108,7 +108,7 @@ export function setupApiStore< A extends { reducerPath: any reducer: Reducer - middleware: Middleware + middleware: Middleware util: { resetApiState(): any } }, R extends Record> @@ -121,6 +121,7 @@ export function setupApiStore< api.middleware ), }) + type StoreType = ReturnType extends EnhancedStore< {}, any, diff --git a/src/query/tests/queryLifecycle.test.tsx b/src/query/tests/queryLifecycle.test.tsx index 7e8ad269b7..2760413cf6 100644 --- a/src/query/tests/queryLifecycle.test.tsx +++ b/src/query/tests/queryLifecycle.test.tsx @@ -87,6 +87,129 @@ test('query: onStart, onSuccess and onError', async () => { expect(onSuccess).not.toHaveBeenCalled() }) +test('getCacheEntry (success)', async () => { + const snapshot = jest.fn() + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/success', + async onQuery( + arg, + { dispatch, getState, getCacheEntry }, + { resultPromise } + ) { + try { + snapshot(getCacheEntry()) + const result = await resultPromise + onSuccess(result) + snapshot(getCacheEntry()) + } catch (e) { + onError(e) + snapshot(getCacheEntry()) + } + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + + await waitFor(() => { + expect(onSuccess).toHaveBeenCalled() + }) + + expect(snapshot).toHaveBeenCalledTimes(2) + expect(snapshot.mock.calls[0][0]).toMatchObject({ + endpointName: 'injected', + isError: false, + isLoading: true, + isSuccess: false, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'pending', + }) + expect(snapshot.mock.calls[1][0]).toMatchObject({ + data: { + value: 'success', + }, + endpointName: 'injected', + fulfilledTimeStamp: expect.any(Number), + isError: false, + isLoading: false, + isSuccess: true, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'fulfilled', + }) +}) + +test('getCacheEntry (success)', async () => { + const snapshot = jest.fn() + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/error', + async onQuery( + arg, + { dispatch, getState, getCacheEntry }, + { resultPromise } + ) { + try { + snapshot(getCacheEntry()) + const result = await resultPromise + onSuccess(result) + snapshot(getCacheEntry()) + } catch (e) { + onError(e) + snapshot(getCacheEntry()) + } + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + + await waitFor(() => { + expect(onError).toHaveBeenCalled() + }) + + expect(snapshot.mock.calls[0][0]).toMatchObject({ + endpointName: 'injected', + isError: false, + isLoading: true, + isSuccess: false, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'pending', + }) + expect(snapshot.mock.calls[1][0]).toMatchObject({ + error: { + data: { value: 'error' }, + status: 500, + }, + endpointName: 'injected', + isError: true, + isLoading: false, + isSuccess: false, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'rejected', + }) +}) + /* other test scenarios: diff --git a/src/query/utils/toOptionalPromise.ts b/src/query/utils/toOptionalPromise.ts index a7d92a70f0..4c5add1afc 100644 --- a/src/query/utils/toOptionalPromise.ts +++ b/src/query/utils/toOptionalPromise.ts @@ -7,7 +7,7 @@ * No interaction with the Promise => no error */ export interface OptionalPromise - extends Pick, 'then' | 'catch'> {} + extends Pick, 'then' | 'catch' | 'finally'> {} /** * Wraps a Promise in a new Promise that will only throw, if either `.then` or `.catch` have been accessed on it prior to throwing. @@ -26,5 +26,9 @@ export function toOptionalPromise(promise: Promise): OptionalPromise { interacted = true return wrapped.catch(onreject) as any }, + finally(cb) { + interacted = true + return wrapped.finally(cb) as any + }, } } From 568d52041c7c31975174ef9933078af917bca7cd Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Wed, 5 May 2021 22:56:59 +0200 Subject: [PATCH 08/18] small guard, fix tests --- src/query/core/buildMiddleware/cacheLifecycle.ts | 3 ++- src/query/core/buildMiddleware/queryLifecycle.ts | 2 +- src/query/tests/buildHooks.test.tsx | 2 +- src/query/tests/cacheLifecycle.test.ts | 2 +- src/query/tests/createApi.test.ts | 4 ++-- src/query/tests/optimisticUpdates.test.tsx | 2 +- src/query/tests/refetchingBehaviors.test.tsx | 2 +- 7 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index 2b6b752dd5..3f03b25167 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -83,7 +83,8 @@ export const build: SubMiddlewareBuilder = ({ queryCacheKey: string, mwApi: SubMiddlewareApi ) { - const { onCacheEntryAdded } = context.endpointDefinitions[endpointName] + const onCacheEntryAdded = + context.endpointDefinitions[endpointName]?.onCacheEntryAdded if (!onCacheEntryAdded) return const neverResolvedError = new Error( diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index 5725f29e24..7a97548bb5 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -26,7 +26,7 @@ export const build: SubMiddlewareBuilder = ({ requestId, arg: { endpointName, originalArgs }, } = action.meta - const { onQuery } = context.endpointDefinitions[endpointName] + const onQuery = context.endpointDefinitions[endpointName]?.onQuery if (onQuery) { const lifecycle = {} as CacheLifecycle const resultPromise = toOptionalPromise( diff --git a/src/query/tests/buildHooks.test.tsx b/src/query/tests/buildHooks.test.tsx index 1f02a2dbdc..dbc58f1324 100644 --- a/src/query/tests/buildHooks.test.tsx +++ b/src/query/tests/buildHooks.test.tsx @@ -1841,7 +1841,7 @@ describe('hooks with createApi defaults set', () => { }), }) - const storeRef = setupApiStore(defaultApi, { + const storeRef = setupApiStore(api, { ...actionsReducer, }) diff --git a/src/query/tests/cacheLifecycle.test.ts b/src/query/tests/cacheLifecycle.test.ts index b067ac474b..27b0fd0400 100644 --- a/src/query/tests/cacheLifecycle.test.ts +++ b/src/query/tests/cacheLifecycle.test.ts @@ -1,5 +1,5 @@ import { createApi } from '@reduxjs/toolkit/query' -import { waitFor, waitForDomChange } from '@testing-library/react' +import { waitFor } from '@testing-library/react' import { fetchBaseQuery } from '../fetchBaseQuery' import { setupApiStore, waitMs } from './helpers' diff --git a/src/query/tests/createApi.test.ts b/src/query/tests/createApi.test.ts index 8ae5524e5b..8ab76b7416 100644 --- a/src/query/tests/createApi.test.ts +++ b/src/query/tests/createApi.test.ts @@ -530,7 +530,7 @@ describe('additional transformResponse behaviors', () => { api.endpoints.mutation.initiate({}) ) - expect(result.data).toEqual({ banana: 'bread' }) + expect('data' in result && result.data).toEqual({ banana: 'bread' }) }) test('transformResponse can inject baseQuery meta into the end result from a mutation', async () => { @@ -538,7 +538,7 @@ describe('additional transformResponse behaviors', () => { api.endpoints.mutationWithMeta.initiate({}) ) - expect(result.data).toEqual({ + expect('data' in result && result.data).toEqual({ banana: 'bread', meta: { request: { diff --git a/src/query/tests/optimisticUpdates.test.tsx b/src/query/tests/optimisticUpdates.test.tsx index ed0c72e57b..f92e578b9d 100644 --- a/src/query/tests/optimisticUpdates.test.tsx +++ b/src/query/tests/optimisticUpdates.test.tsx @@ -180,7 +180,7 @@ describe('updateQueryResult', () => { }) act(() => { - returnValue = storeRef.store.dispatch( + storeRef.store.dispatch( api.util.patchQueryResult('post', '3', returnValue.inversePatches) ) }) diff --git a/src/query/tests/refetchingBehaviors.test.tsx b/src/query/tests/refetchingBehaviors.test.tsx index 50b908a452..b8ea128c70 100644 --- a/src/query/tests/refetchingBehaviors.test.tsx +++ b/src/query/tests/refetchingBehaviors.test.tsx @@ -411,7 +411,7 @@ describe('customListenersHandler', () => { expect(dispatchSpy).toHaveBeenCalled() expect( defaultApi.internalActions.onOnline.match( - dispatchSpy.mock.calls[1][0] as AnyAction + dispatchSpy.mock.calls[1][0] as any ) ).toBe(true) From d9d7bd4d5453283e8142d96c5228ccc13584806e Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Thu, 6 May 2021 23:24:07 +0200 Subject: [PATCH 09/18] deprecation of old lifecycle with "best effort" fallback --- .../core/buildMiddleware/cacheLifecycle.ts | 12 +- .../core/buildMiddleware/queryLifecycle.ts | 3 + src/query/core/buildThunks.ts | 26 +-- src/query/createApi.ts | 40 ++++ src/query/endpointDefinitions.ts | 176 +++++++----------- src/query/tests/createApi.test.ts | 2 + src/query/tests/optimisticUpdates.test.tsx | 10 +- 7 files changed, 125 insertions(+), 144 deletions(-) diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index 3f03b25167..fa8b6a2685 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -31,7 +31,8 @@ export const build: SubMiddlewareBuilder = ({ action.meta.arg.endpointName, action.meta.arg.originalArgs, cacheKey, - mwApi + mwApi, + action.meta.requestId ) } } else if (mutationThunk.pending.match(action)) { @@ -41,7 +42,8 @@ export const build: SubMiddlewareBuilder = ({ action.meta.arg.endpointName, action.meta.arg.originalArgs, cacheKey, - mwApi + mwApi, + action.meta.requestId ) } } else if (isFullfilledThunk(action)) { @@ -81,7 +83,8 @@ export const build: SubMiddlewareBuilder = ({ endpointName: string, originalArgs: any, queryCacheKey: string, - mwApi: SubMiddlewareApi + mwApi: SubMiddlewareApi, + requestId: string ) { const onCacheEntryAdded = context.endpointDefinitions[endpointName]?.onCacheEntryAdded @@ -107,11 +110,14 @@ export const build: SubMiddlewareBuilder = ({ ) lifecycleMap[queryCacheKey] = lifecycle const selector = (api.endpoints[endpointName] as any).select(originalArgs) + const extra = mwApi.dispatch((_, __, extra) => extra) const runningHandler = onCacheEntryAdded( originalArgs, { ...mwApi, getCacheEntry: () => selector(mwApi.getState()), + requestId, + extra, }, { firstValueResolved, diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index 7a97548bb5..0b21e087b2 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -40,11 +40,14 @@ export const build: SubMiddlewareBuilder = ({ originalArgs ) + const extra = mwApi.dispatch((_, __, extra) => extra) onQuery( originalArgs, { ...mwApi, getCacheEntry: () => selector(mwApi.getState()), + requestId, + extra, }, { resultPromise } ) diff --git a/src/query/core/buildThunks.ts b/src/query/core/buildThunks.ts index c477cb1771..e09346ad5e 100644 --- a/src/query/core/buildThunks.ts +++ b/src/query/core/buildThunks.ts @@ -240,17 +240,6 @@ export function buildThunks< > = async (arg, { signal, rejectWithValue, ...api }) => { const endpointDefinition = endpointDefinitions[arg.endpointName] - const context: Record = {} - const queryApi: - | QueryApi - | MutationApi = { - ...api, - context, - } - - if (endpointDefinition.onStart) - endpointDefinition.onStart(arg.originalArgs, queryApi) - try { let transformResponse: ( baseQueryReturnValue: any, @@ -282,25 +271,12 @@ export function buildThunks< ) } if (result.error) throw new HandledError(result.error, result.meta) - if (endpointDefinition.onSuccess) - endpointDefinition.onSuccess( - arg.originalArgs, - queryApi, - result.data, - result.meta - ) + return { fulfilledTimeStamp: Date.now(), result: await transformResponse(result.data, result.meta), } } catch (error) { - if (endpointDefinition.onError) - endpointDefinition.onError( - arg.originalArgs, - queryApi, - error instanceof HandledError ? error.value : error, - error instanceof HandledError ? error.meta : undefined - ) if (error instanceof HandledError) { return rejectWithValue(error.value) } diff --git a/src/query/createApi.ts b/src/query/createApi.ts index ce1bd575ff..e415025dad 100644 --- a/src/query/createApi.ts +++ b/src/query/createApi.ts @@ -277,6 +277,26 @@ export function buildCreateApi, ...Module[]]>( } x.providesTags ??= x.provides } + if (x.onStart || x.onSuccess || x.onError) { + if ( + typeof process !== 'undefined' && + process.env.NODE_ENV === 'development' + ) { + console.warn( + '`onStart`, `onSuccess` and `onError` have been replaced by `onQuery`, please change your code accordingly' + ) + } + x.onQuery ??= async (arg, api, { resultPromise }) => { + const queryApi = { ...api, context: {} } + x.onStart?.(arg, queryApi) + try { + const result = await resultPromise + x.onSuccess?.(arg, queryApi, result, undefined) + } catch (error) { + x.onError?.(arg, queryApi, error, undefined) + } + } + } return { ...x, type: DefinitionType.query } as any }, mutation: (x) => { @@ -292,6 +312,26 @@ export function buildCreateApi, ...Module[]]>( } x.invalidatesTags ??= x.invalidates } + if (x.onStart || x.onSuccess || x.onError) { + if ( + typeof process !== 'undefined' && + process.env.NODE_ENV === 'development' + ) { + console.warn( + '`onStart`, `onSuccess` and `onError` have been replaced by `onQuery`, please change your code accordingly' + ) + } + x.onQuery ??= async (arg, api, { resultPromise }) => { + const queryApi = { ...api, context: {} } + x.onStart?.(arg, queryApi) + try { + const result = await resultPromise + x.onSuccess?.(arg, queryApi, result, undefined) + } catch (error) { + x.onError?.(arg, queryApi, error, undefined) + } + } + } return { ...x, type: DefinitionType.mutation } as any }, }) diff --git a/src/query/endpointDefinitions.ts b/src/query/endpointDefinitions.ts index 22eeb8293f..2e4c3a0741 100644 --- a/src/query/endpointDefinitions.ts +++ b/src/query/endpointDefinitions.ts @@ -60,9 +60,29 @@ interface EndpointDefinitionWithQueryFn< transformResponse?: never } -export type LifecycleApi = { - dispatch: ThunkDispatch - getState: () => unknown +export type LifecycleApi< + ReducerPath extends string = string, + CacheEntry = unknown +> = { + /** + * The dispatch method for the store + */ + dispatch: ThunkDispatch + /** + * A method to get the current state + */ + getState(): RootState + /** + * `extra` as provided as `thunk.extraArgument` to the `configureStore` `getDefaultMiddleware` option. + */ + extra: unknown + /** + * A unique ID generated for the mutation + */ + requestId: string + /** + * Gets the current value of this cache entry. + */ getCacheEntry: () => CacheEntry } @@ -148,26 +168,17 @@ export type ResultDescription< | ReadonlyArray> | GetResultDescriptionFn +/** @deprecated please use `onQuery` instead */ export interface QueryApi { - /** - * The dispatch method for the store - */ + /** @deprecated please use `onQuery` instead */ dispatch: ThunkDispatch - /** - * A method to get the current state - */ + /** @deprecated please use `onQuery` instead */ getState(): RootState - /** - * `extra` as provided as `thunk.extraArgument` to the `configureStore` `getDefaultMiddleware` option. - */ + /** @deprecated please use `onQuery` instead */ extra: unknown - /** - * A unique ID generated for the mutation - */ + /** @deprecated please use `onQuery` instead */ requestId: string - /** - * A variable shared between `onStart`, `onError` and `onSuccess` of one request to pass data forward between them - */ + /** @deprecated please use `onQuery` instead */ context: Context } @@ -176,8 +187,7 @@ interface QueryExtraOptions< ResultType, QueryArg, BaseQuery extends BaseQueryFn, - ReducerPath extends string = string, - Context = Record + ReducerPath extends string = string > { type: DefinitionType.query /** @@ -206,41 +216,26 @@ interface QueryExtraOptions< invalidatesTags?: never /** @deprecated */ invalidates?: never - /** - * Called when the query is triggered. - * @param arg - The argument supplied to the query - * @param queryApi - An object containing `dispatch`, `getState()`, `extra`, `request`Id`, `context` - */ - onStart?(arg: QueryArg, queryApi: QueryApi): void - /** - * Called when an error response is returned by the query. - * @param arg - The argument supplied to the query - * @param queryApi - A query API containing `dispatch`, `getState()`, `extra`, `request`Id`, `context` - * @param error - The error returned by the query - * @param meta - Meta item from the base query - */ + /** @deprecated please use `onQuery` instead */ + onStart?(arg: QueryArg, queryApi: QueryApi): void + /** @deprecated please use `onQuery` instead */ onError?( arg: QueryArg, - queryApi: QueryApi, + queryApi: QueryApi, error: unknown, - meta: BaseQueryMeta + meta: undefined ): void - /** - * Called when a successful response is returned by the query. - * @param arg - The argument supplied to the query - * @param queryApi - A query API containing `dispatch`, `getState()`, `extra`, `request`Id`, `context` - * @param result - The response returned by the query - * @param meta - Meta item from the base query - */ + /** @deprecated please use `onQuery` instead */ onSuccess?( arg: QueryArg, - queryApi: QueryApi, + queryApi: QueryApi, result: ResultType, - meta: BaseQueryMeta | undefined + meta: undefined ): void onCacheEntryAdded?( arg: QueryArg, api: LifecycleApi< + ReducerPath, QueryResultSelectorResult< { type: DefinitionType.query } & BaseEndpointDefinition< QueryArg, @@ -254,6 +249,7 @@ interface QueryExtraOptions< onQuery?( arg: QueryArg, api: LifecycleApi< + ReducerPath, QueryResultSelectorResult< { type: DefinitionType.query } & BaseEndpointDefinition< QueryArg, @@ -271,18 +267,11 @@ export type QueryDefinition< BaseQuery extends BaseQueryFn, TagTypes extends string, ResultType, - ReducerPath extends string = string, - Context = Record + ReducerPath extends string = string > = BaseEndpointDefinition & - QueryExtraOptions< - TagTypes, - ResultType, - QueryArg, - BaseQuery, - ReducerPath, - Context - > + QueryExtraOptions +/** @deprecated please use `onQuery` instead */ export interface MutationApi { /** * The dispatch method for the store @@ -311,8 +300,7 @@ interface MutationExtraOptions< ResultType, QueryArg, BaseQuery extends BaseQueryFn, - ReducerPath extends string = string, - Context = Record + ReducerPath extends string = string > { type: DefinitionType.mutation /** @@ -338,41 +326,26 @@ interface MutationExtraOptions< providesTags?: never /** @deprecated */ provides?: never - /** - * Called when the mutation is triggered. - * @param arg - The argument supplied to the query - * @param mutationApi - An object containing `dispatch`, `getState()`, `extra`, `request`Id`, `context` - */ - onStart?(arg: QueryArg, mutationApi: MutationApi): void - /** - * Called when an error response is returned by the mutation. - * @param arg - The argument supplied to the query - * @param mutationApi - A mutation API containing `dispatch`, `getState()`, `extra`, `request`Id`, `context` - * @param error - The error returned by the mutation - * @param meta - Meta item from the base query - */ + /** @deprecated please use `onQuery` instead */ + onStart?(arg: QueryArg, mutationApi: MutationApi): void + /** @deprecated please use `onQuery` instead */ onError?( arg: QueryArg, - mutationApi: MutationApi, + mutationApi: MutationApi, error: unknown, - meta: BaseQueryMeta + meta: undefined ): void - /** - * Called when a successful response is returned by the mutation. - * @param arg - The argument supplied to the query - * @param mutationApi - A mutation API containing `dispatch`, `getState()`, `extra`, `request`Id`, `context` - * @param result - The response returned by the mutation - * @param meta - Meta item from the base query - */ + /** @deprecated please use `onQuery` instead */ onSuccess?( arg: QueryArg, - mutationApi: MutationApi, + mutationApi: MutationApi, result: ResultType, - meta: BaseQueryMeta | undefined + meta: undefined ): void onCacheEntryAdded?( arg: QueryArg, api: LifecycleApi< + ReducerPath, MutationResultSelectorResult< { type: DefinitionType.mutation } & BaseEndpointDefinition< QueryArg, @@ -386,6 +359,7 @@ interface MutationExtraOptions< onQuery?( arg: QueryArg, api: LifecycleApi< + ReducerPath, MutationResultSelectorResult< { type: DefinitionType.mutation } & BaseEndpointDefinition< QueryArg, @@ -403,17 +377,9 @@ export type MutationDefinition< BaseQuery extends BaseQueryFn, TagTypes extends string, ResultType, - ReducerPath extends string = string, - Context = Record + ReducerPath extends string = string > = BaseEndpointDefinition & - MutationExtraOptions< - TagTypes, - ResultType, - QueryArg, - BaseQuery, - ReducerPath, - Context - > + MutationExtraOptions export type EndpointDefinition< QueryArg, @@ -460,8 +426,10 @@ export type EndpointBuilder< * query: (id) => ({ url: `post/${id}` }), * // Pick out data and prevent nested properties in a hook or selector * transformResponse: (response) => response.data, - * // The 2nd parameter is the destructured `queryApi` - * onStart(id, { dispatch, getState, extra, requestId, context }) {}, + * // trigger side effects or optimistic updates + * onQuery(id, { dispatch, getState, getCacheEntry }, { resultPromise }) {}, + * // handle subscriptions etc + * onCacheEntryAdded(id, { dispatch, getState, getCacheEntry }, { firstValueResolved, cleanup }) {}, * // `result` is the server response * onSuccess(id, queryApi, result) {}, * onError(id, queryApi) {}, @@ -490,38 +458,28 @@ export type EndpointBuilder< * query: ({ id, ...patch }) => ({ url: `post/${id}`, method: 'PATCH', body: patch }), * // Pick out data and prevent nested properties in a hook or selector * transformResponse: (response) => response.data, - * // onStart, onSuccess, onError are useful for optimistic updates - * // The 2nd parameter is the destructured `mutationApi` - * onStart({ id, ...patch }, { dispatch, getState, extra, requestId, context }) {}, - * // `result` is the server response - * onSuccess({ id }, mutationApi, result) {}, - * onError({ id }, { dispatch, getState, extra, requestId, context }) {}, + * // trigger side effects or optimistic updates + * onQuery(id, { dispatch, getState, getCacheEntry }, { resultPromise }) {}, + * // handle subscriptions etc + * onCacheEntryAdded(id, { dispatch, getState, getCacheEntry }, { firstValueResolved, cleanup }) {}, * invalidatesTags: (result, error, id) => [{ type: 'Post', id }], * }), * }), * }); * ``` */ - mutation>( + mutation( definition: OmitFromUnion< MutationDefinition< QueryArg, BaseQuery, TagTypes, ResultType, - ReducerPath, - Context + ReducerPath >, 'type' > - ): MutationDefinition< - QueryArg, - BaseQuery, - TagTypes, - ResultType, - ReducerPath, - Context - > + ): MutationDefinition } export type AssertTagTypes = >(t: T) => T diff --git a/src/query/tests/createApi.test.ts b/src/query/tests/createApi.test.ts index 8ab76b7416..65c2b59d7c 100644 --- a/src/query/tests/createApi.test.ts +++ b/src/query/tests/createApi.test.ts @@ -637,6 +637,7 @@ describe('query endpoint lifecycles - onStart, onSuccess, onError', () => { const failAttempt = storeRef.store.dispatch(api.endpoints.query.initiate()) expect(storeRef.store.getState().testReducer.count).toBe(0) await failAttempt + await waitMs(10) expect(storeRef.store.getState().testReducer.count).toBe(-1) const successAttempt = storeRef.store.dispatch( @@ -644,6 +645,7 @@ describe('query endpoint lifecycles - onStart, onSuccess, onError', () => { ) expect(storeRef.store.getState().testReducer.count).toBe(0) await successAttempt + await waitMs(10) expect(storeRef.store.getState().testReducer.count).toBe(1) }) diff --git a/src/query/tests/optimisticUpdates.test.tsx b/src/query/tests/optimisticUpdates.test.tsx index f92e578b9d..9a42dce8da 100644 --- a/src/query/tests/optimisticUpdates.test.tsx +++ b/src/query/tests/optimisticUpdates.test.tsx @@ -30,11 +30,7 @@ const api = createApi({ query: (id) => `post/${id}`, providesTags: ['Post'], }), - updatePost: build.mutation< - void, - Pick & Partial, - { undoPost: Patch[] } - >({ + updatePost: build.mutation & Partial>({ query: ({ id, ...patch }) => ({ url: `post/${id}`, method: 'PATCH', @@ -106,7 +102,7 @@ describe('basic lifecycle', () => { 'arg', expect.any(Object), 'success', - 'meta' + undefined ) }) @@ -132,7 +128,7 @@ describe('basic lifecycle', () => { expect(onError).toHaveBeenCalledWith( 'arg', expect.any(Object), - 'error', + { message: 'error' }, undefined ) expect(onSuccess).not.toHaveBeenCalled() From e2e1fbc0cdc461da92d4a4f936e3b04c393c0daf Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Thu, 6 May 2021 23:46:24 +0200 Subject: [PATCH 10/18] move lifecycle callbacks type definitions into their respective middleware --- .../core/buildMiddleware/cacheLifecycle.ts | 108 +++++++++++++++- .../core/buildMiddleware/queryLifecycle.ts | 69 +++++++++- src/query/core/buildSelectors.ts | 1 - src/query/endpointDefinitions.ts | 122 +----------------- 4 files changed, 177 insertions(+), 123 deletions(-) diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index fa8b6a2685..356d078b06 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -1,7 +1,113 @@ import { isAsyncThunkAction, isFulfilled } from '@reduxjs/toolkit' -import { toOptionalPromise } from '../../utils/toOptionalPromise' +import { AnyAction } from 'redux' +import { ThunkDispatch } from 'redux-thunk' +import { BaseQueryFn } from '../../baseQueryTypes' +import { + OptionalPromise, + toOptionalPromise, +} from '../../utils/toOptionalPromise' +import { RootState } from '../apiState' +import { + MutationResultSelectorResult, + QueryResultSelectorResult, +} from '../buildSelectors' import { SubMiddlewareApi, SubMiddlewareBuilder } from './types' +declare module '../../endpointDefinitions' { + export type LifecycleApi< + ReducerPath extends string = string, + CacheEntry = unknown + > = { + /** + * The dispatch method for the store + */ + dispatch: ThunkDispatch + /** + * A method to get the current state + */ + getState(): RootState + /** + * `extra` as provided as `thunk.extraArgument` to the `configureStore` `getDefaultMiddleware` option. + */ + extra: unknown + /** + * A unique ID generated for the mutation + */ + requestId: string + /** + * Gets the current value of this cache entry. + */ + getCacheEntry: () => CacheEntry + } + + export interface CacheLifecyclePromises { + /** + * Promise that will resolve with the first value for this cache key. + * This allows you to `await` until an actual value is in cache. + * + * If the cache entry is removed from the cache before any value has ever + * been resolved, this Promise will reject with + * `new Error('Promise never resolved before cleanup.')` + * to prevent memory leaks. + * You can just re-throw that error (or not handle it at all) - + * it will be caught outside of `cacheEntryAdded`. + */ + firstValueResolved: OptionalPromise + /** + * Promise that allows you to wait for the point in time when the cache entry + * has been removed from the cache, by not being used/subscribed to any more + * in the application for too long or by dispatching `api.util.resetApiState`. + */ + cleanup: Promise + } + + interface QueryExtraOptions< + TagTypes extends string, + ResultType, + QueryArg, + BaseQuery extends BaseQueryFn, + ReducerPath extends string = string + > { + onCacheEntryAdded?( + arg: QueryArg, + api: LifecycleApi< + ReducerPath, + QueryResultSelectorResult< + { type: DefinitionType.query } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + >, + promises: CacheLifecyclePromises + ): Promise | void + } + + interface MutationExtraOptions< + TagTypes extends string, + ResultType, + QueryArg, + BaseQuery extends BaseQueryFn, + ReducerPath extends string = string + > { + onCacheEntryAdded?( + arg: QueryArg, + api: LifecycleApi< + ReducerPath, + MutationResultSelectorResult< + { type: DefinitionType.mutation } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + >, + promises: CacheLifecyclePromises + ): Promise | void + } +} + export const build: SubMiddlewareBuilder = ({ api, reducerPath, diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index 0b21e087b2..a549a9e534 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -1,7 +1,74 @@ import { isPending, isRejected, isFulfilled } from '@reduxjs/toolkit' -import { toOptionalPromise } from '../../utils/toOptionalPromise' +import { BaseQueryFn } from '../../baseQueryTypes' +import { + OptionalPromise, + toOptionalPromise, +} from '../../utils/toOptionalPromise' +import { + MutationResultSelectorResult, + QueryResultSelectorResult, +} from '../buildSelectors' import { SubMiddlewareBuilder } from './types' +declare module '../../endpointDefinitions' { + export interface QueryLifecyclePromises { + /** + * Promise that will resolve with the (transformed) query result. + + * If the query fails, this promise will reject with the error. + * + * This allows you to `await` for the query to finish. + */ + resultPromise: OptionalPromise + } + + interface QueryExtraOptions< + TagTypes extends string, + ResultType, + QueryArg, + BaseQuery extends BaseQueryFn, + ReducerPath extends string = string + > { + onQuery?( + arg: QueryArg, + api: LifecycleApi< + ReducerPath, + QueryResultSelectorResult< + { type: DefinitionType.query } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + >, + promises: QueryLifecyclePromises + ): Promise | void + } + + interface MutationExtraOptions< + TagTypes extends string, + ResultType, + QueryArg, + BaseQuery extends BaseQueryFn, + ReducerPath extends string = string + > { + onQuery?( + arg: QueryArg, + api: LifecycleApi< + ReducerPath, + MutationResultSelectorResult< + { type: DefinitionType.mutation } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + >, + promises: QueryLifecyclePromises + ): Promise | void + } +} + export const build: SubMiddlewareBuilder = ({ api, context, diff --git a/src/query/core/buildSelectors.ts b/src/query/core/buildSelectors.ts index 6feb962285..3ef2f425ad 100644 --- a/src/query/core/buildSelectors.ts +++ b/src/query/core/buildSelectors.ts @@ -6,7 +6,6 @@ import { RootState as _RootState, getRequestStatusFlags, RequestStatusFlags, - CombinedState, } from './apiState' import { EndpointDefinitions, diff --git a/src/query/endpointDefinitions.ts b/src/query/endpointDefinitions.ts index 2e4c3a0741..80c0925312 100644 --- a/src/query/endpointDefinitions.ts +++ b/src/query/endpointDefinitions.ts @@ -1,9 +1,5 @@ import { AnyAction, ThunkDispatch } from '@reduxjs/toolkit' import { RootState } from './core/apiState' -import { - QueryResultSelectorResult, - MutationResultSelectorResult, -} from './core/buildSelectors' import { BaseQueryExtraOptions, BaseQueryFn, @@ -60,64 +56,6 @@ interface EndpointDefinitionWithQueryFn< transformResponse?: never } -export type LifecycleApi< - ReducerPath extends string = string, - CacheEntry = unknown -> = { - /** - * The dispatch method for the store - */ - dispatch: ThunkDispatch - /** - * A method to get the current state - */ - getState(): RootState - /** - * `extra` as provided as `thunk.extraArgument` to the `configureStore` `getDefaultMiddleware` option. - */ - extra: unknown - /** - * A unique ID generated for the mutation - */ - requestId: string - /** - * Gets the current value of this cache entry. - */ - getCacheEntry: () => CacheEntry -} - -export interface CacheLifecyclePromises { - /** - * Promise that will resolve with the first value for this cache key. - * This allows you to `await` until an actual value is in cache. - * - * If the cache entry is removed from the cache before any value has ever - * been resolved, this Promise will reject with - * `new Error('Promise never resolved before cleanup.')` - * to prevent memory leaks. - * You can just re-throw that error (or not handle it at all) - - * it will be caught outside of `cacheEntryAdded`. - */ - firstValueResolved: OptionalPromise - /** - * Promise that allows you to wait for the point in time when the cache entry - * has been removed from the cache, by not being used/subscribed to any more - * in the application for too long or by dispatching `api.util.resetApiState`. - */ - cleanup: Promise -} - -interface QueryLifecyclePromises { - /** - * Promise that will resolve with the (transformed) query result. - - * If the query fails, this promise will reject with the error. - * - * This allows you to `await` for the query to finish. - */ - resultPromise: OptionalPromise -} - export type BaseEndpointDefinition< QueryArg, BaseQuery extends BaseQueryFn, @@ -182,7 +120,7 @@ export interface QueryApi { context: Context } -interface QueryExtraOptions< +export interface QueryExtraOptions< TagTypes extends string, ResultType, QueryArg, @@ -232,34 +170,6 @@ interface QueryExtraOptions< result: ResultType, meta: undefined ): void - onCacheEntryAdded?( - arg: QueryArg, - api: LifecycleApi< - ReducerPath, - QueryResultSelectorResult< - { type: DefinitionType.query } & BaseEndpointDefinition< - QueryArg, - BaseQuery, - ResultType - > - > - >, - promises: CacheLifecyclePromises - ): Promise | void - onQuery?( - arg: QueryArg, - api: LifecycleApi< - ReducerPath, - QueryResultSelectorResult< - { type: DefinitionType.query } & BaseEndpointDefinition< - QueryArg, - BaseQuery, - ResultType - > - > - >, - promises: QueryLifecyclePromises - ): Promise | void } export type QueryDefinition< @@ -295,7 +205,7 @@ export interface MutationApi { context: Context } -interface MutationExtraOptions< +export interface MutationExtraOptions< TagTypes extends string, ResultType, QueryArg, @@ -342,34 +252,6 @@ interface MutationExtraOptions< result: ResultType, meta: undefined ): void - onCacheEntryAdded?( - arg: QueryArg, - api: LifecycleApi< - ReducerPath, - MutationResultSelectorResult< - { type: DefinitionType.mutation } & BaseEndpointDefinition< - QueryArg, - BaseQuery, - ResultType - > - > - >, - promises: CacheLifecyclePromises - ): Promise | void - onQuery?( - arg: QueryArg, - api: LifecycleApi< - ReducerPath, - MutationResultSelectorResult< - { type: DefinitionType.mutation } & BaseEndpointDefinition< - QueryArg, - BaseQuery, - ResultType - > - > - >, - promises: QueryLifecyclePromises - ): Promise | void } export type MutationDefinition< From 89b58134eb36f0a0d9f44d63cf2a837e516bb7a3 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Fri, 7 May 2021 00:10:57 +0200 Subject: [PATCH 11/18] force loading of references? --- src/query/core/buildMiddleware/cacheLifecycle.ts | 2 ++ src/query/core/buildMiddleware/queryLifecycle.ts | 2 ++ src/query/core/module.ts | 8 +++++++- src/query/endpointDefinitions.ts | 1 - 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index 356d078b06..190ad3ac13 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -13,6 +13,8 @@ import { } from '../buildSelectors' import { SubMiddlewareApi, SubMiddlewareBuilder } from './types' +export type ReferenceCacheLifecycle = never + declare module '../../endpointDefinitions' { export type LifecycleApi< ReducerPath extends string = string, diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index a549a9e534..624aa8fb04 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -10,6 +10,8 @@ import { } from '../buildSelectors' import { SubMiddlewareBuilder } from './types' +export type ReferenceQueryLifecycle = never + declare module '../../endpointDefinitions' { export interface QueryLifecyclePromises { /** diff --git a/src/query/core/module.ts b/src/query/core/module.ts index a2f40cf794..ca00b87de1 100644 --- a/src/query/core/module.ts +++ b/src/query/core/module.ts @@ -36,6 +36,9 @@ import { InternalSerializeQueryArgs } from '../defaultSerializeQueryArgs' import { SliceActions } from './buildSlice' import { BaseQueryFn } from '../baseQueryTypes' +import type { ReferenceCacheLifecycle } from './buildMiddleware/cacheLifecycle' +import type { ReferenceQueryLifecycle } from './buildMiddleware/queryLifecycle' + /** * `ifOlderThan` - (default: `false` | `number`) - _number is value in seconds_ * - If specified, it will only run the query if the difference between `new Date()` and the last `fulfilledTimeStamp` is greater than the given value @@ -51,7 +54,10 @@ export type PrefetchOptions = | { force?: boolean } export const coreModuleName = Symbol() -export type CoreModule = typeof coreModuleName +export type CoreModule = + | typeof coreModuleName + | ReferenceCacheLifecycle + | ReferenceQueryLifecycle declare module '../apiTypes' { export interface ApiModules< diff --git a/src/query/endpointDefinitions.ts b/src/query/endpointDefinitions.ts index 80c0925312..dacb87e1be 100644 --- a/src/query/endpointDefinitions.ts +++ b/src/query/endpointDefinitions.ts @@ -17,7 +17,6 @@ import { CastAny, } from './tsHelpers' import { NEVER } from './fakeBaseQuery' -import { OptionalPromise } from './utils/toOptionalPromise' const resultType = Symbol() const baseQuery = Symbol() From 40b47a8e6568bff093cf1b13d20564da9b0ef1c6 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sat, 8 May 2021 01:35:01 +0200 Subject: [PATCH 12/18] `updateCacheEntry`, `.undo` add `updateCacheEntry` to `QueryLifecycleApi`, add `.undo` to `updateCacheValue` return type re-scope middleware variables tests --- .../core/buildMiddleware/cacheCollection.ts | 64 ++-- .../core/buildMiddleware/cacheLifecycle.ts | 306 ++++++++++-------- src/query/core/buildMiddleware/polling.ts | 188 +++++------ .../core/buildMiddleware/queryLifecycle.ts | 124 ++++--- src/query/core/buildMiddleware/types.ts | 2 +- src/query/core/buildThunks.ts | 37 ++- src/query/core/index.ts | 3 + src/query/tests/cacheLifecycle.test.ts | 71 +++- src/query/tests/helpers.tsx | 1 + src/query/tests/optimisticUpdates.test.tsx | 12 +- src/query/tests/queryLifecycle.test.tsx | 75 +++++ 11 files changed, 535 insertions(+), 348 deletions(-) diff --git a/src/query/core/buildMiddleware/cacheCollection.ts b/src/query/core/buildMiddleware/cacheCollection.ts index 1224c43334..59a3b970d0 100644 --- a/src/query/core/buildMiddleware/cacheCollection.ts +++ b/src/query/core/buildMiddleware/cacheCollection.ts @@ -7,45 +7,47 @@ import { } from './types' export const build: SubMiddlewareBuilder = ({ reducerPath, api }) => { - const currentRemovalTimeouts: QueryStateMeta = {} - const { removeQueryResult, unsubscribeQueryResult } = api.internalActions - return (mwApi) => (next) => (action): any => { - const result = next(action) + return (mwApi) => { + const currentRemovalTimeouts: QueryStateMeta = {} - if (unsubscribeQueryResult.match(action)) { - handleUnsubscribe(action.payload, mwApi) - } + return (next) => (action): any => { + const result = next(action) - if (api.util.resetApiState.match(action)) { - for (const [key, timeout] of Object.entries(currentRemovalTimeouts)) { - if (timeout) clearTimeout(timeout) - delete currentRemovalTimeouts[key] + if (unsubscribeQueryResult.match(action)) { + handleUnsubscribe(action.payload, mwApi) } - } - return result - } + if (api.util.resetApiState.match(action)) { + for (const [key, timeout] of Object.entries(currentRemovalTimeouts)) { + if (timeout) clearTimeout(timeout) + delete currentRemovalTimeouts[key] + } + } - function handleUnsubscribe( - { queryCacheKey }: QuerySubstateIdentifier, - api: SubMiddlewareApi - ) { - const keepUnusedDataFor = api.getState()[reducerPath].config - .keepUnusedDataFor - const currentTimeout = currentRemovalTimeouts[queryCacheKey] - if (currentTimeout) { - clearTimeout(currentTimeout) + return result } - currentRemovalTimeouts[queryCacheKey] = setTimeout(() => { - const subscriptions = api.getState()[reducerPath].subscriptions[ - queryCacheKey - ] - if (!subscriptions || Object.keys(subscriptions).length === 0) { - api.dispatch(removeQueryResult({ queryCacheKey })) + + function handleUnsubscribe( + { queryCacheKey }: QuerySubstateIdentifier, + api: SubMiddlewareApi + ) { + const keepUnusedDataFor = api.getState()[reducerPath].config + .keepUnusedDataFor + const currentTimeout = currentRemovalTimeouts[queryCacheKey] + if (currentTimeout) { + clearTimeout(currentTimeout) } - delete currentRemovalTimeouts![queryCacheKey] - }, keepUnusedDataFor * 1000) + currentRemovalTimeouts[queryCacheKey] = setTimeout(() => { + const subscriptions = api.getState()[reducerPath].subscriptions[ + queryCacheKey + ] + if (!subscriptions || Object.keys(subscriptions).length === 0) { + api.dispatch(removeQueryResult({ queryCacheKey })) + } + delete currentRemovalTimeouts![queryCacheKey] + }, keepUnusedDataFor * 1000) + } } } diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index 190ad3ac13..916725b285 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -1,25 +1,65 @@ import { isAsyncThunkAction, isFulfilled } from '@reduxjs/toolkit' -import { AnyAction } from 'redux' -import { ThunkDispatch } from 'redux-thunk' -import { BaseQueryFn } from '../../baseQueryTypes' +import type { AnyAction } from 'redux' +import type { ThunkDispatch } from 'redux-thunk' +import type { BaseQueryFn } from '../../baseQueryTypes' +import { DefinitionType } from '../../endpointDefinitions' import { OptionalPromise, toOptionalPromise, } from '../../utils/toOptionalPromise' -import { RootState } from '../apiState' -import { +import type { RootState } from '../apiState' +import type { MutationResultSelectorResult, QueryResultSelectorResult, } from '../buildSelectors' -import { SubMiddlewareApi, SubMiddlewareBuilder } from './types' +import type { PatchCollection, Recipe } from '../buildThunks' +import type { SubMiddlewareApi, SubMiddlewareBuilder } from './types' export type ReferenceCacheLifecycle = never declare module '../../endpointDefinitions' { - export type LifecycleApi< - ReducerPath extends string = string, - CacheEntry = unknown - > = { + export interface QueryLifecycleApi< + QueryArg, + BaseQuery extends BaseQueryFn, + ResultType, + ReducerPath extends string = string + > extends LifecycleApi { + /** + * Gets the current value of this cache entry. + */ + getCacheEntry(): QueryResultSelectorResult< + { type: DefinitionType.query } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + /** + * Updates the current cache entry value. + * For documentation see `api.util.updateQueryResult. + */ + updateCacheEntry(updateRecipe: Recipe): PatchCollection + } + + export interface MutationLifecycleApi< + QueryArg, + BaseQuery extends BaseQueryFn, + ResultType, + ReducerPath extends string = string + > extends LifecycleApi { + /** + * Gets the current value of this cache entry. + */ + getCacheEntry(): MutationResultSelectorResult< + { type: DefinitionType.mutation } & BaseEndpointDefinition< + QueryArg, + BaseQuery, + ResultType + > + > + } + + export interface LifecycleApi { /** * The dispatch method for the store */ @@ -36,10 +76,6 @@ declare module '../../endpointDefinitions' { * A unique ID generated for the mutation */ requestId: string - /** - * Gets the current value of this cache entry. - */ - getCacheEntry: () => CacheEntry } export interface CacheLifecyclePromises { @@ -72,16 +108,7 @@ declare module '../../endpointDefinitions' { > { onCacheEntryAdded?( arg: QueryArg, - api: LifecycleApi< - ReducerPath, - QueryResultSelectorResult< - { type: DefinitionType.query } & BaseEndpointDefinition< - QueryArg, - BaseQuery, - ResultType - > - > - >, + api: QueryLifecycleApi, promises: CacheLifecyclePromises ): Promise | void } @@ -95,16 +122,7 @@ declare module '../../endpointDefinitions' { > { onCacheEntryAdded?( arg: QueryArg, - api: LifecycleApi< - ReducerPath, - MutationResultSelectorResult< - { type: DefinitionType.mutation } & BaseEndpointDefinition< - QueryArg, - BaseQuery, - ResultType - > - > - >, + api: MutationLifecycleApi, promises: CacheLifecyclePromises ): Promise | void } @@ -117,125 +135,135 @@ export const build: SubMiddlewareBuilder = ({ queryThunk, mutationThunk, }) => { - type CacheLifecycle = { - valueResolved?(value: unknown): unknown - cleanup(): void - } - const lifecycleMap: Record = {} - const isQueryThunk = isAsyncThunkAction(queryThunk) const isMutationThunk = isAsyncThunkAction(mutationThunk) const isFullfilledThunk = isFulfilled(queryThunk, mutationThunk) - return (mwApi) => (next) => (action): any => { - const result = next(action) - - const cacheKey = getCacheKey(action) - - if (queryThunk.pending.match(action)) { - const state = mwApi.getState()[reducerPath].queries[cacheKey] - if (state?.requestId === action.meta.requestId) { - handleNewKey( - action.meta.arg.endpointName, - action.meta.arg.originalArgs, - cacheKey, - mwApi, - action.meta.requestId - ) - } - } else if (mutationThunk.pending.match(action)) { - const state = mwApi.getState()[reducerPath].mutations[cacheKey] - if (state) { - handleNewKey( - action.meta.arg.endpointName, - action.meta.arg.originalArgs, - cacheKey, - mwApi, - action.meta.requestId - ) - } - } else if (isFullfilledThunk(action)) { - const lifecycle = lifecycleMap[cacheKey] - if (lifecycle?.valueResolved) { - lifecycle.valueResolved(action.payload.result) - delete lifecycle.valueResolved - } - } else if ( - api.internalActions.removeQueryResult.match(action) || - api.internalActions.unsubscribeMutationResult.match(action) - ) { - const lifecycle = lifecycleMap[cacheKey] - if (lifecycle) { - delete lifecycleMap[cacheKey] - lifecycle.cleanup() - } - } else if (api.util.resetApiState.match(action)) { - for (const [cacheKey, lifecycle] of Object.entries(lifecycleMap)) { - delete lifecycleMap[cacheKey] - lifecycle.cleanup() + return (mwApi) => { + type CacheLifecycle = { + valueResolved?(value: unknown): unknown + cleanup(): void + } + const lifecycleMap: Record = {} + + return (next) => (action): any => { + const result = next(action) + + const cacheKey = getCacheKey(action) + + if (queryThunk.pending.match(action)) { + const state = mwApi.getState()[reducerPath].queries[cacheKey] + if (state?.requestId === action.meta.requestId) { + handleNewKey( + action.meta.arg.endpointName, + action.meta.arg.originalArgs, + cacheKey, + mwApi, + action.meta.requestId + ) + } + } else if (mutationThunk.pending.match(action)) { + const state = mwApi.getState()[reducerPath].mutations[cacheKey] + if (state) { + handleNewKey( + action.meta.arg.endpointName, + action.meta.arg.originalArgs, + cacheKey, + mwApi, + action.meta.requestId + ) + } + } else if (isFullfilledThunk(action)) { + const lifecycle = lifecycleMap[cacheKey] + if (lifecycle?.valueResolved) { + lifecycle.valueResolved(action.payload.result) + delete lifecycle.valueResolved + } + } else if ( + api.internalActions.removeQueryResult.match(action) || + api.internalActions.unsubscribeMutationResult.match(action) + ) { + const lifecycle = lifecycleMap[cacheKey] + if (lifecycle) { + delete lifecycleMap[cacheKey] + lifecycle.cleanup() + } + } else if (api.util.resetApiState.match(action)) { + for (const [cacheKey, lifecycle] of Object.entries(lifecycleMap)) { + delete lifecycleMap[cacheKey] + lifecycle.cleanup() + } } + + return result } - return result - } + function getCacheKey(action: any) { + if (isQueryThunk(action)) return action.meta.arg.queryCacheKey + if (isMutationThunk(action)) return action.meta.requestId + if (api.internalActions.removeQueryResult.match(action)) + return action.payload.queryCacheKey + return '' + } - function getCacheKey(action: any) { - if (isQueryThunk(action)) return action.meta.arg.queryCacheKey - if (isMutationThunk(action)) return action.meta.requestId - if (api.internalActions.removeQueryResult.match(action)) - return action.payload.queryCacheKey - return '' - } + function handleNewKey( + endpointName: string, + originalArgs: any, + queryCacheKey: string, + mwApi: SubMiddlewareApi, + requestId: string + ) { + const endpointDefinition = context.endpointDefinitions[endpointName] + const onCacheEntryAdded = endpointDefinition?.onCacheEntryAdded + if (!onCacheEntryAdded) return - function handleNewKey( - endpointName: string, - originalArgs: any, - queryCacheKey: string, - mwApi: SubMiddlewareApi, - requestId: string - ) { - const onCacheEntryAdded = - context.endpointDefinitions[endpointName]?.onCacheEntryAdded - if (!onCacheEntryAdded) return - - const neverResolvedError = new Error( - 'Promise never resolved before cleanup.' - ) - let lifecycle = {} as CacheLifecycle - - const cleanup = new Promise((resolve) => { - lifecycle.cleanup = resolve - }) - const firstValueResolved = toOptionalPromise( - Promise.race([ - new Promise((resolve) => { - lifecycle.valueResolved = resolve - }), - cleanup.then(() => { - throw neverResolvedError - }), - ]) - ) - lifecycleMap[queryCacheKey] = lifecycle - const selector = (api.endpoints[endpointName] as any).select(originalArgs) - const extra = mwApi.dispatch((_, __, extra) => extra) - const runningHandler = onCacheEntryAdded( - originalArgs, - { + const neverResolvedError = new Error( + 'Promise never resolved before cleanup.' + ) + let lifecycle = {} as CacheLifecycle + + const cleanup = new Promise((resolve) => { + lifecycle.cleanup = resolve + }) + const firstValueResolved = toOptionalPromise( + Promise.race([ + new Promise((resolve) => { + lifecycle.valueResolved = resolve + }), + cleanup.then(() => { + throw neverResolvedError + }), + ]) + ) + lifecycleMap[queryCacheKey] = lifecycle + const selector = (api.endpoints[endpointName] as any).select(originalArgs) + const extra = mwApi.dispatch((_, __, extra) => extra) + const lifecycleApi = { ...mwApi, getCacheEntry: () => selector(mwApi.getState()), requestId, extra, - }, - { + updateCacheEntry: (endpointDefinition.type === DefinitionType.query + ? (updateRecipe: Recipe) => + mwApi.dispatch( + api.util.updateQueryResult( + endpointName as never, + originalArgs, + updateRecipe + ) + ) + : undefined) as any, + } + + const runningHandler = onCacheEntryAdded(originalArgs, lifecycleApi, { firstValueResolved, cleanup, - } - ) - // if a `neverResolvedError` was thrown, but not handled in the running handler, do not let it leak out further - Promise.resolve(runningHandler).catch((e) => { - if (e === neverResolvedError) return - throw e - }) + }) + // if a `neverResolvedError` was thrown, but not handled in the running handler, do not let it leak out further + Promise.resolve(runningHandler).catch((e) => { + if (e === neverResolvedError) return + throw e + }) + } } } diff --git a/src/query/core/buildMiddleware/polling.ts b/src/query/core/buildMiddleware/polling.ts index a8878f19c8..c5ca0c40f7 100644 --- a/src/query/core/buildMiddleware/polling.ts +++ b/src/query/core/buildMiddleware/polling.ts @@ -12,121 +12,125 @@ export const build: SubMiddlewareBuilder = ({ api, refetchQuery, }) => { - const currentPolls: QueryStateMeta<{ - nextPollTimestamp: number - timeout?: TimeoutId - pollingInterval: number - }> = {} - - return (mwApi) => (next) => (action): any => { - const result = next(action) + return (mwApi) => { + const currentPolls: QueryStateMeta<{ + nextPollTimestamp: number + timeout?: TimeoutId + pollingInterval: number + }> = {} + return (next) => (action): any => { + const result = next(action) + + if (api.internalActions.updateSubscriptionOptions.match(action)) { + updatePollingInterval(action.payload, mwApi) + } - if (api.internalActions.updateSubscriptionOptions.match(action)) { - updatePollingInterval(action.payload, mwApi) - } + if ( + queryThunk.pending.match(action) || + (queryThunk.rejected.match(action) && action.meta.condition) + ) { + updatePollingInterval(action.meta.arg, mwApi) + } - if ( - queryThunk.pending.match(action) || - (queryThunk.rejected.match(action) && action.meta.condition) - ) { - updatePollingInterval(action.meta.arg, mwApi) - } + if ( + queryThunk.fulfilled.match(action) || + (queryThunk.rejected.match(action) && !action.meta.condition) + ) { + startNextPoll(action.meta.arg, mwApi) + } - if ( - queryThunk.fulfilled.match(action) || - (queryThunk.rejected.match(action) && !action.meta.condition) - ) { - startNextPoll(action.meta.arg, mwApi) - } + if (api.util.resetApiState.match(action)) { + clearPolls() + } - if (api.util.resetApiState.match(action)) { - clearPolls() + return result } - return result - } + function startNextPoll( + { queryCacheKey }: QuerySubstateIdentifier, + api: SubMiddlewareApi + ) { + const state = api.getState()[reducerPath] + const querySubState = state.queries[queryCacheKey] + const subscriptions = state.subscriptions[queryCacheKey] - function startNextPoll( - { queryCacheKey }: QuerySubstateIdentifier, - api: SubMiddlewareApi - ) { - const state = api.getState()[reducerPath] - const querySubState = state.queries[queryCacheKey] - const subscriptions = state.subscriptions[queryCacheKey] + if (!querySubState || querySubState.status === QueryStatus.uninitialized) + return - if (!querySubState || querySubState.status === QueryStatus.uninitialized) - return + const lowestPollingInterval = findLowestPollingInterval(subscriptions) + if (!Number.isFinite(lowestPollingInterval)) return - const lowestPollingInterval = findLowestPollingInterval(subscriptions) - if (!Number.isFinite(lowestPollingInterval)) return + const currentPoll = currentPolls[queryCacheKey] - const currentPoll = currentPolls[queryCacheKey] + if (currentPoll?.timeout) { + clearTimeout(currentPoll.timeout) + currentPoll.timeout = undefined + } - if (currentPoll?.timeout) { - clearTimeout(currentPoll.timeout) - currentPoll.timeout = undefined + const nextPollTimestamp = Date.now() + lowestPollingInterval + + const currentInterval: typeof currentPolls[number] = (currentPolls[ + queryCacheKey + ] = { + nextPollTimestamp, + pollingInterval: lowestPollingInterval, + timeout: setTimeout(() => { + currentInterval!.timeout = undefined + api.dispatch(refetchQuery(querySubState, queryCacheKey)) + }, lowestPollingInterval), + }) } - const nextPollTimestamp = Date.now() + lowestPollingInterval - - const currentInterval: typeof currentPolls[number] = (currentPolls[ - queryCacheKey - ] = { - nextPollTimestamp, - pollingInterval: lowestPollingInterval, - timeout: setTimeout(() => { - currentInterval!.timeout = undefined - api.dispatch(refetchQuery(querySubState, queryCacheKey)) - }, lowestPollingInterval), - }) - } + function updatePollingInterval( + { queryCacheKey }: QuerySubstateIdentifier, + api: SubMiddlewareApi + ) { + const state = api.getState()[reducerPath] + const querySubState = state.queries[queryCacheKey] + const subscriptions = state.subscriptions[queryCacheKey] + + if ( + !querySubState || + querySubState.status === QueryStatus.uninitialized + ) { + return + } - function updatePollingInterval( - { queryCacheKey }: QuerySubstateIdentifier, - api: SubMiddlewareApi - ) { - const state = api.getState()[reducerPath] - const querySubState = state.queries[queryCacheKey] - const subscriptions = state.subscriptions[queryCacheKey] + const lowestPollingInterval = findLowestPollingInterval(subscriptions) + const currentPoll = currentPolls[queryCacheKey] - if (!querySubState || querySubState.status === QueryStatus.uninitialized) { - return - } + if (!Number.isFinite(lowestPollingInterval)) { + if (currentPoll?.timeout) { + clearTimeout(currentPoll.timeout) + } + delete currentPolls[queryCacheKey] + return + } - const lowestPollingInterval = findLowestPollingInterval(subscriptions) - const currentPoll = currentPolls[queryCacheKey] + const nextPollTimestamp = Date.now() + lowestPollingInterval - if (!Number.isFinite(lowestPollingInterval)) { - if (currentPoll?.timeout) { - clearTimeout(currentPoll.timeout) + if (!currentPoll || nextPollTimestamp < currentPoll.nextPollTimestamp) { + startNextPoll({ queryCacheKey }, api) } - delete currentPolls[queryCacheKey] - return } - const nextPollTimestamp = Date.now() + lowestPollingInterval - - if (!currentPoll || nextPollTimestamp < currentPoll.nextPollTimestamp) { - startNextPoll({ queryCacheKey }, api) + function clearPolls() { + for (const [key, poll] of Object.entries(currentPolls)) { + if (poll?.timeout) clearTimeout(poll.timeout) + delete currentPolls[key] + } } } - function clearPolls() { - for (const [key, poll] of Object.entries(currentPolls)) { - if (poll?.timeout) clearTimeout(poll.timeout) - delete currentPolls[key] + function findLowestPollingInterval(subscribers: Subscribers = {}) { + let lowestPollingInterval = Number.POSITIVE_INFINITY + for (const subscription of Object.values(subscribers)) { + if (!!subscription.pollingInterval) + lowestPollingInterval = Math.min( + subscription.pollingInterval, + lowestPollingInterval + ) } + return lowestPollingInterval } } - -function findLowestPollingInterval(subscribers: Subscribers = {}) { - let lowestPollingInterval = Number.POSITIVE_INFINITY - for (const subscription of Object.values(subscribers)) { - if (!!subscription.pollingInterval) - lowestPollingInterval = Math.min( - subscription.pollingInterval, - lowestPollingInterval - ) - } - return lowestPollingInterval -} diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index 624aa8fb04..811959a66d 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -1,13 +1,11 @@ import { isPending, isRejected, isFulfilled } from '@reduxjs/toolkit' import { BaseQueryFn } from '../../baseQueryTypes' +import { DefinitionType } from '../../endpointDefinitions' import { OptionalPromise, toOptionalPromise, } from '../../utils/toOptionalPromise' -import { - MutationResultSelectorResult, - QueryResultSelectorResult, -} from '../buildSelectors' +import { Recipe } from '../buildThunks' import { SubMiddlewareBuilder } from './types' export type ReferenceQueryLifecycle = never @@ -33,16 +31,7 @@ declare module '../../endpointDefinitions' { > { onQuery?( arg: QueryArg, - api: LifecycleApi< - ReducerPath, - QueryResultSelectorResult< - { type: DefinitionType.query } & BaseEndpointDefinition< - QueryArg, - BaseQuery, - ResultType - > - > - >, + api: QueryLifecycleApi, promises: QueryLifecyclePromises ): Promise | void } @@ -56,16 +45,7 @@ declare module '../../endpointDefinitions' { > { onQuery?( arg: QueryArg, - api: LifecycleApi< - ReducerPath, - MutationResultSelectorResult< - { type: DefinitionType.mutation } & BaseEndpointDefinition< - QueryArg, - BaseQuery, - ResultType - > - > - >, + api: MutationLifecycleApi, promises: QueryLifecyclePromises ): Promise | void } @@ -77,60 +57,70 @@ export const build: SubMiddlewareBuilder = ({ queryThunk, mutationThunk, }) => { - type CacheLifecycle = { - resolve(value: unknown): unknown - reject(value: unknown): unknown - } - const lifecycleMap: Record = {} - const isPendingThunk = isPending(queryThunk, mutationThunk) const isRejectedThunk = isRejected(queryThunk, mutationThunk) const isFullfilledThunk = isFulfilled(queryThunk, mutationThunk) - return (mwApi) => (next) => (action): any => { - const result = next(action) + return (mwApi) => { + type CacheLifecycle = { + resolve(value: unknown): unknown + reject(value: unknown): unknown + } + const lifecycleMap: Record = {} - if (isPendingThunk(action)) { - const { - requestId, - arg: { endpointName, originalArgs }, - } = action.meta - const onQuery = context.endpointDefinitions[endpointName]?.onQuery - if (onQuery) { - const lifecycle = {} as CacheLifecycle - const resultPromise = toOptionalPromise( - new Promise((resolve, reject) => { - lifecycle.resolve = resolve - lifecycle.reject = reject - }) - ) - lifecycleMap[requestId] = lifecycle - const selector = (api.endpoints[endpointName] as any).select( - originalArgs - ) + return (next) => (action): any => { + const result = next(action) - const extra = mwApi.dispatch((_, __, extra) => extra) - onQuery( - originalArgs, - { + if (isPendingThunk(action)) { + const { + requestId, + arg: { endpointName, originalArgs }, + } = action.meta + const endpointDefinition = context.endpointDefinitions[endpointName] + const onQuery = endpointDefinition?.onQuery + if (onQuery) { + const lifecycle = {} as CacheLifecycle + const resultPromise = toOptionalPromise( + new Promise((resolve, reject) => { + lifecycle.resolve = resolve + lifecycle.reject = reject + }) + ) + lifecycleMap[requestId] = lifecycle + const selector = (api.endpoints[endpointName] as any).select( + originalArgs + ) + + const extra = mwApi.dispatch((_, __, extra) => extra) + const lifecycleApi = { ...mwApi, getCacheEntry: () => selector(mwApi.getState()), requestId, extra, - }, - { resultPromise } - ) + updateCacheEntry: (endpointDefinition.type === DefinitionType.query + ? (updateRecipe: Recipe) => + mwApi.dispatch( + api.util.updateQueryResult( + endpointName as never, + originalArgs, + updateRecipe + ) + ) + : undefined) as any, + } + onQuery(originalArgs, lifecycleApi, { resultPromise }) + } + } else if (isFullfilledThunk(action)) { + const { requestId } = action.meta + lifecycleMap[requestId]?.resolve(action.payload.result) + delete lifecycleMap[requestId] + } else if (isRejectedThunk(action)) { + const { requestId } = action.meta + lifecycleMap[requestId]?.reject(action.payload ?? action.error) + delete lifecycleMap[requestId] } - } else if (isFullfilledThunk(action)) { - const { requestId } = action.meta - lifecycleMap[requestId]?.resolve(action.payload.result) - delete lifecycleMap[requestId] - } else if (isRejectedThunk(action)) { - const { requestId } = action.meta - lifecycleMap[requestId]?.reject(action.payload ?? action.error) - delete lifecycleMap[requestId] - } - return result + return result + } } } diff --git a/src/query/core/buildMiddleware/types.ts b/src/query/core/buildMiddleware/types.ts index d7a6ac1ef7..8463b39b5f 100644 --- a/src/query/core/buildMiddleware/types.ts +++ b/src/query/core/buildMiddleware/types.ts @@ -24,7 +24,7 @@ export interface BuildMiddlewareInput< context: ApiContext queryThunk: AsyncThunk mutationThunk: AsyncThunk - api: Api + api: Api assertTagType: AssertTagTypes } diff --git a/src/query/core/buildThunks.ts b/src/query/core/buildThunks.ts index e09346ad5e..ebbcea5bb8 100644 --- a/src/query/core/buildThunks.ts +++ b/src/query/core/buildThunks.ts @@ -36,7 +36,7 @@ import { isRejected, isRejectedWithValue, } from '@reduxjs/toolkit' -import { Patch, isDraftable, produceWithPatches, enablePatches } from 'immer' +import { Patch, isDraftable, produceWithPatches } from 'immer' import { AnyAction, createAsyncThunk, @@ -135,8 +135,8 @@ function defaultTransformResponse(baseQueryReturnValue: unknown) { return baseQueryReturnValue } -type MaybeDrafted = T | Draft -type Recipe = (data: MaybeDrafted) => void | MaybeDrafted +export type MaybeDrafted = T | Draft +export type Recipe = (data: MaybeDrafted) => void | MaybeDrafted export type PatchQueryResultThunk< Definitions extends EndpointDefinitions, @@ -156,7 +156,23 @@ export type UpdateQueryResultThunk< updateRecipe: Recipe> ) => ThunkAction -type PatchCollection = { patches: Patch[]; inversePatches: Patch[] } +/** + * An object returned from dispatching a `api.util.updateQueryResult` call. + */ +export type PatchCollection = { + /** + * An `immer` Patch describing the cache update. + */ + patches: Patch[] + /** + * An `immer` Patch to revert the cache update. + */ + inversePatches: Patch[] + /** + * A function that will undo the cache update. + */ + undo: () => void +} export function buildThunks< BaseQuery extends BaseQueryFn, @@ -203,14 +219,19 @@ export function buildThunks< any, any >).select(args)(getState()) - let ret: PatchCollection = { patches: [], inversePatches: [] } + let ret: PatchCollection = { + patches: [], + inversePatches: [], + undo: () => + dispatch( + api.util.patchQueryResult(endpointName, args, ret.inversePatches) + ), + } if (currentState.status === QueryStatus.uninitialized) { return ret } if ('data' in currentState) { if (isDraftable(currentState.data)) { - // call "enablePatches" as late as possible - enablePatches() const [, patches, inversePatches] = produceWithPatches( currentState.data, updateRecipe @@ -228,7 +249,7 @@ export function buildThunks< } } - dispatch(patchQueryResult(endpointName, args, ret.patches)) + dispatch(api.util.patchQueryResult(endpointName, args, ret.patches)) return ret } diff --git a/src/query/core/index.ts b/src/query/core/index.ts index e3f2efbbba..ba4384d69b 100644 --- a/src/query/core/index.ts +++ b/src/query/core/index.ts @@ -1,6 +1,9 @@ +import { enablePatches } from 'immer' import { buildCreateApi, CreateApi } from '../createApi' import { coreModule, coreModuleName } from './module' +enablePatches() + const createApi = buildCreateApi(coreModule()) export { createApi, coreModule } diff --git a/src/query/tests/cacheLifecycle.test.ts b/src/query/tests/cacheLifecycle.test.ts index 27b0fd0400..0cad7db957 100644 --- a/src/query/tests/cacheLifecycle.test.ts +++ b/src/query/tests/cacheLifecycle.test.ts @@ -1,7 +1,6 @@ import { createApi } from '@reduxjs/toolkit/query' -import { waitFor } from '@testing-library/react' import { fetchBaseQuery } from '../fetchBaseQuery' -import { setupApiStore, waitMs } from './helpers' +import { fakeTimerWaitFor, setupApiStore, waitMs } from './helpers' beforeAll(() => { jest.useFakeTimers() @@ -95,7 +94,7 @@ test('query: await firstValueResolved, await cleanup (success)', async () => { expect(gotFirstValue).not.toHaveBeenCalled() expect(onCleanup).not.toHaveBeenCalled() - await waitFor(() => { + await fakeTimerWaitFor(() => { expect(gotFirstValue).toHaveBeenCalled() }) expect(gotFirstValue).toHaveBeenCalledWith({ value: 'success' }) @@ -291,7 +290,7 @@ test('getCacheEntry', async () => { ) promise.unsubscribe() - await waitFor(() => { + await fakeTimerWaitFor(() => { expect(gotFirstValue).toHaveBeenCalled() }) @@ -332,3 +331,67 @@ test('getCacheEntry', async () => { status: 'uninitialized', }) }) + +test('updateCacheEntry', async () => { + const trackCalls = jest.fn() + + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query<{ value: string }, string>({ + query: () => '/success', + async onCacheEntryAdded( + arg, + { dispatch, getState, getCacheEntry, updateCacheEntry }, + { cleanup, firstValueResolved } + ) { + expect(getCacheEntry().data).toEqual(undefined) + // calling `updateCacheEntry` when there is no data yet should not do anything + updateCacheEntry((draft) => { + draft.value = 'TEST' + trackCalls() + }) + expect(trackCalls).toHaveBeenCalledTimes(0) + expect(getCacheEntry().data).toEqual(undefined) + + gotFirstValue(await firstValueResolved) + + expect(getCacheEntry().data).toEqual({ value: 'success' }) + updateCacheEntry((draft) => { + draft.value = 'TEST' + trackCalls() + }) + expect(trackCalls).toHaveBeenCalledTimes(1) + expect(getCacheEntry().data).toEqual({ value: 'TEST' }) + + await cleanup + + expect(getCacheEntry().data).toEqual(undefined) + // calling `updateCacheEntry` when there is no data any more should not do anything + updateCacheEntry((draft) => { + draft.value = 'TEST2' + trackCalls() + }) + expect(trackCalls).toHaveBeenCalledTimes(1) + expect(getCacheEntry().data).toEqual(undefined) + + onCleanup() + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + promise.unsubscribe() + + await fakeTimerWaitFor(() => { + expect(gotFirstValue).toHaveBeenCalled() + }) + + jest.advanceTimersByTime(61000) + + await fakeTimerWaitFor(() => { + expect(onCleanup).toHaveBeenCalled() + }) +}) diff --git a/src/query/tests/helpers.tsx b/src/query/tests/helpers.tsx index 951b84d35e..0d1978c949 100644 --- a/src/query/tests/helpers.tsx +++ b/src/query/tests/helpers.tsx @@ -55,6 +55,7 @@ export const hookWaitFor = async (cb: () => void, time = 2000) => { } } } +export const fakeTimerWaitFor = hookWaitFor export const useRenderCounter = () => { const countRef = React.useRef(0) diff --git a/src/query/tests/optimisticUpdates.test.tsx b/src/query/tests/optimisticUpdates.test.tsx index 9a42dce8da..8d9643c956 100644 --- a/src/query/tests/optimisticUpdates.test.tsx +++ b/src/query/tests/optimisticUpdates.test.tsx @@ -36,15 +36,13 @@ const api = createApi({ method: 'PATCH', body: patch, }), - onStart({ id, ...patch }, { dispatch, context }) { - context.undoPost = dispatch( + async onQuery({ id, ...patch }, { dispatch }, { resultPromise }) { + const { undo } = dispatch( api.util.updateQueryResult('post', id, (draft) => { Object.assign(draft, patch) }) - ).inversePatches - }, - onError({ id }, { dispatch, context }) { - dispatch(api.util.patchQueryResult('post', id, context.undoPost)) + ) + resultPromise.catch(undo) }, invalidatesTags: ['Post'], }), @@ -173,6 +171,7 @@ describe('updateQueryResult', () => { expect(returnValue).toEqual({ inversePatches: [{ op: 'replace', path: ['contents'], value: 'TODO' }], patches: [{ op: 'replace', path: ['contents'], value: 'I love cheese!' }], + undo: expect.any(Function), }) act(() => { @@ -216,6 +215,7 @@ describe('updateQueryResult', () => { expect(returnValue).toEqual({ inversePatches: [], patches: [], + undo: expect.any(Function), }) }) }) diff --git a/src/query/tests/queryLifecycle.test.tsx b/src/query/tests/queryLifecycle.test.tsx index 2760413cf6..b3a534c4db 100644 --- a/src/query/tests/queryLifecycle.test.tsx +++ b/src/query/tests/queryLifecycle.test.tsx @@ -2,6 +2,8 @@ import { createApi } from '@reduxjs/toolkit/query' import { waitFor } from '@testing-library/react' import { fetchBaseQuery } from '../fetchBaseQuery' import { setupApiStore } from './helpers' +import { server } from './mocks/server' +import { rest } from 'msw' const api = createApi({ baseQuery: fetchBaseQuery({ baseUrl: 'http://example.com' }), @@ -210,6 +212,79 @@ test('getCacheEntry (success)', async () => { }) }) +test('updateCacheEntry', async () => { + const trackCalls = jest.fn() + + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query<{ value: string }, string>({ + query: () => '/success', + async onQuery( + arg, + { dispatch, getState, getCacheEntry, updateCacheEntry }, + { resultPromise } + ) { + // calling `updateCacheEntry` when there is no data yet should not do anything + // but if there is a cache value it will be updated & overwritten by the next succesful result + updateCacheEntry((draft) => { + draft.value += '.' + }) + + try { + const val = await resultPromise + onSuccess(getCacheEntry().data) + } catch (error) { + updateCacheEntry((draft) => { + draft.value += 'x' + }) + onError(getCacheEntry().data) + } + }, + }), + }), + }) + + // request 1: success + expect(onSuccess).not.toHaveBeenCalled() + storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) + + await waitFor(() => { + expect(onSuccess).toHaveBeenCalled() + }) + expect(onSuccess).toHaveBeenCalledWith({ value: 'success' }) + onSuccess.mockClear() + + // request 2: error + expect(onError).not.toHaveBeenCalled() + server.use( + rest.get('http://example.com/success', (_, req, ctx) => + req.once(ctx.status(500), ctx.json({ value: 'failed' })) + ) + ) + storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg', { forceRefetch: true }) + ) + + await waitFor(() => { + expect(onError).toHaveBeenCalled() + }) + expect(onError).toHaveBeenCalledWith({ value: 'success.x' }) + + // request 3: success + expect(onSuccess).not.toHaveBeenCalled() + + storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg', { forceRefetch: true }) + ) + + await waitFor(() => { + expect(onSuccess).toHaveBeenCalled() + }) + expect(onSuccess).toHaveBeenCalledWith({ value: 'success' }) + onSuccess.mockClear() +}) + /* other test scenarios: From 35c658c429745e18e3f0c88f44480099e6214427 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sat, 8 May 2021 16:01:49 +0200 Subject: [PATCH 13/18] fix weird type inference test error? --- package-lock.json | 18 ------------------ src/query/tests/helpers.tsx | 31 +++++++++++++------------------ 2 files changed, 13 insertions(+), 36 deletions(-) diff --git a/package-lock.json b/package-lock.json index a50a27e8ce..dc511a7566 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2805,18 +2805,6 @@ "@types/react": "*", "hoist-non-react-statics": "^3.3.0", "redux": "^4.0.0" - }, - "dependencies": { - "redux": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/redux/-/redux-4.0.5.tgz", - "integrity": "sha512-VSz1uMAH24DM6MF72vcojpYPtrTUu3ByVWfPL1nPfVRb5mZVTve5GnNCUV53QM/BZ66xfWrm0CTWoM+Xlz8V1w==", - "dev": true, - "requires": { - "loose-envify": "^1.4.0", - "symbol-observable": "^1.2.0" - } - } } }, "@types/react-test-renderer": { @@ -11836,12 +11824,6 @@ } } }, - "symbol-observable": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/symbol-observable/-/symbol-observable-1.2.0.tgz", - "integrity": "sha512-e900nM8RRtGhlV36KGEU9k65K3mPb1WV70OdjfxlG2EAuM1noi/E/BaW/uMhL7bPEssK8QV57vN3esixjUvcXQ==", - "dev": true - }, "symbol-tree": { "version": "3.2.4", "resolved": "https://registry.npmjs.org/symbol-tree/-/symbol-tree-3.2.4.tgz", diff --git a/src/query/tests/helpers.tsx b/src/query/tests/helpers.tsx index 0d1978c949..1c4903896e 100644 --- a/src/query/tests/helpers.tsx +++ b/src/query/tests/helpers.tsx @@ -107,38 +107,33 @@ export const actionsReducer = { export function setupApiStore< A extends { - reducerPath: any + reducerPath: 'api' reducer: Reducer middleware: Middleware util: { resetApiState(): any } }, - R extends Record> + R extends Record> = Record >(api: A, extraReducers?: R, withoutListeners?: boolean) { const getStore = () => configureStore({ - reducer: { [api.reducerPath]: api.reducer, ...extraReducers }, + reducer: { api: api.reducer, ...extraReducers }, middleware: (gdm) => gdm({ serializableCheck: false, immutableCheck: false }).concat( api.middleware ), }) - type StoreType = ReturnType extends EnhancedStore< - {}, - any, - infer M + type StoreType = EnhancedStore< + { + api: ReturnType + } & { + [K in keyof R]: ReturnType + }, + AnyAction, + ReturnType extends EnhancedStore + ? M + : never > - ? EnhancedStore< - { - [K in A['reducerPath']]: ReturnType - } & - { - [K in keyof R]: ReturnType - }, - AnyAction, - M - > - : never const initialStore = getStore() as StoreType const refObj = { From 0624edd916ec6db63eddae90f8a2798ebee55a9b Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Wed, 12 May 2021 18:52:23 +0200 Subject: [PATCH 14/18] fix duplicate running lifecycles --- .../core/buildMiddleware/cacheLifecycle.ts | 5 +++- src/query/tests/cacheLifecycle.test.ts | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index 916725b285..4ff934f7bd 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -147,13 +147,16 @@ export const build: SubMiddlewareBuilder = ({ const lifecycleMap: Record = {} return (next) => (action): any => { + const stateBefore = mwApi.getState() + const result = next(action) const cacheKey = getCacheKey(action) if (queryThunk.pending.match(action)) { + const oldState = stateBefore[reducerPath].queries[cacheKey] const state = mwApi.getState()[reducerPath].queries[cacheKey] - if (state?.requestId === action.meta.requestId) { + if (!oldState && state) { handleNewKey( action.meta.arg.endpointName, action.meta.arg.originalArgs, diff --git a/src/query/tests/cacheLifecycle.test.ts b/src/query/tests/cacheLifecycle.test.ts index 0cad7db957..78d84c9f19 100644 --- a/src/query/tests/cacheLifecycle.test.ts +++ b/src/query/tests/cacheLifecycle.test.ts @@ -395,3 +395,27 @@ test('updateCacheEntry', async () => { expect(onCleanup).toHaveBeenCalled() }) }) + +test('dispatching further actions does not trigger another lifecycle', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/success', + async onCacheEntryAdded() { + onNewCacheEntry() + }, + }), + }), + }) + await storeRef.store.dispatch(extended.endpoints.injected.initiate()) + expect(onNewCacheEntry).toHaveBeenCalledTimes(1) + + await storeRef.store.dispatch(extended.endpoints.injected.initiate()) + expect(onNewCacheEntry).toHaveBeenCalledTimes(1) + + await storeRef.store.dispatch( + extended.endpoints.injected.initiate(undefined, { forceRefetch: true }) + ) + expect(onNewCacheEntry).toHaveBeenCalledTimes(1) +}) From 8a52b3430ff6dca9957b35ec2889fef27ac0b9ef Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Wed, 12 May 2021 23:09:04 +0200 Subject: [PATCH 15/18] mutation tests, mutation unsubscribe fix, mutation selector fix --- .../core/buildMiddleware/cacheLifecycle.ts | 9 +- src/query/tests/cacheLifecycle.test.ts | 578 +++++++++++------- 2 files changed, 365 insertions(+), 222 deletions(-) diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index 4ff934f7bd..79d3cfa9ca 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -206,6 +206,8 @@ export const build: SubMiddlewareBuilder = ({ if (isMutationThunk(action)) return action.meta.requestId if (api.internalActions.removeQueryResult.match(action)) return action.payload.queryCacheKey + if (api.internalActions.unsubscribeMutationResult.match(action)) + return action.payload.requestId return '' } @@ -239,7 +241,12 @@ export const build: SubMiddlewareBuilder = ({ ]) ) lifecycleMap[queryCacheKey] = lifecycle - const selector = (api.endpoints[endpointName] as any).select(originalArgs) + const selector = (api.endpoints[endpointName] as any).select( + endpointDefinition.type === DefinitionType.query + ? originalArgs + : queryCacheKey + ) + const extra = mwApi.dispatch((_, __, extra) => extra) const lifecycleApi = { ...mwApi, diff --git a/src/query/tests/cacheLifecycle.test.ts b/src/query/tests/cacheLifecycle.test.ts index 78d84c9f19..f9cfcc4ee8 100644 --- a/src/query/tests/cacheLifecycle.test.ts +++ b/src/query/tests/cacheLifecycle.test.ts @@ -24,49 +24,278 @@ beforeEach(() => { onCatch.mockClear() }) -test('query: new cache entry only', async () => { - const extended = api.injectEndpoints({ - overrideExisting: true, - endpoints: (build) => ({ - injected: build.query({ - query: () => '/success', - onCacheEntryAdded(arg, { dispatch, getState }) { - onNewCacheEntry(arg) - }, - }), - }), - }) - storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) - expect(onNewCacheEntry).toHaveBeenCalledWith('arg') -}) - -test('query: await cleanup', async () => { - const extended = api.injectEndpoints({ - overrideExisting: true, - endpoints: (build) => ({ - injected: build.query({ - query: () => '/success', - async onCacheEntryAdded(arg, { dispatch, getState }, { cleanup }) { - onNewCacheEntry(arg) - await cleanup - onCleanup() - }, - }), - }), - }) - storeRef.store - .dispatch(extended.endpoints.injected.initiate('arg')) - .unsubscribe() - - expect(onNewCacheEntry).toHaveBeenCalledWith('arg') - expect(onCleanup).not.toHaveBeenCalled() - jest.advanceTimersByTime(59000), await waitMs() - expect(onCleanup).not.toHaveBeenCalled() - jest.advanceTimersByTime(2000), await waitMs() - expect(onCleanup).toHaveBeenCalled() -}) - -test('query: await firstValueResolved, await cleanup (success)', async () => { +describe.each([['query'], ['mutation']] as const)( + 'generic cases: %s', + (type) => { + test(`${type}: new cache entry only`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/success', + onCacheEntryAdded(arg, { dispatch, getState }) { + onNewCacheEntry(arg) + }, + }), + }), + }) + storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + }) + + test(`${type}: await cleanup`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/success', + async onCacheEntryAdded(arg, { dispatch, getState }, { cleanup }) { + onNewCacheEntry(arg) + await cleanup + onCleanup() + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + expect(onCleanup).not.toHaveBeenCalled() + + promise.unsubscribe(), await waitMs() + if (type === 'query') { + jest.advanceTimersByTime(59000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000), await waitMs() + } + + expect(onCleanup).toHaveBeenCalled() + }) + + test(`${type}: await firstValueResolved, await cleanup (success)`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/success', + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + await cleanup + onCleanup() + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCleanup).not.toHaveBeenCalled() + + await fakeTimerWaitFor(() => { + expect(gotFirstValue).toHaveBeenCalled() + }) + expect(gotFirstValue).toHaveBeenCalledWith({ value: 'success' }) + expect(onCleanup).not.toHaveBeenCalled() + + promise.unsubscribe(), await waitMs() + if (type === 'query') { + jest.advanceTimersByTime(59000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000), await waitMs() + } + + expect(onCleanup).toHaveBeenCalled() + }) + + test(`${type}: await firstValueResolved, await cleanup (firstValueResolved never resolves)`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + // this will wait until cleanup, then reject => nothing past that line will execute + // but since this special "cleanup" rejection is handled outside, there will be no + // uncaught rejection error + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + await cleanup + onCleanup() + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + + promise.unsubscribe(), await waitMs() + if (type === 'query') { + jest.advanceTimersByTime(120000), await waitMs() + } + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCleanup).not.toHaveBeenCalled() + }) + + test(`${type}: try { await firstValueResolved }, await cleanup (firstValueResolved never resolves)`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + + try { + // this will wait until cleanup, then reject => nothing else in this try..catch block will execute + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + } catch (e) { + onCatch(e) + } + await cleanup + onCleanup() + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + promise.unsubscribe(), await waitMs() + if (type === 'query') { + jest.advanceTimersByTime(59000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000), await waitMs() + } + + expect(onCleanup).toHaveBeenCalled() + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCatch.mock.calls[0][0]).toMatchObject({ + message: 'Promise never resolved before cleanup.', + }) + }) + + test(`${type}: try { await firstValueResolved, await cleanup } (firstValueResolved never resolves)`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + + try { + // this will wait until cleanup, then reject => nothing else in this try..catch block will execute + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + // cleanup in this scenario only needs to be done for stuff within this try..catch block - totally valid scenario + await cleanup + onCleanup() + } catch (e) { + onCatch(e) + } + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + + promise.unsubscribe(), await waitMs() + if (type === 'query') { + jest.advanceTimersByTime(59000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000), await waitMs() + } + expect(onCleanup).not.toHaveBeenCalled() + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCatch.mock.calls[0][0]).toMatchObject({ + message: 'Promise never resolved before cleanup.', + }) + }) + + test(`${type}: try { await firstValueResolved } finally { await cleanup } (firstValueResolved never resolves)`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve + async onCacheEntryAdded( + arg, + { dispatch, getState }, + { cleanup, firstValueResolved } + ) { + onNewCacheEntry(arg) + + try { + // this will wait until cleanup, then reject => nothing else in this try..catch block will execute + const firstValue = await firstValueResolved + gotFirstValue(firstValue) + } catch (e) { + onCatch(e) + } finally { + await cleanup + onCleanup() + } + }, + }), + }), + }) + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + + expect(onNewCacheEntry).toHaveBeenCalledWith('arg') + + promise.unsubscribe(), await waitMs() + if (type === 'query') { + jest.advanceTimersByTime(59000), await waitMs() + expect(onCleanup).not.toHaveBeenCalled() + jest.advanceTimersByTime(2000), await waitMs() + } + expect(onCleanup).toHaveBeenCalled() + expect(gotFirstValue).not.toHaveBeenCalled() + expect(onCatch.mock.calls[0][0]).toMatchObject({ + message: 'Promise never resolved before cleanup.', + }) + }) + } +) + +test(`query: getCacheEntry`, async () => { + const snapshot = jest.fn() const extended = api.injectEndpoints({ overrideExisting: true, endpoints: (build) => ({ @@ -74,202 +303,71 @@ test('query: await firstValueResolved, await cleanup (success)', async () => { query: () => '/success', async onCacheEntryAdded( arg, - { dispatch, getState }, + { dispatch, getState, getCacheEntry }, { cleanup, firstValueResolved } ) { - onNewCacheEntry(arg) - const firstValue = await firstValueResolved - gotFirstValue(firstValue) + snapshot(getCacheEntry()) + gotFirstValue(await firstValueResolved) + snapshot(getCacheEntry()) await cleanup - onCleanup() + snapshot(getCacheEntry()) }, }), }), }) - storeRef.store - .dispatch(extended.endpoints.injected.initiate('arg')) - .unsubscribe() - expect(onNewCacheEntry).toHaveBeenCalledWith('arg') - - expect(gotFirstValue).not.toHaveBeenCalled() - expect(onCleanup).not.toHaveBeenCalled() + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + promise.unsubscribe() await fakeTimerWaitFor(() => { expect(gotFirstValue).toHaveBeenCalled() }) - expect(gotFirstValue).toHaveBeenCalledWith({ value: 'success' }) - expect(onCleanup).not.toHaveBeenCalled() - - jest.advanceTimersByTime(59000), await waitMs() - expect(onCleanup).not.toHaveBeenCalled() - jest.advanceTimersByTime(2000), await waitMs() - expect(onCleanup).toHaveBeenCalled() -}) - -test('query: await firstValueResolved, await cleanup (firstValueResolved never resolves)', async () => { - const extended = api.injectEndpoints({ - overrideExisting: true, - endpoints: (build) => ({ - injected: build.query({ - query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve - async onCacheEntryAdded( - arg, - { dispatch, getState }, - { cleanup, firstValueResolved } - ) { - onNewCacheEntry(arg) - // this will wait until cleanup, then reject => nothing past that line will execute - // but since this special "cleanup" rejection is handled outside, there will be no - // uncaught rejection error - const firstValue = await firstValueResolved - gotFirstValue(firstValue) - await cleanup - onCleanup() - }, - }), - }), - }) - storeRef.store - .dispatch(extended.endpoints.injected.initiate('arg')) - .unsubscribe() - expect(onNewCacheEntry).toHaveBeenCalledWith('arg') jest.advanceTimersByTime(120000), await waitMs() - expect(gotFirstValue).not.toHaveBeenCalled() - expect(onCleanup).not.toHaveBeenCalled() -}) -test('query: try { await firstValueResolved }, await cleanup (firstValueResolved never resolves)', async () => { - const extended = api.injectEndpoints({ - overrideExisting: true, - endpoints: (build) => ({ - injected: build.query({ - query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve - async onCacheEntryAdded( - arg, - { dispatch, getState }, - { cleanup, firstValueResolved } - ) { - onNewCacheEntry(arg) - - try { - // this will wait until cleanup, then reject => nothing else in this try..catch block will execute - const firstValue = await firstValueResolved - gotFirstValue(firstValue) - } catch (e) { - onCatch(e) - } - await cleanup - onCleanup() - }, - }), - }), - }) - storeRef.store - .dispatch(extended.endpoints.injected.initiate('arg')) - .unsubscribe() - expect(onNewCacheEntry).toHaveBeenCalledWith('arg') - - jest.advanceTimersByTime(59000), await waitMs() - expect(onCleanup).not.toHaveBeenCalled() - jest.advanceTimersByTime(2000), await waitMs() - expect(onCleanup).toHaveBeenCalled() - expect(gotFirstValue).not.toHaveBeenCalled() - expect(onCatch.mock.calls[0][0]).toMatchObject({ - message: 'Promise never resolved before cleanup.', - }) -}) - -test('query: try { await firstValueResolved, await cleanup } (firstValueResolved never resolves)', async () => { - const extended = api.injectEndpoints({ - overrideExisting: true, - endpoints: (build) => ({ - injected: build.query({ - query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve - async onCacheEntryAdded( - arg, - { dispatch, getState }, - { cleanup, firstValueResolved } - ) { - onNewCacheEntry(arg) - - try { - // this will wait until cleanup, then reject => nothing else in this try..catch block will execute - const firstValue = await firstValueResolved - gotFirstValue(firstValue) - // cleanup in this scenario only needs to be done for stuff within this try..catch block - totally valid scenario - await cleanup - onCleanup() - } catch (e) { - onCatch(e) - } - }, - }), - }), - }) - storeRef.store - .dispatch(extended.endpoints.injected.initiate('arg')) - .unsubscribe() - expect(onNewCacheEntry).toHaveBeenCalledWith('arg') - - jest.advanceTimersByTime(59000), await waitMs() - expect(onCleanup).not.toHaveBeenCalled() - jest.advanceTimersByTime(2000), await waitMs() - expect(onCleanup).not.toHaveBeenCalled() - expect(gotFirstValue).not.toHaveBeenCalled() - expect(onCatch.mock.calls[0][0]).toMatchObject({ - message: 'Promise never resolved before cleanup.', + expect(snapshot).toHaveBeenCalledTimes(3) + expect(snapshot.mock.calls[0][0]).toMatchObject({ + endpointName: 'injected', + isError: false, + isLoading: true, + isSuccess: false, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'pending', }) -}) - -test('query: try { await firstValueResolved } finally { await cleanup } (firstValueResolved never resolves)', async () => { - const extended = api.injectEndpoints({ - overrideExisting: true, - endpoints: (build) => ({ - injected: build.query({ - query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve - async onCacheEntryAdded( - arg, - { dispatch, getState }, - { cleanup, firstValueResolved } - ) { - onNewCacheEntry(arg) - - try { - // this will wait until cleanup, then reject => nothing else in this try..catch block will execute - const firstValue = await firstValueResolved - gotFirstValue(firstValue) - } catch (e) { - onCatch(e) - } finally { - await cleanup - onCleanup() - } - }, - }), - }), + expect(snapshot.mock.calls[1][0]).toMatchObject({ + data: { + value: 'success', + }, + endpointName: 'injected', + fulfilledTimeStamp: expect.any(Number), + isError: false, + isLoading: false, + isSuccess: true, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'fulfilled', }) - storeRef.store - .dispatch(extended.endpoints.injected.initiate('arg')) - .unsubscribe() - expect(onNewCacheEntry).toHaveBeenCalledWith('arg') - - jest.advanceTimersByTime(59000), await waitMs() - expect(onCleanup).not.toHaveBeenCalled() - jest.advanceTimersByTime(2000), await waitMs() - expect(onCleanup).toHaveBeenCalled() - expect(gotFirstValue).not.toHaveBeenCalled() - expect(onCatch.mock.calls[0][0]).toMatchObject({ - message: 'Promise never resolved before cleanup.', + expect(snapshot.mock.calls[2][0]).toMatchObject({ + isError: false, + isLoading: false, + isSuccess: false, + isUninitialized: true, + status: 'uninitialized', }) }) -test('getCacheEntry', async () => { +test(`mutation: getCacheEntry`, async () => { const snapshot = jest.fn() const extended = api.injectEndpoints({ overrideExisting: true, endpoints: (build) => ({ - injected: build.query({ + injected: build.mutation({ query: () => '/success', async onCacheEntryAdded( arg, @@ -288,13 +386,11 @@ test('getCacheEntry', async () => { const promise = storeRef.store.dispatch( extended.endpoints.injected.initiate('arg') ) - promise.unsubscribe() - await fakeTimerWaitFor(() => { expect(gotFirstValue).toHaveBeenCalled() }) - jest.advanceTimersByTime(120000), await waitMs() + promise.unsubscribe(), await waitMs() expect(snapshot).toHaveBeenCalledTimes(3) expect(snapshot.mock.calls[0][0]).toMatchObject({ @@ -304,7 +400,6 @@ test('getCacheEntry', async () => { isSuccess: false, isUninitialized: false, originalArgs: 'arg', - requestId: promise.requestId, startedTimeStamp: expect.any(Number), status: 'pending', }) @@ -319,7 +414,6 @@ test('getCacheEntry', async () => { isSuccess: true, isUninitialized: false, originalArgs: 'arg', - requestId: promise.requestId, startedTimeStamp: expect.any(Number), status: 'fulfilled', }) @@ -419,3 +513,45 @@ test('dispatching further actions does not trigger another lifecycle', async () ) expect(onNewCacheEntry).toHaveBeenCalledTimes(1) }) + +test('dispatching a query initializer with `subscribe: false` does not start a lifecycle', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.query({ + query: () => '/success', + async onCacheEntryAdded() { + onNewCacheEntry() + }, + }), + }), + }) + await storeRef.store.dispatch( + extended.endpoints.injected.initiate(undefined, { subscribe: false }) + ) + expect(onNewCacheEntry).toHaveBeenCalledTimes(0) + + await storeRef.store.dispatch(extended.endpoints.injected.initiate(undefined)) + expect(onNewCacheEntry).toHaveBeenCalledTimes(1) +}) + +test('dispatching a mutation initializer with `track: false` does not start a lifecycle', async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build.mutation({ + query: () => '/success', + async onCacheEntryAdded() { + onNewCacheEntry() + }, + }), + }), + }) + await storeRef.store.dispatch( + extended.endpoints.injected.initiate(undefined, { track: false }) + ) + expect(onNewCacheEntry).toHaveBeenCalledTimes(0) + + await storeRef.store.dispatch(extended.endpoints.injected.initiate(undefined)) + expect(onNewCacheEntry).toHaveBeenCalledTimes(1) +}) From 97555347bf01edaab57c077e358fb7270296dd60 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Thu, 13 May 2021 11:47:15 +0200 Subject: [PATCH 16/18] fix queryLifecycle mutation `getCacheEntry`, more tests --- .../core/buildMiddleware/queryLifecycle.ts | 4 +- src/query/tests/queryLifecycle.test.tsx | 219 +++++++++++++----- 2 files changed, 170 insertions(+), 53 deletions(-) diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index 811959a66d..78f9e4c0e6 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -88,7 +88,9 @@ export const build: SubMiddlewareBuilder = ({ ) lifecycleMap[requestId] = lifecycle const selector = (api.endpoints[endpointName] as any).select( - originalArgs + endpointDefinition.type === DefinitionType.query + ? originalArgs + : requestId ) const extra = mwApi.dispatch((_, __, extra) => extra) diff --git a/src/query/tests/queryLifecycle.test.tsx b/src/query/tests/queryLifecycle.test.tsx index b3a534c4db..d50d485940 100644 --- a/src/query/tests/queryLifecycle.test.tsx +++ b/src/query/tests/queryLifecycle.test.tsx @@ -21,80 +21,208 @@ beforeEach(() => { onError.mockClear() }) -test('query: onStart only', async () => { - const extended = api.injectEndpoints({ - overrideExisting: true, - endpoints: (build) => ({ - injected: build.query({ - query: () => '/success', - onQuery(arg) { - onStart(arg) - }, - }), - }), - }) - storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) - expect(onStart).toHaveBeenCalledWith('arg') -}) +describe.each([['query'], ['mutation']] as const)( + 'generic cases: %s', + (type) => { + test(`${type}: onStart only`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/success', + onQuery(arg) { + onStart(arg) + }, + }), + }), + }) + storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) + expect(onStart).toHaveBeenCalledWith('arg') + }) + + test(`${type}: onStart and onSuccess`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/success', + async onQuery(arg, {}, { resultPromise }) { + onStart(arg) + // awaiting without catching like this would result in an `unhandledRejection` exception if there was an error + // unfortunately we cannot test for that in jest. + const result = await resultPromise + onSuccess(result) + }, + }), + }), + }) + storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) + expect(onStart).toHaveBeenCalledWith('arg') + await waitFor(() => { + expect(onSuccess).toHaveBeenCalledWith({ value: 'success' }) + }) + }) + + test(`${type}: onStart and onError`, async () => { + const extended = api.injectEndpoints({ + overrideExisting: true, + endpoints: (build) => ({ + injected: build[type as 'mutation']({ + query: () => '/error', + async onQuery(arg, {}, { resultPromise }) { + onStart(arg) + try { + const result = await resultPromise + onSuccess(result) + } catch (e) { + onError(e) + } + }, + }), + }), + }) + storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) + expect(onStart).toHaveBeenCalledWith('arg') + await waitFor(() => { + expect(onError).toHaveBeenCalledWith({ + status: 500, + data: { value: 'error' }, + }) + }) + expect(onSuccess).not.toHaveBeenCalled() + }) + } +) -test('query: onStart and onSuccess', async () => { +test('query: getCacheEntry (success)', async () => { + const snapshot = jest.fn() const extended = api.injectEndpoints({ overrideExisting: true, endpoints: (build) => ({ injected: build.query({ query: () => '/success', - async onQuery(arg, {}, { resultPromise }) { - onStart(arg) - // awaiting without catching like this would result in an `unhandledRejection` exception if there was an error - // unfortunately we cannot test for that in jest. - const result = await resultPromise - onSuccess(result) + async onQuery( + arg, + { dispatch, getState, getCacheEntry }, + { resultPromise } + ) { + try { + snapshot(getCacheEntry()) + const result = await resultPromise + onSuccess(result) + snapshot(getCacheEntry()) + } catch (e) { + onError(e) + snapshot(getCacheEntry()) + } }, }), }), }) - storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) - expect(onStart).toHaveBeenCalledWith('arg') + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + await waitFor(() => { - expect(onSuccess).toHaveBeenCalledWith({ value: 'success' }) + expect(onSuccess).toHaveBeenCalled() + }) + + expect(snapshot).toHaveBeenCalledTimes(2) + expect(snapshot.mock.calls[0][0]).toMatchObject({ + endpointName: 'injected', + isError: false, + isLoading: true, + isSuccess: false, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'pending', + }) + expect(snapshot.mock.calls[1][0]).toMatchObject({ + data: { + value: 'success', + }, + endpointName: 'injected', + fulfilledTimeStamp: expect.any(Number), + isError: false, + isLoading: false, + isSuccess: true, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'fulfilled', }) }) -test('query: onStart, onSuccess and onError', async () => { +test('query: getCacheEntry (error)', async () => { + const snapshot = jest.fn() const extended = api.injectEndpoints({ overrideExisting: true, endpoints: (build) => ({ injected: build.query({ query: () => '/error', - async onQuery(arg, {}, { resultPromise }) { - onStart(arg) + async onQuery( + arg, + { dispatch, getState, getCacheEntry }, + { resultPromise } + ) { try { + snapshot(getCacheEntry()) const result = await resultPromise onSuccess(result) + snapshot(getCacheEntry()) } catch (e) { onError(e) + snapshot(getCacheEntry()) } }, }), }), }) - storeRef.store.dispatch(extended.endpoints.injected.initiate('arg')) - expect(onStart).toHaveBeenCalledWith('arg') + const promise = storeRef.store.dispatch( + extended.endpoints.injected.initiate('arg') + ) + await waitFor(() => { - expect(onError).toHaveBeenCalledWith({ - status: 500, + expect(onError).toHaveBeenCalled() + }) + + expect(snapshot.mock.calls[0][0]).toMatchObject({ + endpointName: 'injected', + isError: false, + isLoading: true, + isSuccess: false, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'pending', + }) + expect(snapshot.mock.calls[1][0]).toMatchObject({ + error: { data: { value: 'error' }, - }) + status: 500, + }, + endpointName: 'injected', + isError: true, + isLoading: false, + isSuccess: false, + isUninitialized: false, + originalArgs: 'arg', + requestId: promise.requestId, + startedTimeStamp: expect.any(Number), + status: 'rejected', }) - expect(onSuccess).not.toHaveBeenCalled() }) -test('getCacheEntry (success)', async () => { +test('mutation: getCacheEntry (success)', async () => { const snapshot = jest.fn() const extended = api.injectEndpoints({ overrideExisting: true, endpoints: (build) => ({ - injected: build.query({ + injected: build.mutation({ query: () => '/success', async onQuery( arg, @@ -130,7 +258,6 @@ test('getCacheEntry (success)', async () => { isSuccess: false, isUninitialized: false, originalArgs: 'arg', - requestId: promise.requestId, startedTimeStamp: expect.any(Number), status: 'pending', }) @@ -145,18 +272,17 @@ test('getCacheEntry (success)', async () => { isSuccess: true, isUninitialized: false, originalArgs: 'arg', - requestId: promise.requestId, startedTimeStamp: expect.any(Number), status: 'fulfilled', }) }) -test('getCacheEntry (success)', async () => { +test('mutation: getCacheEntry (error)', async () => { const snapshot = jest.fn() const extended = api.injectEndpoints({ overrideExisting: true, endpoints: (build) => ({ - injected: build.query({ + injected: build.mutation({ query: () => '/error', async onQuery( arg, @@ -191,7 +317,6 @@ test('getCacheEntry (success)', async () => { isSuccess: false, isUninitialized: false, originalArgs: 'arg', - requestId: promise.requestId, startedTimeStamp: expect.any(Number), status: 'pending', }) @@ -206,13 +331,12 @@ test('getCacheEntry (success)', async () => { isSuccess: false, isUninitialized: false, originalArgs: 'arg', - requestId: promise.requestId, startedTimeStamp: expect.any(Number), status: 'rejected', }) }) -test('updateCacheEntry', async () => { +test('query: updateCacheEntry', async () => { const trackCalls = jest.fn() const extended = api.injectEndpoints({ @@ -284,12 +408,3 @@ test('updateCacheEntry', async () => { expect(onSuccess).toHaveBeenCalledWith({ value: 'success' }) onSuccess.mockClear() }) - -/* - other test scenarios: - - - cleanup happens before the query resolves -> should reject the promise - - cleanup happens before the query resolves -> should reject the promise, but the promise should not cause an unhandledRejection if not caught -*/ From cc22a091c0bcccd67650fdc7f844d6e605f78bca Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Thu, 13 May 2021 12:13:02 +0200 Subject: [PATCH 17/18] move promises into api object --- .../core/buildMiddleware/cacheLifecycle.ts | 42 +++++++++++++++---- .../core/buildMiddleware/queryLifecycle.ts | 30 ++++++++++--- src/query/createApi.ts | 4 +- src/query/tests/cacheLifecycle.test.ts | 33 +++++++-------- src/query/tests/optimisticUpdates.test.tsx | 2 +- src/query/tests/queryLifecycle.test.tsx | 19 ++++----- 6 files changed, 84 insertions(+), 46 deletions(-) diff --git a/src/query/core/buildMiddleware/cacheLifecycle.ts b/src/query/core/buildMiddleware/cacheLifecycle.ts index 79d3cfa9ca..b2402689ce 100644 --- a/src/query/core/buildMiddleware/cacheLifecycle.ts +++ b/src/query/core/buildMiddleware/cacheLifecycle.ts @@ -18,7 +18,7 @@ import type { SubMiddlewareApi, SubMiddlewareBuilder } from './types' export type ReferenceCacheLifecycle = never declare module '../../endpointDefinitions' { - export interface QueryLifecycleApi< + export interface QueryBaseLifecycleApi< QueryArg, BaseQuery extends BaseQueryFn, ResultType, @@ -41,7 +41,7 @@ declare module '../../endpointDefinitions' { updateCacheEntry(updateRecipe: Recipe): PatchCollection } - export interface MutationLifecycleApi< + export interface MutationBaseLifecycleApi< QueryArg, BaseQuery extends BaseQueryFn, ResultType, @@ -99,6 +99,27 @@ declare module '../../endpointDefinitions' { cleanup: Promise } + export interface QueryCacheLifecycleApi< + QueryArg, + BaseQuery extends BaseQueryFn, + ResultType, + ReducerPath extends string = string + > extends QueryBaseLifecycleApi, + CacheLifecyclePromises {} + + export interface MutationCacheLifecycleApi< + QueryArg, + BaseQuery extends BaseQueryFn, + ResultType, + ReducerPath extends string = string + > extends MutationBaseLifecycleApi< + QueryArg, + BaseQuery, + ResultType, + ReducerPath + >, + CacheLifecyclePromises {} + interface QueryExtraOptions< TagTypes extends string, ResultType, @@ -108,8 +129,7 @@ declare module '../../endpointDefinitions' { > { onCacheEntryAdded?( arg: QueryArg, - api: QueryLifecycleApi, - promises: CacheLifecyclePromises + api: QueryCacheLifecycleApi ): Promise | void } @@ -122,8 +142,12 @@ declare module '../../endpointDefinitions' { > { onCacheEntryAdded?( arg: QueryArg, - api: MutationLifecycleApi, - promises: CacheLifecyclePromises + api: MutationCacheLifecycleApi< + QueryArg, + BaseQuery, + ResultType, + ReducerPath + > ): Promise | void } } @@ -263,12 +287,12 @@ export const build: SubMiddlewareBuilder = ({ ) ) : undefined) as any, - } - const runningHandler = onCacheEntryAdded(originalArgs, lifecycleApi, { firstValueResolved, cleanup, - }) + } + + const runningHandler = onCacheEntryAdded(originalArgs, lifecycleApi) // if a `neverResolvedError` was thrown, but not handled in the running handler, do not let it leak out further Promise.resolve(runningHandler).catch((e) => { if (e === neverResolvedError) return diff --git a/src/query/core/buildMiddleware/queryLifecycle.ts b/src/query/core/buildMiddleware/queryLifecycle.ts index 78f9e4c0e6..dad278fcc9 100644 --- a/src/query/core/buildMiddleware/queryLifecycle.ts +++ b/src/query/core/buildMiddleware/queryLifecycle.ts @@ -31,8 +31,7 @@ declare module '../../endpointDefinitions' { > { onQuery?( arg: QueryArg, - api: QueryLifecycleApi, - promises: QueryLifecyclePromises + api: QueryLifecycleApi ): Promise | void } @@ -45,10 +44,30 @@ declare module '../../endpointDefinitions' { > { onQuery?( arg: QueryArg, - api: MutationLifecycleApi, - promises: QueryLifecyclePromises + api: MutationLifecycleApi ): Promise | void } + + export interface QueryLifecycleApi< + QueryArg, + BaseQuery extends BaseQueryFn, + ResultType, + ReducerPath extends string = string + > extends QueryBaseLifecycleApi, + QueryLifecyclePromises {} + + export interface MutationLifecycleApi< + QueryArg, + BaseQuery extends BaseQueryFn, + ResultType, + ReducerPath extends string = string + > extends MutationBaseLifecycleApi< + QueryArg, + BaseQuery, + ResultType, + ReducerPath + >, + QueryLifecyclePromises {} } export const build: SubMiddlewareBuilder = ({ @@ -109,8 +128,9 @@ export const build: SubMiddlewareBuilder = ({ ) ) : undefined) as any, + resultPromise, } - onQuery(originalArgs, lifecycleApi, { resultPromise }) + onQuery(originalArgs, lifecycleApi) } } else if (isFullfilledThunk(action)) { const { requestId } = action.meta diff --git a/src/query/createApi.ts b/src/query/createApi.ts index e415025dad..2849f574a9 100644 --- a/src/query/createApi.ts +++ b/src/query/createApi.ts @@ -286,7 +286,7 @@ export function buildCreateApi, ...Module[]]>( '`onStart`, `onSuccess` and `onError` have been replaced by `onQuery`, please change your code accordingly' ) } - x.onQuery ??= async (arg, api, { resultPromise }) => { + x.onQuery ??= async (arg, { resultPromise, ...api }) => { const queryApi = { ...api, context: {} } x.onStart?.(arg, queryApi) try { @@ -321,7 +321,7 @@ export function buildCreateApi, ...Module[]]>( '`onStart`, `onSuccess` and `onError` have been replaced by `onQuery`, please change your code accordingly' ) } - x.onQuery ??= async (arg, api, { resultPromise }) => { + x.onQuery ??= async (arg, { resultPromise, ...api }) => { const queryApi = { ...api, context: {} } x.onStart?.(arg, queryApi) try { diff --git a/src/query/tests/cacheLifecycle.test.ts b/src/query/tests/cacheLifecycle.test.ts index f9cfcc4ee8..2125e1f772 100644 --- a/src/query/tests/cacheLifecycle.test.ts +++ b/src/query/tests/cacheLifecycle.test.ts @@ -49,7 +49,7 @@ describe.each([['query'], ['mutation']] as const)( endpoints: (build) => ({ injected: build[type as 'mutation']({ query: () => '/success', - async onCacheEntryAdded(arg, { dispatch, getState }, { cleanup }) { + async onCacheEntryAdded(arg, { dispatch, getState, cleanup }) { onNewCacheEntry(arg) await cleanup onCleanup() @@ -82,8 +82,7 @@ describe.each([['query'], ['mutation']] as const)( query: () => '/success', async onCacheEntryAdded( arg, - { dispatch, getState }, - { cleanup, firstValueResolved } + { dispatch, getState, cleanup, firstValueResolved } ) { onNewCacheEntry(arg) const firstValue = await firstValueResolved @@ -127,8 +126,7 @@ describe.each([['query'], ['mutation']] as const)( query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve async onCacheEntryAdded( arg, - { dispatch, getState }, - { cleanup, firstValueResolved } + { dispatch, getState, cleanup, firstValueResolved } ) { onNewCacheEntry(arg) // this will wait until cleanup, then reject => nothing past that line will execute @@ -163,8 +161,7 @@ describe.each([['query'], ['mutation']] as const)( query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve async onCacheEntryAdded( arg, - { dispatch, getState }, - { cleanup, firstValueResolved } + { dispatch, getState, cleanup, firstValueResolved } ) { onNewCacheEntry(arg) @@ -208,8 +205,7 @@ describe.each([['query'], ['mutation']] as const)( query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve async onCacheEntryAdded( arg, - { dispatch, getState }, - { cleanup, firstValueResolved } + { dispatch, getState, cleanup, firstValueResolved } ) { onNewCacheEntry(arg) @@ -254,8 +250,7 @@ describe.each([['query'], ['mutation']] as const)( query: () => '/error', // we will initiate only once and that one time will be an error -> firstValueResolved will never resolve async onCacheEntryAdded( arg, - { dispatch, getState }, - { cleanup, firstValueResolved } + { dispatch, getState, cleanup, firstValueResolved } ) { onNewCacheEntry(arg) @@ -303,8 +298,7 @@ test(`query: getCacheEntry`, async () => { query: () => '/success', async onCacheEntryAdded( arg, - { dispatch, getState, getCacheEntry }, - { cleanup, firstValueResolved } + { dispatch, getState, getCacheEntry, cleanup, firstValueResolved } ) { snapshot(getCacheEntry()) gotFirstValue(await firstValueResolved) @@ -371,8 +365,7 @@ test(`mutation: getCacheEntry`, async () => { query: () => '/success', async onCacheEntryAdded( arg, - { dispatch, getState, getCacheEntry }, - { cleanup, firstValueResolved } + { dispatch, getState, getCacheEntry, cleanup, firstValueResolved } ) { snapshot(getCacheEntry()) gotFirstValue(await firstValueResolved) @@ -436,8 +429,14 @@ test('updateCacheEntry', async () => { query: () => '/success', async onCacheEntryAdded( arg, - { dispatch, getState, getCacheEntry, updateCacheEntry }, - { cleanup, firstValueResolved } + { + dispatch, + getState, + getCacheEntry, + updateCacheEntry, + cleanup, + firstValueResolved, + } ) { expect(getCacheEntry().data).toEqual(undefined) // calling `updateCacheEntry` when there is no data yet should not do anything diff --git a/src/query/tests/optimisticUpdates.test.tsx b/src/query/tests/optimisticUpdates.test.tsx index 8d9643c956..166909775d 100644 --- a/src/query/tests/optimisticUpdates.test.tsx +++ b/src/query/tests/optimisticUpdates.test.tsx @@ -36,7 +36,7 @@ const api = createApi({ method: 'PATCH', body: patch, }), - async onQuery({ id, ...patch }, { dispatch }, { resultPromise }) { + async onQuery({ id, ...patch }, { dispatch, resultPromise }) { const { undo } = dispatch( api.util.updateQueryResult('post', id, (draft) => { Object.assign(draft, patch) diff --git a/src/query/tests/queryLifecycle.test.tsx b/src/query/tests/queryLifecycle.test.tsx index d50d485940..882e0afb6c 100644 --- a/src/query/tests/queryLifecycle.test.tsx +++ b/src/query/tests/queryLifecycle.test.tsx @@ -46,7 +46,7 @@ describe.each([['query'], ['mutation']] as const)( endpoints: (build) => ({ injected: build[type as 'mutation']({ query: () => '/success', - async onQuery(arg, {}, { resultPromise }) { + async onQuery(arg, { resultPromise }) { onStart(arg) // awaiting without catching like this would result in an `unhandledRejection` exception if there was an error // unfortunately we cannot test for that in jest. @@ -69,7 +69,7 @@ describe.each([['query'], ['mutation']] as const)( endpoints: (build) => ({ injected: build[type as 'mutation']({ query: () => '/error', - async onQuery(arg, {}, { resultPromise }) { + async onQuery(arg, { resultPromise }) { onStart(arg) try { const result = await resultPromise @@ -103,8 +103,7 @@ test('query: getCacheEntry (success)', async () => { query: () => '/success', async onQuery( arg, - { dispatch, getState, getCacheEntry }, - { resultPromise } + { dispatch, getState, getCacheEntry, resultPromise } ) { try { snapshot(getCacheEntry()) @@ -165,8 +164,7 @@ test('query: getCacheEntry (error)', async () => { query: () => '/error', async onQuery( arg, - { dispatch, getState, getCacheEntry }, - { resultPromise } + { dispatch, getState, getCacheEntry, resultPromise } ) { try { snapshot(getCacheEntry()) @@ -226,8 +224,7 @@ test('mutation: getCacheEntry (success)', async () => { query: () => '/success', async onQuery( arg, - { dispatch, getState, getCacheEntry }, - { resultPromise } + { dispatch, getState, getCacheEntry, resultPromise } ) { try { snapshot(getCacheEntry()) @@ -286,8 +283,7 @@ test('mutation: getCacheEntry (error)', async () => { query: () => '/error', async onQuery( arg, - { dispatch, getState, getCacheEntry }, - { resultPromise } + { dispatch, getState, getCacheEntry, resultPromise } ) { try { snapshot(getCacheEntry()) @@ -346,8 +342,7 @@ test('query: updateCacheEntry', async () => { query: () => '/success', async onQuery( arg, - { dispatch, getState, getCacheEntry, updateCacheEntry }, - { resultPromise } + { dispatch, getState, getCacheEntry, updateCacheEntry, resultPromise } ) { // calling `updateCacheEntry` when there is no data yet should not do anything // but if there is a cache value it will be updated & overwritten by the next succesful result From 1f0b4e993a6dc35abb7555d7a83028b4e81b9309 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Thu, 13 May 2021 13:10:54 +0200 Subject: [PATCH 18/18] docs updates --- docs/api/rtk-query/createApi.md | 87 ++++++++++++++-------- docs/usage/rtk-query/mutations.md | 72 ++++++++++-------- docs/usage/rtk-query/optimistic-updates.md | 44 ++++++++--- docs/usage/rtk-query/queries.md | 32 ++++++-- src/query/core/buildThunks.ts | 2 - src/query/endpointDefinitions.ts | 17 ++--- 6 files changed, 165 insertions(+), 89 deletions(-) diff --git a/docs/api/rtk-query/createApi.md b/docs/api/rtk-query/createApi.md index 268c0998a6..e3d2909c0c 100644 --- a/docs/api/rtk-query/createApi.md +++ b/docs/api/rtk-query/createApi.md @@ -38,7 +38,7 @@ export const { useGetPokemonByNameQuery } = pokemonApi `createApi` accepts a single configuration object parameter with the following options: ```ts no-transpile - baseQuery(args: InternalQueryArgs, api: QueryApi): any; + baseQuery(args: InternalQueryArgs, api: BaseQueryApi): any; endpoints(build: EndpointBuilder): Definitions; tagTypes?: readonly TagTypes[]; reducerPath?: ReducerPath; @@ -106,47 +106,74 @@ export const defaultSerializeQueryArgs: SerializeQueryArgs = ({ }, {}) ``` -- `provides` _(optional)_ +- `providesTags` _(optional)_ [summary](docblock://query/endpointDefinitions.ts?token=QueryExtraOptions.provides) -- `invalidates` _(optional)_ +- `invalidatesTags` _(optional)_ [summary](docblock://query/endpointDefinitions.ts?token=MutationExtraOptions.invalidates) -- `onStart`, `onError` and `onSuccess` _(optional)_ - Available to both [queries](../../usage/rtk-query/queries.md) and [mutations](../../usage/rtk-query/mutations.md) +- `onQuery` _(optional)_ - Available to both [queries](../../usage/rtk-query/queries.md) and [mutations](../../usage/rtk-query/mutations.md) - Can be used in `mutations` for [optimistic updates](../../usage/rtk-query/optimistic-updates.md). - - ```ts title="Mutation lifecycle signatures" - function onStart( + - ```ts title="Mutation onQuery signature" + async function onQuery( arg: QueryArg, - mutationApi: MutationApi - ): void - function onError( - arg: QueryArg, - mutationApi: MutationApi, - error: unknown - ): void - function onSuccess( - arg: QueryArg, - mutationApi: MutationApi, - result: ResultType - ): void + { + dispatch, + getState, + extra, + requestId, + resultPromise, + getCacheEntry, + }: MutationLifecycleApi + ): Promise ``` - - ```ts title="Query lifecycle signatures" - function onStart( + - ```ts title="Query onQuery signature" + async function onQuery( arg: QueryArg, - queryApi: QueryApi - ): void - function onError( + { + dispatch, + getState, + extra, + requestId, + resultPromise, + getCacheEntry, + updateCacheEntry, + }: QueryLifecycleApi + ): Promise + ``` +- `onCacheEntryAdded` _(optional)_ - Available to both [queries](../../usage/rtk-query/queries.md) and [mutations](../../usage/rtk-query/mutations.md) + + - Can be used for TODO + - ```ts title="Mutation onCacheEntryAdded signature" + async function onCacheEntryAdded( arg: QueryArg, - queryApi: QueryApi, - error: unknown - ): void - function onSuccess( + { + dispatch, + getState, + extra, + requestId, + cleanup, + firstValueResolved, + getCacheEntry, + }: MutationCacheLifecycleApi + ): Promise + ``` + - ```ts title="Query onCacheEntryAdded signature" + async function onCacheEntryAdded( arg: QueryArg, - queryApi: QueryApi, - result: ResultType - ): void + { + dispatch, + getState, + extra, + requestId, + cleanup, + firstValueResolved, + getCacheEntry, + updateCacheEntry, + }: QueryCacheLifecycleApi + ): Promise ``` #### How endpoints get used diff --git a/docs/usage/rtk-query/mutations.md b/docs/usage/rtk-query/mutations.md index 72f279049c..b53b7c686c 100644 --- a/docs/usage/rtk-query/mutations.md +++ b/docs/usage/rtk-query/mutations.md @@ -23,23 +23,33 @@ const api = createApi({ }), // Pick out data and prevent nested properties in a hook or selector transformResponse: (response) => response.data, - // onStart, onSuccess, onError are useful for optimistic updates - // The 2nd parameter is the destructured `mutationApi` - onStart( - { id, ...patch }, - { dispatch, getState, extra, requestId, context } - ) {}, - // `result` is the server response - onSuccess({ id }, mutationApi, result) {}, - onError({ id }, { dispatch, getState, extra, requestId, context }) {}, invalidatesTags: ['Post'], + // onQuery is useful for optimistic updates + // The 2nd parameter is the destructured `MutationLifecycleApi` + async onQuery( + arg, + { dispatch, getState, resultPromise, requestId, extra, getCacheEntry } + ) {}, + // The 2nd parameter is the destructured `MutationCacheLifecycleApi` + async onCacheEntryAdded( + arg, + { + dispatch, + getState, + extra, + requestId, + cleanup, + firstValueResolved, + getCacheEntry, + } + ) {}, }), }), }) ``` :::info -Notice the `onStart`, `onSuccess`, `onError` methods? Be sure to check out how they can be used for [optimistic updates](./optimistic-updates) +Notice the `onQuery` method? Be sure to check out how it can be used for [optimistic updates](./optimistic-updates) ::: ### Type interfaces @@ -56,29 +66,29 @@ export type MutationDefinition< type: DefinitionType.mutation invalidatesTags?: ResultDescription providesTags?: never - onStart?(arg: QueryArg, mutationApi: MutationApi): void - onError?( + onQuery?( arg: QueryArg, - mutationApi: MutationApi, - error: unknown, - meta: BaseQueryMeta - ): void - onSuccess?( + { + dispatch, + getState, + resultPromise, + requestId, + extra, + getCacheEntry, + }: MutationLifecycleApi + ): Promise + onCacheEntryAdded?( arg: QueryArg, - mutationApi: MutationApi, - result: ResultType, - meta: BaseQueryMeta | undefined - ): void -} -``` - -```ts title="MutationApi" -export interface MutationApi { - dispatch: ThunkDispatch - getState(): RootState - extra: unknown - requestId: string - context: Context + { + dispatch, + getState, + extra, + requestId, + cleanup, + firstValueResolved, + getCacheEntry, + }: MutationCacheLifecycleApi + ): Promise } ``` diff --git a/docs/usage/rtk-query/optimistic-updates.md b/docs/usage/rtk-query/optimistic-updates.md index 568a2f790f..4d06b85dc1 100644 --- a/docs/usage/rtk-query/optimistic-updates.md +++ b/docs/usage/rtk-query/optimistic-updates.md @@ -11,10 +11,11 @@ When you're performing an update on some data that _already exists_ in the cache The core concepts are: -- in the `onStart` phase of a mutation, you manually set the cached data via `updateQueryResult` -- then, in `onError`, you roll it back via `patchQueryResult`. You don't have to worry about the `onSuccess` lifecycle here. +- when you start a query or mutation, `onQuery` will be executed +- you manually update the cached data by dispatching `api.util.updateQueryResult` +- then, in the case that `promiseResult` rejects, you roll it back via the `.undo` property of the object you got back from the earlier dispatch. -```ts title="Example optimistic update mutation" +```ts title="Example optimistic update mutation (async await)" const api = createApi({ baseQuery, tagTypes: ['Post'], @@ -33,24 +34,43 @@ const api = createApi({ method: 'PATCH', body: patch, }), - onStart({ id, ...patch }, { dispatch, context }) { - // When we start the request, just immediately update the cache - context.undoPost = dispatch( + async onQuery({ id, ...patch }, { dispatch, resultPromise }) { + const patchResult = dispatch( api.util.updateQueryResult('getPost', id, (draft) => { Object.assign(draft, patch) }) - ).inversePatches - }, - onError({ id }, { dispatch, context }) { - // If there is an error, roll it back - dispatch(api.util.patchQueryResult('getPost', id, context.undoPost)) - }, + ) + try { + await resultPromise + } catch { + patchResult.undo() + } + } invalidatesTags: ['Post'], }), }), }) ``` +or, if you prefer the slighty shorter version with `.catch` + +```diff +- async onQuery({ id, ...patch }, { dispatch, resultPromise }) { ++ onQuery({ id, ...patch }, { dispatch, resultPromise }) { + const patchResult = dispatch( + api.util.updateQueryResult('getPost', id, (draft) => { + Object.assign(draft, patch) + }) + ) +- try { +- await resultPromise +- } catch { +- patchResult.undo() +- } ++ resultPromise.catch(patchResult.undo) + } +``` + ### Example [View Example](./examples#react-optimistic-updates) diff --git a/docs/usage/rtk-query/queries.md b/docs/usage/rtk-query/queries.md index c9cb823e30..ee29316354 100644 --- a/docs/usage/rtk-query/queries.md +++ b/docs/usage/rtk-query/queries.md @@ -21,12 +21,34 @@ const api = createApi({ query: (id) => ({ url: `post/${id}` }), // Pick out data and prevent nested properties in a hook or selector transformResponse: (response) => response.data, - // The 2nd parameter is the destructured `queryApi` - onStart(id, { dispatch, getState, extra, requestId, context }) {}, - // `result` is the server response - onSuccess(id, queryApi, result) {}, - onError(id, queryApi) {}, providesTags: (result, error, id) => [{ type: 'Post', id }], + // The 2nd parameter is the destructured `QueryLifecycleApi` + async onQuery( + arg, + { + dispatch, + getState, + extra, + requestId, + resultPromise, + getCacheEntry, + updateCacheEntry, + } + ) {}, + // The 2nd parameter is the destructured `QueryCacheLifecycleApi` + async onCacheEntryAdded( + arg, + { + dispatch, + getState, + extra, + requestId, + cleanup, + firstValueResolved, + getCacheEntry, + updateCacheEntry, + } + ) {}, }), }), }) diff --git a/src/query/core/buildThunks.ts b/src/query/core/buildThunks.ts index ebbcea5bb8..8c5b56b649 100644 --- a/src/query/core/buildThunks.ts +++ b/src/query/core/buildThunks.ts @@ -19,9 +19,7 @@ import { calculateProvidedBy, EndpointDefinition, EndpointDefinitions, - MutationApi, MutationDefinition, - QueryApi, QueryArgFrom, QueryDefinition, ResultTypeFrom, diff --git a/src/query/endpointDefinitions.ts b/src/query/endpointDefinitions.ts index dacb87e1be..4e9033dfc0 100644 --- a/src/query/endpointDefinitions.ts +++ b/src/query/endpointDefinitions.ts @@ -307,14 +307,12 @@ export type EndpointBuilder< * query: (id) => ({ url: `post/${id}` }), * // Pick out data and prevent nested properties in a hook or selector * transformResponse: (response) => response.data, - * // trigger side effects or optimistic updates - * onQuery(id, { dispatch, getState, getCacheEntry }, { resultPromise }) {}, - * // handle subscriptions etc - * onCacheEntryAdded(id, { dispatch, getState, getCacheEntry }, { firstValueResolved, cleanup }) {}, * // `result` is the server response - * onSuccess(id, queryApi, result) {}, - * onError(id, queryApi) {}, * providesTags: (result, error, id) => [{ type: 'Post', id }], + * // trigger side effects or optimistic updates + * onQuery(id, { dispatch, getState, extra, requestId, resultPromise, getCacheEntry, updateCacheEntry }) {}, + * // handle subscriptions etc + * onCacheEntryAdded(id, { dispatch, getState, extra, requestId, cleanup, firstValueResolved, getCacheEntry, updateCacheEntry }) {}, * }), * }), *}); @@ -339,11 +337,12 @@ export type EndpointBuilder< * query: ({ id, ...patch }) => ({ url: `post/${id}`, method: 'PATCH', body: patch }), * // Pick out data and prevent nested properties in a hook or selector * transformResponse: (response) => response.data, + * // `result` is the server response + * invalidatesTags: (result, error, id) => [{ type: 'Post', id }], * // trigger side effects or optimistic updates - * onQuery(id, { dispatch, getState, getCacheEntry }, { resultPromise }) {}, + * onQuery(id, { dispatch, getState, extra, requestId, resultPromise, getCacheEntry }) {}, * // handle subscriptions etc - * onCacheEntryAdded(id, { dispatch, getState, getCacheEntry }, { firstValueResolved, cleanup }) {}, - * invalidatesTags: (result, error, id) => [{ type: 'Post', id }], + * onCacheEntryAdded(id, { dispatch, getState, extra, requestId, cleanup, firstValueResolved, getCacheEntry }) {}, * }), * }), * });