From 2f2a2112a05f512f95e31c3362aca856fcf07105 Mon Sep 17 00:00:00 2001 From: Matt Sutkowski Date: Mon, 17 Jan 2022 10:42:09 -0800 Subject: [PATCH 1/3] Cleanup polls on unsubscribeQueryResult --- .../src/query/core/buildMiddleware/polling.ts | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/toolkit/src/query/core/buildMiddleware/polling.ts b/packages/toolkit/src/query/core/buildMiddleware/polling.ts index 7f5ac3a918..80f58daa76 100644 --- a/packages/toolkit/src/query/core/buildMiddleware/polling.ts +++ b/packages/toolkit/src/query/core/buildMiddleware/polling.ts @@ -19,10 +19,24 @@ export const build: SubMiddlewareBuilder = ({ timeout?: TimeoutId pollingInterval: number }> = {} + return (next) => (action): any => { const result = next(action) + if (api.internalActions.unsubscribeQueryResult.match(action)) { + const { queryCacheKey } = action.payload + const existingSubscriptionCount = Object.keys( + mwApi.getState()[reducerPath].subscriptions[queryCacheKey] || {} + ).length + + // There are no other components subscribed and sharing a poll for this queryCacheKey, so we can + // safely remove it + if (existingSubscriptionCount === 0) { + cleanupPollForKey(queryCacheKey) + } + } + if (api.internalActions.updateSubscriptionOptions.match(action)) { updatePollingInterval(action.payload, mwApi) } @@ -102,10 +116,7 @@ export const build: SubMiddlewareBuilder = ({ const currentPoll = currentPolls[queryCacheKey] if (!Number.isFinite(lowestPollingInterval)) { - if (currentPoll?.timeout) { - clearTimeout(currentPoll.timeout) - } - delete currentPolls[queryCacheKey] + cleanupPollForKey(queryCacheKey) return } @@ -116,10 +127,15 @@ export const build: SubMiddlewareBuilder = ({ } } + function cleanupPollForKey(key: string) { + const existingPoll = currentPolls[key] + existingPoll?.timeout && clearTimeout(existingPoll.timeout) + delete currentPolls[key] + } + function clearPolls() { - for (const [key, poll] of Object.entries(currentPolls)) { - if (poll?.timeout) clearTimeout(poll.timeout) - delete currentPolls[key] + for (const key of Object.keys(currentPolls)) { + cleanupPollForKey(key) } } } From 226b6a6ae5c5976acbec817748831f55b7517d7d Mon Sep 17 00:00:00 2001 From: Matt Sutkowski Date: Fri, 21 Jan 2022 08:58:22 -0800 Subject: [PATCH 2/3] Add polling tests --- .../src/query/core/buildMiddleware/polling.ts | 18 +-- .../toolkit/src/query/tests/polling.test.tsx | 110 ++++++++++++++++++ 2 files changed, 114 insertions(+), 14 deletions(-) create mode 100644 packages/toolkit/src/query/tests/polling.test.tsx diff --git a/packages/toolkit/src/query/core/buildMiddleware/polling.ts b/packages/toolkit/src/query/core/buildMiddleware/polling.ts index 80f58daa76..a48527efae 100644 --- a/packages/toolkit/src/query/core/buildMiddleware/polling.ts +++ b/packages/toolkit/src/query/core/buildMiddleware/polling.ts @@ -24,20 +24,10 @@ export const build: SubMiddlewareBuilder = ({ (action): any => { const result = next(action) - if (api.internalActions.unsubscribeQueryResult.match(action)) { - const { queryCacheKey } = action.payload - const existingSubscriptionCount = Object.keys( - mwApi.getState()[reducerPath].subscriptions[queryCacheKey] || {} - ).length - - // There are no other components subscribed and sharing a poll for this queryCacheKey, so we can - // safely remove it - if (existingSubscriptionCount === 0) { - cleanupPollForKey(queryCacheKey) - } - } - - if (api.internalActions.updateSubscriptionOptions.match(action)) { + if ( + api.internalActions.updateSubscriptionOptions.match(action) || + api.internalActions.unsubscribeQueryResult.match(action) + ) { updatePollingInterval(action.payload, mwApi) } diff --git a/packages/toolkit/src/query/tests/polling.test.tsx b/packages/toolkit/src/query/tests/polling.test.tsx new file mode 100644 index 0000000000..af60e7c23e --- /dev/null +++ b/packages/toolkit/src/query/tests/polling.test.tsx @@ -0,0 +1,110 @@ +import { createApi } from '@reduxjs/toolkit/query' +import { setupApiStore, waitMs } from './helpers' + +const mockBaseQuery = jest + .fn() + .mockImplementation((args: any) => ({ data: args })) + +const api = createApi({ + baseQuery: mockBaseQuery, + tagTypes: ['Posts'], + endpoints: (build) => ({ + getPosts: build.query({ + query(pageNumber) { + return { url: 'posts', params: pageNumber } + }, + providesTags: ['Posts'], + }), + }), +}) +const { getPosts } = api.endpoints + +const storeRef = setupApiStore(api) + +const getSubscribersForQueryCacheKey = (queryCacheKey: string) => + storeRef.store.getState()[api.reducerPath].subscriptions[queryCacheKey] || {} +const createSubscriptionGetter = (queryCacheKey: string) => () => + getSubscribersForQueryCacheKey(queryCacheKey) + +describe('polling tests', () => { + it('clears intervals when seeing a resetApiState action', async () => { + await storeRef.store.dispatch( + getPosts.initiate(1, { + subscriptionOptions: { pollingInterval: 10 }, + subscribe: true, + }) + ) + + expect(mockBaseQuery).toHaveBeenCalledTimes(1) + + storeRef.store.dispatch(api.util.resetApiState()) + + await waitMs(30) + + expect(mockBaseQuery).toHaveBeenCalledTimes(1) + }) + + it('replaces polling interval when the subscription options are updated', async () => { + const { requestId, queryCacheKey, ...subscription } = + storeRef.store.dispatch( + getPosts.initiate(1, { + subscriptionOptions: { pollingInterval: 10 }, + subscribe: true, + }) + ) + + const getSubs = createSubscriptionGetter(queryCacheKey) + + expect(Object.keys(getSubs())).toHaveLength(1) + expect(getSubs()[requestId].pollingInterval).toBe(10) + + subscription.updateSubscriptionOptions({ pollingInterval: 20 }) + + expect(Object.keys(getSubs())).toHaveLength(1) + expect(getSubs()[requestId].pollingInterval).toBe(20) + }) + + it(`doesn't replace the interval when removing a shared query instance with a poll `, async () => { + const subscriptionOne = storeRef.store.dispatch( + getPosts.initiate(1, { + subscriptionOptions: { pollingInterval: 10 }, + subscribe: true, + }) + ) + + storeRef.store.dispatch( + getPosts.initiate(1, { + subscriptionOptions: { pollingInterval: 10 }, + subscribe: true, + }) + ) + + const getSubs = createSubscriptionGetter(subscriptionOne.queryCacheKey) + + expect(Object.keys(getSubs())).toHaveLength(2) + + subscriptionOne.unsubscribe() + + expect(Object.keys(getSubs())).toHaveLength(1) + }) + + it('uses lowest specified interval when two components are mounted', async () => { + storeRef.store.dispatch( + getPosts.initiate(1, { + subscriptionOptions: { pollingInterval: 30000 }, + subscribe: true, + }) + ) + + storeRef.store.dispatch( + getPosts.initiate(1, { + subscriptionOptions: { pollingInterval: 10 }, + subscribe: true, + }) + ) + + await waitMs(20) + + expect(mockBaseQuery.mock.calls.length).toBeGreaterThanOrEqual(2) + }) +}) From ecdbb0efb652089a14004a426ef0b3cf472f3759 Mon Sep 17 00:00:00 2001 From: Matt Sutkowski Date: Sat, 22 Jan 2022 09:04:19 -0800 Subject: [PATCH 3/3] Address feedback --- packages/toolkit/src/query/core/buildMiddleware/polling.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/toolkit/src/query/core/buildMiddleware/polling.ts b/packages/toolkit/src/query/core/buildMiddleware/polling.ts index a48527efae..a30f3f4fe9 100644 --- a/packages/toolkit/src/query/core/buildMiddleware/polling.ts +++ b/packages/toolkit/src/query/core/buildMiddleware/polling.ts @@ -103,13 +103,13 @@ export const build: SubMiddlewareBuilder = ({ } const lowestPollingInterval = findLowestPollingInterval(subscriptions) - const currentPoll = currentPolls[queryCacheKey] if (!Number.isFinite(lowestPollingInterval)) { cleanupPollForKey(queryCacheKey) return } + const currentPoll = currentPolls[queryCacheKey] const nextPollTimestamp = Date.now() + lowestPollingInterval if (!currentPoll || nextPollTimestamp < currentPoll.nextPollTimestamp) { @@ -119,7 +119,9 @@ export const build: SubMiddlewareBuilder = ({ function cleanupPollForKey(key: string) { const existingPoll = currentPolls[key] - existingPoll?.timeout && clearTimeout(existingPoll.timeout) + if (existingPoll?.timeout) { + clearTimeout(existingPoll.timeout) + } delete currentPolls[key] }