Add a new `useLoadableQuery` hook #11300

jerelmiller · 2023-10-17T18:19:59Z

Introduces a new useLoadableQuery hook. This hook works similarly to useBackgroundQuery in that it returns a queryRef that can be used to suspend a query via useReadQuery. Its goal is to provide a more ergonomic way to load the query during a user interaction (for example when wanting to preload some data) that would otherwise be clunky with useBackgroundQuery.

function App() {
  // `queryRef` is `null` until `loadQuery` is called the first time
  const [loadQuery, queryRef, { refetch, fetchMore, reset }] = useLoadableQuery(query, options)

  return (
    <>
      <button onClick={() => loadQuery(variables)}>Load query</button>
      <button onClick={() => reset()}>Reset query</button>
      <Suspense fallback={<SuspenseFallback />}>
        {queryRef && <Child queryRef={queryRef} />}
      </Suspense>
    </>
  );
}

function Child({ queryRef }) {
  const { data } = useReadQuery(queryRef)

  // ...
}

changeset-bot · 2023-10-17T18:20:04Z

🦋 Changeset detected

Latest commit: c20e373

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	Minor

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

github-actions · 2023-10-17T18:21:51Z

size-limit report 📦

Path	Size
dist/apollo-client.min.cjs	37.7 KB (+1.16% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs"	44.16 KB (+1.01% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production)	42.64 KB (+1.06% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js"	32.78 KB (+0.01% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production)	31.43 KB (+0.01% 🔺)
import { ApolloProvider } from "dist/react/index.js"	1.28 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production)	1.26 KB (0%)
import { useQuery } from "dist/react/index.js"	4.34 KB (0%)
import { useQuery } from "dist/react/index.js" (production)	4.16 KB (-0.03% 🔽)
import { useLazyQuery } from "dist/react/index.js"	4.65 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production)	4.47 KB (-0.03% 🔽)
import { useMutation } from "dist/react/index.js"	2.61 KB (0%)
import { useMutation } from "dist/react/index.js" (production)	2.59 KB (-0.04% 🔽)
import { useSubscription } from "dist/react/index.js"	2.29 KB (0%)
import { useSubscription } from "dist/react/index.js" (production)	2.25 KB (-0.05% 🔽)
import { useSuspenseQuery } from "dist/react/index.js"	4.33 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production)	3.79 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js"	3.82 KB (-0.03% 🔽)
import { useBackgroundQuery } from "dist/react/index.js" (production)	3.27 KB (0%)
import { useReadQuery } from "dist/react/index.js"	3.05 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production)	3 KB (-0.13% 🔽)
import { useFragment } from "dist/react/index.js"	2.15 KB (0%)
import { useFragment } from "dist/react/index.js" (production)	2.1 KB (0%)
import { useLoadableQuery } from "dist/react/index.js"	4.09 KB (+100% 🔺)
import { useLoadableQuery } from "dist/react/index.js" (production)	3.53 KB (+100% 🔺)

jerelmiller · 2023-11-10T19:51:47Z

/release:pr

github-actions · 2023-11-10T19:55:23Z

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

src/testing/matchers/ProfiledComponent.ts

jerelmiller · 2023-11-11T00:41:06Z

/release:pr

github-actions · 2023-11-11T00:42:16Z

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

jerelmiller · 2023-11-11T04:22:41Z

I think this hook should also include a reset method that resets the queryRef back to null and disposes of the current query ref. Depending on code review timing, I may do this as a followup PR.

phryneas

This looks very good!

I've left some small nitpicks/ideas, but nothing major :)

src/testing/matchers/index.d.ts

src/testing/matchers/ProfiledComponent.ts

src/react/types/types.ts

src/react/hooks/useLoadableQuery.ts

phryneas · 2023-11-13T15:09:43Z

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

+    data: { hello: "from cache" },
+    networkStatus: NetworkStatus.ready,
+    error: undefined,
+  });


Maybe add something like expect(ReadQueryHook).not.toRerender() to make sure nothing else will come in after?

phryneas · 2023-11-13T16:26:20Z

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

