From 67581eec25c719527bb04d3fce4808d80226a717 Mon Sep 17 00:00:00 2001 From: Vicary A Date: Sun, 17 Nov 2024 15:31:11 +0800 Subject: [PATCH] fix(package/react): prevent render loops from SWR fetches --- .changeset/poor-mails-shave.md | 5 ++++ packages/gqty/src/Client/context.ts | 6 ++++ packages/react/src/query/useQuery.ts | 33 +++++++++++++++++----- packages/react/test/useQuery.test.tsx | 40 +++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 .changeset/poor-mails-shave.md diff --git a/.changeset/poor-mails-shave.md b/.changeset/poor-mails-shave.md new file mode 100644 index 000000000..18bfdf1da --- /dev/null +++ b/.changeset/poor-mails-shave.md @@ -0,0 +1,5 @@ +--- +'@gqty/react': patch +--- + +fix(package/react): prevent render loops from SWR fetches diff --git a/packages/gqty/src/Client/context.ts b/packages/gqty/src/Client/context.ts index b0946c7df..adbd6eb11 100644 --- a/packages/gqty/src/Client/context.ts +++ b/packages/gqty/src/Client/context.ts @@ -22,6 +22,10 @@ export type SchemaContext< notifyCacheUpdate: boolean; shouldFetch: boolean; hasCacheHit: boolean; + /** + * Useful for identifying SWR fetches where + * `shouldFetch` is true and `hasCacheHit` is false. + */ hasCacheMiss: boolean; }; @@ -86,6 +90,8 @@ export const createContext = ({ this.notifyCacheUpdate ||= data === undefined; } + this.hasCacheMiss ||= cacheNode?.data === undefined; + selectSubscriptions.forEach((fn) => fn(selection, cacheNode)); }, reset() { diff --git a/packages/react/src/query/useQuery.ts b/packages/react/src/query/useQuery.ts index a123d90c4..a72a8f2f2 100644 --- a/packages/react/src/query/useQuery.ts +++ b/packages/react/src/query/useQuery.ts @@ -344,13 +344,19 @@ export const createUseQuery = ( if (options?.ignoreCache === true) { context.shouldFetch = true; - } - // Soft-refetches here may not know if the WeakRefs in the cache is - // already garbage collected. Running the selections again to update - // the context with the latest cache freshness, this will push back the - // garbage collection if the specific implementation has LRU components. - else if (!options?.skipPrepass && isFinite(client.cache.maxAge)) { - prepass(accessor, selections); + } else { + // Soft-refetches here may not know if the WeakRefs in the cache is + // already garbage collected. Running the selections again to update + // the context with the latest cache freshness, this will push back the + // garbage collection if the specific implementation has LRU components. + if (!options?.skipPrepass && isFinite(client.cache.maxAge)) { + prepass(accessor, selections); + } + + // Skip SWR fetches to prevent render-fetch loops. + if (renderSession.get('postFetch') && !context.hasCacheMiss) { + context.shouldFetch = false; + } } if (!context.shouldFetch) { @@ -464,6 +470,19 @@ export const createUseQuery = ( ] ); + // Foolproof check on first render only when initialLoadingState is enabled, + // because it's confusing for this particular use case. + useEffect(() => { + if (initialLoadingState && selections.size === 0) { + if (process.env.NODE_ENV !== 'production') { + console.warn( + '[GQty] No selections found, ' + + 'this is rarely expected with initialLoadingState.' + ); + } + } + }, []); + // context.shouldFetch only changes during component render, which happens // after this hook is called. A useEffect hook that runs every render // triggers a post-render check. diff --git a/packages/react/test/useQuery.test.tsx b/packages/react/test/useQuery.test.tsx index b3721bc8a..c439a8285 100644 --- a/packages/react/test/useQuery.test.tsx +++ b/packages/react/test/useQuery.test.tsx @@ -26,6 +26,46 @@ describe('useQuery', () => { expect(results).toMatchObject([true, true, false]); }); + + it('should correctly update initial state with a valid cache', async () => { + const { useQuery } = await createReactTestClient( + undefined, + undefined, + undefined, + { + cache: new Cache( + { + query: { + dogs: [{ id: 1, name: 'a' }], + }, + }, + { maxAge: Infinity } + ), + } + ); + + const results: boolean[] = []; + + const { result } = renderHook(() => { + const query = useQuery({ + initialLoadingState: true, + }); + + results.push(query.$state.isLoading); + + query.dogs.map((dog) => ({ ...dog })); + + return query.$state.isLoading; + }); + + await waitFor(() => expect(result.current).toBe(false)); + + expect(results).toMatchObject([ + true, // initial + true, // fetch + false, // response + ]); + }); }); test('should fetch without suspense', async () => {