Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(package/react): prevent render loops from SWR fetches #2040

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/poor-mails-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@gqty/react': patch
---

fix(package/react): prevent render loops from SWR fetches
6 changes: 6 additions & 0 deletions packages/gqty/src/Client/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -86,6 +90,8 @@ export const createContext = ({
this.notifyCacheUpdate ||= data === undefined;
}

this.hasCacheMiss ||= cacheNode?.data === undefined;

selectSubscriptions.forEach((fn) => fn(selection, cacheNode));
},
reset() {
Expand Down
33 changes: 26 additions & 7 deletions packages/react/src/query/useQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,19 @@ export const createUseQuery = <TSchema extends BaseGeneratedSchema>(

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) {
Expand Down Expand Up @@ -464,6 +470,19 @@ export const createUseQuery = <TSchema extends BaseGeneratedSchema>(
]
);

// 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.
Expand Down
40 changes: 40 additions & 0 deletions packages/react/test/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Loading