Adjust relative timing of `useFragment` and `useQuery` #11083

phryneas · 2023-07-19T15:52:49Z

Ensures that useQuery triggers rerenders faster or at the same time as useFragment.

Before this change, useQuery used a useState setter callback to trigger a
rerender instead of the useSyncExternalState forceRender callback.
Due to a bug in React (facebook/react#25191), these renders
are not batched together.
That could lead to situations where a child accessing a fragment that was evicted
from the cache would rerender (without data) before a parent useQuery component
had a chance to unmount the child.
After this change, useQuery and useFragment still don't rerender exactly at the same moment
in time - but it is guaranteed that useQuery will always rerender before useFragment will,
which should fix most problems.

I know, it's late for this, but this is the last moment in time we have to fix useFragment before it gets stable - and we should not release it and then immediately change it's timing.

Checklist:

If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
Make sure all of the significant new logic is covered by tests

changeset-bot · 2023-07-19T15:52:52Z

🦋 Changeset detected

Latest commit: d5db4c4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@apollo/client	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

phryneas · 2023-07-19T15:52:58Z

/release:pr

github-actions · 2023-07-19T15:54:26Z

A new release has been made for this PR. You can install it with npm i @apollo/client@0.0.0-pr-11083-20230719155342.

github-actions · 2023-07-19T15:54:39Z

size-limit report 📦

Path	Size
dist/apollo-client.min.cjs	37.16 KB (+0.07% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs"	43.73 KB (+0.09% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production)	42.33 KB (+0.09% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js"	32.39 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production)	31.22 KB (0%)
import { ApolloProvider } from "dist/react/index.js"	1.21 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production)	1.2 KB (0%)
import { useQuery } from "dist/react/index.js"	4.29 KB (+0.67% 🔺)
import { useQuery } from "dist/react/index.js" (production)	4.1 KB (+0.58% 🔺)
import { useLazyQuery } from "dist/react/index.js"	4.6 KB (+0.47% 🔺)
import { useLazyQuery } from "dist/react/index.js" (production)	4.42 KB (+0.61% 🔺)
import { useMutation } from "dist/react/index.js"	2.53 KB (0%)
import { useMutation } from "dist/react/index.js" (production)	2.51 KB (0%)
import { useSubscription } from "dist/react/index.js"	2.24 KB (0%)
import { useSubscription } from "dist/react/index.js" (production)	2.2 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js"	4.71 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production)	4.14 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js"	4.2 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production)	3.63 KB (0%)
import { useReadQuery } from "dist/react/index.js"	2.69 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production)	2.67 KB (0%)
import { useFragment } from "dist/react/index.js"	2.08 KB (+1.39% 🔺)
import { useFragment } from "dist/react/index.js" (production)	2.03 KB (+1.42% 🔺)

phryneas · 2023-07-24T14:42:19Z