+  expect(SuspenseFallback).not.toHaveRendered();
+});
+
+it('suspends and does not use partial data when changing variables and using a "cache-first" fetch policy with returnPartialData', async () => {


Suggested change

it('suspends and does not use partial data when changing variables and using a "cache-first" fetch policy with returnPartialData', async () => {

it('suspends and does not use empty partial data when changing variables and using a "cache-first" fetch policy with returnPartialData', async () => {

this confused me a bit at first, but you mean it won't use a non-existing/empty entry as "partial data", right?

Ya this could use some clarifying behavior here. The idea with this test is that when returnPartialData is true and I have partial data for 1 set of variables, it didn't try and reuse that partial data for a different set of variables. It should start with a clean slate and suspend to fetch the entire data set.

Any way you can get that in there? It makes sense now, but didn't get too obvious from the description, or the test in combination with the description. Could also just be a long comment.

Yes! Updated in b84f0d5. Hope this helps clear things up :)

phryneas · 2023-11-14T10:34:18Z

src/react/hooks/useLoadableQuery.ts

+    [queryRef]
+  );
+
+  const loadQuery: LoadQuery<TVariables> = React.useCallback(


Crazy thought:

Could we maybe prevent this function from being executed before the first render committed by throwing an exception?

It might prevent some abuse scenarios where other hooks might be better suited 🤔

I like that idea. I believe Relay does something similar with its useQueryLoader hook where it prevents you from calling its function during any render, so its definitely a pattern out there in the wild today.

Added in 9a6c748. Fair warning, it might be a tad controversial. BUT Relay has something similar, so I felt it was ok.

… specifying used options

…sposes of it

jerelmiller · 2023-11-28T05:24:44Z

@phryneas this should be fully updated with the updated profiler API and is ready for another look!

jerelmiller · 2023-11-28T05:54:50Z

/release:pr

github-actions · 2023-11-28T05:58:16Z

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

src/testing/internal/profile/profile.tsx

phryneas

I'm through all the files, but only halfway through the tests.
I'm already submitting my accumulated comments since we have meetings coming up now.

phryneas · 2023-11-28T10:40:28Z

src/react/hooks/useLoadableQuery.ts

+  LoadQueryFunction<TVariables>,
+  QueryReference<TData> | null,
+  {
+    fetchMore: FetchMoreFunction<TData, TVariables>;


Suggested change

fetchMore: FetchMoreFunction<TData, TVariables>;

/** {@inheritDoc @apollo/client!ObservableQuery#fetchMore:member(1)} */

fetchMore: FetchMoreFunction<TData, TVariables>;

fetchMore actually doesn't have a comment in ObservableQuery yet, so this might be a good time to add one? ^^

I'm going to hold off on this update for a followup PR. I'd like to hit useSuspenseQuery and useBackgroundQuery as well with this change. This allows me to keep this PR focused on useLoadableQuery as much as possible.

phryneas · 2023-11-28T10:41:55Z

src/react/hooks/useLoadableQuery.ts

+  QueryReference<TData> | null,
+  {
+    fetchMore: FetchMoreFunction<TData, TVariables>;
+    refetch: RefetchFunction<TData, TVariables>;


Suggested change

refetch: RefetchFunction<TData, TVariables>;

/** {@inheritDoc @apollo/client!ObservableQuery#refetch:member(1)} */

refetch: RefetchFunction<TData, TVariables>;

The refetch comment of ObservableQuery doesn't read perfect for the hooks, but it might at least be a starting point until we have some central interface with all the hook options to pick from
(it reads:)

/**

Update the variables of this observable query, and fetch the new results.

This method should be preferred over setVariables in most use cases.

@param variables: The new set of variables. If there are missing variables,

the previous values of those variables will be used.
*/

Love it! Will get this in a followup PR to ensure useBackgroundQuery and useSuspenseQuery can benefit as well.

src/react/hooks/internal/useRenderGuard.ts

src/react/hooks/useLoadableQuery.ts

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

phryneas · 2023-11-28T12:08:19Z

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

+    // Resetting the result allows us to detect when ReadQueryHook is unmounted
+    // since it won't render and overwrite the `null`
+    Profiler.mergeSnapshot({ result: null });
+


Suggested change

// Resetting the result allows us to detect when ReadQueryHook is unmounted

// since it won't render and overwrite the `null`

Profiler.mergeSnapshot({ result: null });

Profiler.mergeSnapshot({ queryRef });

What do you think about tracking the queryRef here instead?

Also, this makes me think... Maybe we should also track unmountedComponents or internally track which components are currently mounted and which are not, so we can make assumptions on that?
This could happen internally in useTrackRender

const { tree } = await Profiler.takeRender(); expect(Component).toBeMountedIn(tree) expect(Component).not.toBeMountedIn(tree) expect("ComponentName").toBeMountedIn(tree) expect("ComponentName").not.toBeMountedIn(tree)

I was thinking the same. I hit this case at the end of the day yesterday and figured I'd revisit today. It's not obvious when something has unmounted so something like this would be much nicer. I love the matcher you have here as well so I'm borrowing that.

I'm going to do this in a separate PR so we can provide some good discussion if need be and move this PR forward.

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

…ractive-query

jerelmiller force-pushed the jerel/use-interactive-query branch from 916b4f3 to 94a424f Compare October 17, 2023 18:23

jerelmiller added this to the Release 3.9 milestone Nov 7, 2023

This comment was marked as resolved.

Sign in to view

jerelmiller removed this from the Release 3.9 milestone Nov 8, 2023

jerelmiller changed the title ~~[WIP] Add a new useInteractiveQuery hook~~ Add a new useInteractiveQuery hook Nov 10, 2023

jerelmiller linked an issue Nov 10, 2023 that may be closed by this pull request

Build a new useLoadableQuery hook #11349

Closed

jerelmiller changed the title ~~Add a new useInteractiveQuery hook~~ Add a new useLoadableQuery hook Nov 10, 2023

jerelmiller requested review from phryneas and alessbell November 10, 2023 19:50

jerelmiller marked this pull request as ready for review November 10, 2023 19:51

jerelmiller force-pushed the jerel/use-interactive-query branch from 4f6b316 to 5678b88 Compare November 10, 2023 22:46

jerelmiller commented Nov 11, 2023

View reviewed changes

src/testing/matchers/ProfiledComponent.ts Outdated Show resolved Hide resolved

phryneas reviewed Nov 14, 2023

View reviewed changes

jerelmiller mentioned this pull request Nov 21, 2023

Minor adjustments to profiler #11376

Merged

jerelmiller added 9 commits November 21, 2023 12:18

Get bones of useInteractiveQuery in place

c735af0

Disable useInteractiveQuery tests with React 17

3034600

Use separate type imports for useInteractiveQuery

bf591e0

Get first test passing with profiler

2a9a14f

Export RefetchWritePolicy type from core/index

8d354e4

Simplify InteractiveQueryOptions type by flattening the type and only…

7e50bf8

… specifying used options

Add comment to loadQuery and add label to variadic arg

48c4a2f

Add proper type tests for useInteractiveQuery

d1fbedf

Pull tests out to top-level

88ba71e

jerelmiller added 2 commits November 27, 2023 18:09

Minor tweak to error message for loadQuery function

fe2cf17

Prevent act warnings when using Profiler.takeRender

383743d

jerelmiller force-pushed the jerel/use-interactive-query branch from af0234b to 383743d Compare November 28, 2023 02:22

jerelmiller added 5 commits November 27, 2023 22:00

Add a reset method to loadQuery that sets the queryRef to null and di…

1a8de39

…sposes of it

Update api report

b7c2dfc

Update size limits

2dc9c8e

Update the changeset to match the updated API

01d6696

Update matcher to allow ProfiledComponent as acceptable value

5c76e4f

jerelmiller force-pushed the jerel/use-interactive-query branch from 496ccfe to 5c76e4f Compare November 28, 2023 05:22

phryneas reviewed Nov 28, 2023

View reviewed changes

src/testing/internal/profile/profile.tsx Outdated Show resolved Hide resolved

phryneas reviewed Nov 28, 2023

View reviewed changes

jerelmiller added 10 commits November 28, 2023 12:30

Pass Profiler directly as the wrapper

8101e02

Return function that returns boolean for useRenderGuard

ea821a9

Create reusable helper to temp disable act warnings

2e72d53

Rename useTrackRender to useTrackRenders

3c9d236

Use Profiler as wrapper in missed test

7226acd

Order takeRender calls in tests in a more logical order

935ff7c

Remove hardcoded size-limit in size-limit.cjs

814ae28

Merge remote-tracking branch 'origin/release-3.9' into jerel/use-inte…

5891a06

…ractive-query

Fix syntax error in changeset

3c8113d

Remove unused generic arg on createProfiler

c20e373

jerelmiller merged commit a815873 into release-3.9 Nov 28, 2023
30 checks passed

jerelmiller deleted the jerel/use-interactive-query branch November 28, 2023 23:44

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

Version Packages (alpha) #11353

Merged

This was referenced Nov 29, 2023

Add tsdoc for refetch, fetchMore and other returned values for suspense hooks #11394

Closed

What is the useLazyQuery alternative to useSuspense and useBackgroundQuery? apollographql/apollo-client-nextjs#150

Closed

github-actions bot locked as resolved and limited conversation to collaborators Dec 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a new `useLoadableQuery` hook #11300

Add a new `useLoadableQuery` hook #11300

jerelmiller commented Oct 17, 2023 •

edited

Loading

changeset-bot bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

This comment was marked as resolved.

jerelmiller commented Nov 10, 2023

github-actions bot commented Nov 10, 2023

jerelmiller commented Nov 11, 2023

github-actions bot commented Nov 11, 2023

jerelmiller commented Nov 11, 2023

phryneas left a comment

phryneas Nov 13, 2023

phryneas Nov 13, 2023

jerelmiller Nov 20, 2023

phryneas Nov 21, 2023

jerelmiller Nov 21, 2023

phryneas Nov 14, 2023

jerelmiller Nov 20, 2023

jerelmiller Nov 21, 2023

jerelmiller commented Nov 28, 2023

jerelmiller commented Nov 28, 2023

github-actions bot commented Nov 28, 2023

phryneas left a comment

phryneas Nov 28, 2023

jerelmiller Nov 28, 2023

phryneas Nov 28, 2023

jerelmiller Nov 28, 2023

phryneas Nov 28, 2023

jerelmiller Nov 28, 2023

jerelmiller Nov 28, 2023

	it('suspends and does not use partial data when changing variables and using a "cache-first" fetch policy with returnPartialData', async () => {
	it('suspends and does not use empty partial data when changing variables and using a "cache-first" fetch policy with returnPartialData', async () => {

	fetchMore: FetchMoreFunction<TData, TVariables>;
	/** {@inheritDoc @apollo/client!ObservableQuery#fetchMore:member(1)} */
	fetchMore: FetchMoreFunction<TData, TVariables>;

	refetch: RefetchFunction<TData, TVariables>;
	/** {@inheritDoc @apollo/client!ObservableQuery#refetch:member(1)} */
	refetch: RefetchFunction<TData, TVariables>;

Add a new useLoadableQuery hook #11300

Add a new useLoadableQuery hook #11300

Conversation

jerelmiller commented Oct 17, 2023 • edited Loading

changeset-bot bot commented Oct 17, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Oct 17, 2023 • edited Loading

size-limit report 📦

This comment was marked as resolved.

jerelmiller commented Nov 10, 2023

github-actions bot commented Nov 10, 2023

jerelmiller commented Nov 11, 2023

github-actions bot commented Nov 11, 2023

jerelmiller commented Nov 11, 2023

phryneas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerelmiller commented Nov 28, 2023

jerelmiller commented Nov 28, 2023

github-actions bot commented Nov 28, 2023

phryneas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add a new `useLoadableQuery` hook #11300

Add a new `useLoadableQuery` hook #11300

jerelmiller commented Oct 17, 2023 •

edited

Loading

changeset-bot bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading