Skip to content

Commit 61996f5

Browse files
fix(react-data-query): Retain queries in cache until GC (#9502)
## Explanation Following this PR, instead of automatically removing queries from the UI query client when a `BaseDataService` instance evicts it from the cache, we retain the query in cache until the `QueryClient` itself cleans it up (via `cacheTime`). This prevents issues with observers in the UI having their cache wiped just because a refetch hasn't happened in a while. Since the UI query client considers the data stale, it may also choose to refetch and repopulate the query in the `BaseDataService` instance before eviction anyways. ## References https://consensyssoftware.atlassian.net/browse/WPC-1168 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes cache lifecycle for all data-service-backed queries and could leave stale UI data visible longer if service eviction was relied on for freshness, though invalidation/hydration paths are unchanged. > > **Overview** > **UI query cache no longer mirrors `BaseDataService` evictions.** When the data service publishes a granular `removed` cache update, `createUIQueryClient` now ignores that event instead of calling `cache.remove`, so subscribed UI observers keep their data until TanStack Query’s own `cacheTime` garbage collection runs. > > Tests replace the old “sync removal on remove event” case with fake-timer coverage: cache stays populated while observers are active even after the service’s long TTL, and entries are cleared only after observers are destroyed and the client `cacheTime` elapses. Changelog records the fix under Unreleased. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a2a6848. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c4c63f4 commit 61996f5

3 files changed

Lines changed: 90 additions & 25 deletions

File tree

packages/react-data-query/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Bump `@metamask/utils` from `^11.9.0` to `^11.11.0` ([#9074](https://github.com/MetaMask/core/pull/9074))
1313
- Make `react-dom` and `react-native` peer dependencies optional ([#9295](https://github.com/MetaMask/core/pull/9295))
1414

15+
### Fixed
16+
17+
- Retain queries in cache until GC ([#9502](https://github.com/MetaMask/core/pull/9502))
18+
1519
## [0.2.1]
1620

1721
### Changed

packages/react-data-query/src/createUIQueryClient.test.ts

Lines changed: 83 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { Messenger } from '@metamask/messenger';
2+
import { Duration, inMilliseconds } from '@metamask/utils';
23
import {
3-
hashQueryKey,
44
InfiniteData,
55
InfiniteQueryObserver,
66
QueryClient,
7+
QueryClientConfig,
78
QueryObserver,
89
} from '@tanstack/query-core';
910

@@ -23,7 +24,7 @@ import { createUIQueryClient } from './createUIQueryClient';
2324

2425
const DATA_SERVICES = ['ExampleDataService'];
2526

26-
function createClients(): {
27+
function createClients(config?: QueryClientConfig): {
2728
service: ExampleDataService;
2829
clientA: QueryClient;
2930
clientB: QueryClient;
@@ -40,8 +41,8 @@ function createClients(): {
4041
>({ namespace: 'ExampleDataService' });
4142
const service = new ExampleDataService(serviceMessenger);
4243

43-
const clientA = createUIQueryClient(DATA_SERVICES, serviceMessenger);
44-
const clientB = createUIQueryClient(DATA_SERVICES, serviceMessenger);
44+
const clientA = createUIQueryClient(DATA_SERVICES, serviceMessenger, config);
45+
const clientB = createUIQueryClient(DATA_SERVICES, serviceMessenger, config);
4546

4647
return { service, clientA, clientB, messenger: serviceMessenger };
4748
}
@@ -67,8 +68,12 @@ describe('createUIQueryClient', () => {
6768
mockTransactionsPage2();
6869
});
6970

71+
afterEach(() => {
72+
jest.useRealTimers();
73+
});
74+
7075
it('proxies requests to the underlying service', async () => {
71-
const { clientA: client } = createClients();
76+
const { clientA: client, service } = createClients();
7277

7378
const result = await client.fetchQuery({
7479
queryKey: getAssetsQueryKey,
@@ -94,10 +99,12 @@ describe('createUIQueryClient', () => {
9499
symbol: 'ETH',
95100
},
96101
]);
102+
103+
service.destroy();
97104
});
98105

99106
it('fetches using observers', async () => {
100-
const { clientA, clientB } = createClients();
107+
const { clientA, clientB, service } = createClients();
101108

102109
const observerA = new QueryObserver(clientA, {
103110
queryKey: getAssetsQueryKey,
@@ -132,10 +139,11 @@ describe('createUIQueryClient', () => {
132139

133140
observerA.destroy();
134141
observerB.destroy();
142+
service.destroy();
135143
});
136144

137145
it('fetches using observers in the same client', async () => {
138-
const { clientA } = createClients();
146+
const { clientA, service } = createClients();
139147

140148
const observerA = new QueryObserver(clientA, {
141149
queryKey: getAssetsQueryKey,
@@ -170,10 +178,11 @@ describe('createUIQueryClient', () => {
170178

171179
observerA.destroy();
172180
observerB.destroy();
181+
service.destroy();
173182
});
174183

175184
it('synchronizes caches after invalidation', async () => {
176-
const { clientA, clientB } = createClients();
185+
const { clientA, clientB, service } = createClients();
177186

178187
const observerA = new QueryObserver(clientA, {
179188
queryKey: getAssetsQueryKey,
@@ -216,10 +225,13 @@ describe('createUIQueryClient', () => {
216225

217226
observerA.destroy();
218227
observerB.destroy();
228+
service.destroy();
219229
});
220230

221-
it('synchronizes cache removal after remove event', async () => {
222-
const { messenger, clientA, clientB } = createClients();
231+
it('does not remove from the cache if observers still are subscribed', async () => {
232+
jest.useFakeTimers();
233+
234+
const { clientA, clientB, service } = createClients();
223235

224236
const observerA = new QueryObserver(clientA, {
225237
queryKey: getAssetsQueryKey,
@@ -231,40 +243,92 @@ describe('createUIQueryClient', () => {
231243

232244
const promiseA = new Promise((resolve) => {
233245
observerA.subscribe((event) => {
234-
if (event.status === 'success') {
246+
if (event.status === 'success' && !event.isFetching) {
235247
resolve(event.data);
236248
}
237249
});
238250
});
239251

240252
const promiseB = new Promise((resolve) => {
241253
observerB.subscribe((event) => {
242-
if (event.status === 'success') {
254+
if (event.status === 'success' && !event.isFetching) {
243255
resolve(event.data);
244256
}
245257
});
246258
});
247259

260+
jest.advanceTimersByTime(0);
261+
248262
await Promise.all([promiseA, promiseB]);
249263

250-
const hash = hashQueryKey(getAssetsQueryKey);
264+
// Advance the full cacheTime of ExampleDataService
265+
jest.advanceTimersByTime(inMilliseconds(1, Duration.Day));
266+
267+
const queryData = clientA.getQueryData(getAssetsQueryKey);
268+
269+
expect(queryData).toBeDefined();
270+
expect(queryData).toStrictEqual(clientB.getQueryData(getAssetsQueryKey));
251271

252-
messenger.publish(`ExampleDataService:cacheUpdated:${hash}`, {
253-
type: 'removed',
254-
state: null,
272+
observerA.destroy();
273+
observerB.destroy();
274+
service.destroy();
275+
});
276+
277+
it('cleans up removed cache entries once all observers are removed', async () => {
278+
jest.useFakeTimers();
279+
280+
const defaultOptions = {
281+
queries: { cacheTime: inMilliseconds(5, Duration.Minute) },
282+
};
283+
284+
const { clientA, clientB, service } = createClients({ defaultOptions });
285+
286+
const observerA = new QueryObserver(clientA, {
287+
queryKey: getAssetsQueryKey,
255288
});
256289

290+
const observerB = new QueryObserver(clientB, {
291+
queryKey: getAssetsQueryKey,
292+
});
293+
294+
const promiseA = new Promise((resolve) => {
295+
observerA.subscribe((event) => {
296+
if (event.status === 'success' && !event.isFetching) {
297+
resolve(event.data);
298+
}
299+
});
300+
});
301+
302+
const promiseB = new Promise((resolve) => {
303+
observerB.subscribe((event) => {
304+
if (event.status === 'success' && !event.isFetching) {
305+
resolve(event.data);
306+
}
307+
});
308+
});
309+
310+
jest.advanceTimersByTime(0);
311+
312+
await Promise.all([promiseA, promiseB]);
313+
314+
jest.advanceTimersByTime(inMilliseconds(1, Duration.Day));
315+
257316
const queryData = clientA.getQueryData(getAssetsQueryKey);
258317

259-
expect(queryData).toBeUndefined();
318+
expect(queryData).toBeDefined();
260319
expect(queryData).toStrictEqual(clientB.getQueryData(getAssetsQueryKey));
261320

262321
observerA.destroy();
263322
observerB.destroy();
323+
324+
jest.advanceTimersByTime(inMilliseconds(5, Duration.Minute));
325+
326+
expect(clientA.getQueryData(getAssetsQueryKey)).toBeUndefined();
327+
service.destroy();
264328
});
265329

266330
it('fetches using paginated observers', async () => {
267-
const { clientA, clientB } = createClients();
331+
const { clientA, clientB, service } = createClients();
268332

269333
const getPreviousPageParam = ({
270334
pageInfo,
@@ -323,6 +387,7 @@ describe('createUIQueryClient', () => {
323387

324388
observerA.destroy();
325389
observerB.destroy();
390+
service.destroy();
326391
});
327392

328393
it('errors if observer attempts to use default query function without a data service', async () => {

packages/react-data-query/src/createUIQueryClient.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,10 @@ export function createUIQueryClient(
112112
payload: DataServiceGranularCacheUpdatedPayload,
113113
): void => {
114114
if (payload.type === 'removed') {
115-
const currentQuery = cache.get(hash);
116-
117-
if (currentQuery) {
118-
cache.remove(currentQuery);
119-
}
120-
} else {
121-
hydrate(client, payload.state);
115+
return;
122116
}
117+
118+
hydrate(client, payload.state);
123119
};
124120

125121
subscriptions.set(hash, cacheListener);

0 commit comments

Comments
 (0)