src/react/hoc/__tests__/queries/lifecycle.test.tsx

    await waitFor(() => {
-      if (IS_REACT_18) {
-        expect(count).toBe(3)
-      } else {
-        expect(count).toBe(12)
+      if (testFailures.length > 0) {
+        throw testFailures[0];
      }
+      expect(count).toBe(12)
    });


A lot of tests had "just 3 renders in React 18" because during the tests, some expect statement failed.
That would be swallowed by React and not propagate to jest - but interrupt the React renders for good. The test would just stop going, but not really test anything.

I've changed a lot of tests to this pattern of atestFailures array to collect errors during rendering, and then checking that inside the waitFor.
Checking it after the waitFor would be more optimal, but if anything interrupted the counting from happening, the test would just time out without ever reporting why. This way the test times out, but then errors with the first in-component error or failed expect.

There is a ton more of those tests with a render behaviour that seems to have gotten problematic with React 18 (and we will have to fix those!) - but so far I only fixed those tests that were somehow affected by the timing changes.

…-timing

phryneas · 2023-07-24T15:24:50Z

src/react/hooks/useLazyQuery.ts

@@ -68,7 +68,7 @@ export function useLazyQuery<TData = any, TVariables extends OperationVariables
        if (!execOptionsRef.current) {
          execOptionsRef.current = Object.create(null);
          // Only the first time populating execOptionsRef.current matters here.
-          internalState.forceUpdate();
+          internalState.forceUpdateState();


This is the one situation where we want to always call the useState setter update - at this point in time, the useSyncExternalStore result has not changed from the last render, and thus React would skip the rerender, which in some situations is necessary.

src/react/hooks/useQuery.ts

phryneas · 2023-07-25T11:14:14Z

/release:pr

github-actions · 2023-07-25T11:15:40Z

A new release has been made for this PR. You can install it with npm i @apollo/client@0.0.0-pr-11083-20230725111454.

phryneas · 2023-07-26T13:33:30Z

Just to add some context here, to better understand what's going on:

`useQuery`

InMemoryCache.evict
InMemoryCache.broadcastWatch
QueryInfo.setDiff
setTimeout(QueryInfo.notify)
listener registered via QueryInfo.setObservableQuery: reobserveCacheFirst(ObservableQuery)
ObservableQuery.reobserveAsConcast
concast -> observer.next
ObservableQuery.reportResult
iterateObserversSafely -> next
useQuery onNext

`useFragment`

InMemoryCache.evict
InMemoryCache.broadcastWatch
callback in useFragment

So it's probably impossible to get this behavior really "synchronous" if we don't want to stuff an ObservableQuery into useFragment just for the fun of it - and even then, stuff could really get out of sync.

alessbell

After talking through the details of this PR with Lenz, this seems like a solid approach to addressing the useFragment timing issue with useQuery given some larger constraints 🙌 🎉

One notable fact I learned in the course of our discussion: https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#reasons_for_delays_longer_than_specified. Thanks for sharing your very thorough deep dive here, @phryneas!

jerelmiller

This stuff all looks great! Thanks for digging into this so far. I've not nothing else that @alessbell hasn't already mentioned. Thanks!

.changeset/honest-ads-act.md

jerelmiller · 2023-07-27T22:25:13Z

src/react/hoc/__tests__/mutations/recycled-queries.test.tsx

@@ -11,8 +11,6 @@ import { mockSingleLink } from '../../../../testing';
 import { graphql } from '../../graphql';
 import { ChildProps } from '../../types';

-const IS_REACT_18 = React.version.startsWith('18')


🎉 this PR is worth it for this change alone 🙌

src/react/hooks/__tests__/useFragment.test.tsx

Co-authored-by: Jerel Miller <jerelmiller@gmail.com>

netlify · 2023-07-28T07:28:28Z

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	`d5db4c4`
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/64c36fa514f7e6000828a235
😎 Deploy Preview	https://deploy-preview-11083--apollo-client-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

…-timing

phryneas added 2 commits July 19, 2023 15:58

work on useFragment timing

11810dc

changeset

473476f

phryneas linked an issue Jul 20, 2023 that may be closed by this pull request

useFragment returns data {} after entity was evicted #11039

Closed

phryneas added 2 commits July 24, 2023 15:56

all tests green

e60068e

remove most IS_REACT_18 checks

e971a18

phryneas commented Jul 24, 2023

View reviewed changes

phryneas added 2 commits July 24, 2023 16:43

Merge remote-tracking branch 'origin/release-3.8' into pr/useFragment…

6b409ff

…-timing

changeset

2dde3e4

phryneas changed the title ~~[draft] use fragment timing~~ Adjust relative timing of useFragment and useQuery Jul 24, 2023

phryneas requested review from alessbell and jerelmiller July 24, 2023 15:10

phryneas marked this pull request as ready for review July 24, 2023 15:11

phryneas commented Jul 24, 2023

View reviewed changes

src/react/hooks/useQuery.ts Show resolved Hide resolved

phryneas commented Jul 24, 2023

View reviewed changes

src/react/hooks/useQuery.ts Outdated Show resolved Hide resolved

Update src/react/hooks/useQuery.ts

30699cc

phryneas self-assigned this Jul 27, 2023

alessbell approved these changes Jul 27, 2023

View reviewed changes

jerelmiller approved these changes Jul 27, 2023

View reviewed changes

Update .changeset/honest-ads-act.md

1643940

Co-authored-by: Jerel Miller <jerelmiller@gmail.com>

phryneas added 2 commits July 28, 2023 09:32

feedback from code review

4594974

Merge remote-tracking branch 'origin/release-3.8' into pr/useFragment…

d5db4c4

…-timing

phryneas merged commit f766e83 into release-3.8 Jul 28, 2023
8 checks passed

github-actions bot mentioned this pull request Jul 28, 2023

Version Packages (rc) #11088

Merged

jerelmiller deleted the pr/useFragment-timing branch July 28, 2023 07:43

This was referenced Aug 2, 2023

Version Packages (alpha) #11122

Closed

Version Packages #11126

Merged

Version Packages (alpha) #11127

Closed

jerelmiller mentioned this pull request Aug 23, 2023

useQuery loading flag get updated to false before onCompleted function has been called #11161

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjust relative timing of `useFragment` and `useQuery` #11083

Adjust relative timing of `useFragment` and `useQuery` #11083

phryneas commented Jul 19, 2023 •

edited

Loading

changeset-bot bot commented Jul 19, 2023 •

edited

Loading

phryneas commented Jul 19, 2023

github-actions bot commented Jul 19, 2023

github-actions bot commented Jul 19, 2023 •

edited

Loading

phryneas Jul 24, 2023 •

edited

Loading

phryneas Jul 24, 2023

phryneas commented Jul 25, 2023

github-actions bot commented Jul 25, 2023

phryneas commented Jul 26, 2023

alessbell left a comment

jerelmiller left a comment

jerelmiller Jul 27, 2023

netlify bot commented Jul 28, 2023 •

edited

Loading

Adjust relative timing of useFragment and useQuery #11083

Adjust relative timing of useFragment and useQuery #11083

Conversation

phryneas commented Jul 19, 2023 • edited Loading

Checklist:

changeset-bot bot commented Jul 19, 2023 • edited Loading

🦋 Changeset detected

phryneas commented Jul 19, 2023

github-actions bot commented Jul 19, 2023

github-actions bot commented Jul 19, 2023 • edited Loading

size-limit report 📦

phryneas Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

phryneas Jul 24, 2023

Choose a reason for hiding this comment

phryneas commented Jul 25, 2023

github-actions bot commented Jul 25, 2023

phryneas commented Jul 26, 2023

useQuery

useFragment

alessbell left a comment

Choose a reason for hiding this comment

jerelmiller left a comment

Choose a reason for hiding this comment

jerelmiller Jul 27, 2023

Choose a reason for hiding this comment

netlify bot commented Jul 28, 2023 • edited Loading

✅ Deploy Preview for apollo-client-docs ready!

Adjust relative timing of `useFragment` and `useQuery` #11083

Adjust relative timing of `useFragment` and `useQuery` #11083

phryneas commented Jul 19, 2023 •

edited

Loading

changeset-bot bot commented Jul 19, 2023 •

edited

Loading

github-actions bot commented Jul 19, 2023 •

edited

Loading

phryneas Jul 24, 2023 •

edited

Loading

`useQuery`

`useFragment`

netlify bot commented Jul 28, 2023 •

edited

Loading