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

How to get loading state when using useDeferredValue with useReadQuery? #12227

Closed
Temzasse opened this issue Dec 16, 2024 · 11 comments
Closed

How to get loading state when using useDeferredValue with useReadQuery? #12227

Temzasse opened this issue Dec 16, 2024 · 11 comments

Comments

@Temzasse
Copy link

Temzasse commented Dec 16, 2024

Issue Description

Hi 👋🏻

This is a follow-up issue related to a discussion on Bluesky where I tagged @phryneas and asked a React Suspense related question.

Basically the original question was: how do we stop React from showing the Suspense fallback when using preloadQuery + useReadQuery in a situation where using startTransition is not feasible?

During the discussion various methods were suggested but in the end useDeferredValue was suggested as a potential solution, for example:

const deferredQueryRef = useDeferredValue(queryRef); // get `queryRef` from `loader`
const { data } = useReadQuery(deferredQueryRef);

With this approach there is one problem tho, how do we get the loading state after the initial suspension?

The context for this issue is that with Tanstack Router it seems to be impossible/impractical to use startTransition in conjunction with useNavigate's navigate function (to update the search params) due to the way Tanstack Router uses useSyncExternalStore internally. See the following quote from one of the Tanstack maintainers:

Dominik: bottom line is that transitions around navigate "work", but are short-circuited by useSyncExternalStore used by the router :/. It's a trade-off. FWIW, useDeferredValue on the QueryKey is what works for react-query.

We tried using preloadQuery and useReadQuery in the following way with Tanstack Router's loader method which receives the query variables from the search params managed by Tanstack Router:

import { useTransition, type FormEvent } from "react";
import { gql, useReadQuery } from "@apollo/client";
import { createFileRoute, useNavigate } from "@tanstack/react-router";

// https://countries.trevorblades.com
const GET_LANGUAGE = gql`
  query Language($code: ID!) {
    language(code: $code) {
      code
      name
    }
  }
`;

type RouteSearch = { code: string };
type Result = { language: { name: string } | null };
type Variables = { code: string };

export const Route = createFileRoute("/example")({
  component: RouteComponent,
  errorComponent: ErrorComponent,
  pendingComponent: PendingComponent,
  validateSearch: (search): RouteSearch => ({
    code: (search.code as string | undefined) || "en",
  }),
  loaderDeps: ({ search }) => ({
    code: search.code,
  }),
  loader: async ({ context, deps }) => {
    return {
      queryRef: context.preloadQuery<Result, Variables>(GET_LANGUAGE, {
        variables: { code: deps.code },
        fetchPolicy: "network-only",
      }),
    };
  },
});

function RouteComponent() {
  const search = Route.useSearch();
  const navigate = useNavigate({ from: Route.fullPath });
  const [isPending, startTransition] = useTransition();
  const { queryRef } = Route.useLoaderData();
  const { data } = useReadQuery(queryRef);

  function handleSubmit(e: FormEvent<HTMLFormElement>) {
    e.preventDefault();

    const data = new FormData(e.currentTarget);
    const code = data.get("code") as string;

    if (code.length !== 2) return;

    /**
     * --------------------------- ⚠️ Apollo issue ⚠️ --------------------------
     * This should stop React from showing Suspense fallback but it doesn't.
     * It seems that transitions are not well supported in Tanstack Router,
     * so instead of wrapping the `navigate` call in `startTransition`, we
     * should rely on `useDeferredValue` somehow to stop React from showing the
     * Suspense fallback.
     */
    startTransition(() => {
      navigate({
        search: { code: data.get("code") as string },
        replace: true,
      });
    });
    // ------------------------------------------------------------------------
  }

  return (
    <div>
      <h1>Find language</h1>

      <form onSubmit={handleSubmit}>
        <label>
          <span>Enter a language code (e.g. en, ja, fi, sv)</span>
          <input type="text" name="code" defaultValue={search.code} />
        </label>
        <button type="submit">Search</button>
      </form>

      <div>
        {isPending ? (
          <p>Pending...</p>
        ) : data.language ? (
          <h2>{data.language.name}</h2>
        ) : (
          <div>No language found</div>
        )}
      </div>
    </div>
  );
}

function ErrorComponent() {
  return <div>Error</div>;
}

function PendingComponent() {
  return <div>Loading...</div>;
}

With this code whenever the search params change the Suspense fallback is briefly shown even though the call to navigate is wrapped in startTransition. This limitation is not really an Apollo issue but it does surface a need to have a way to stop showing the Suspense fallback without startTransition. As mentioned in at start the only other way to do that is with useDeferredValue but with that approach we lose the inline pending indicator.

I tried to do the following to get the pending state:

import { equal } from "@wry/equality";

// ...
const isPending = !equal(queryRef, deferredQueryRef);

But that doesn't seem to work. I guess it's because queryRef contains symbols and functions which are not stable and thus always result in false in the equal comparison 🤔

However, that same approach seems to work with useSuspenseQuery so we are able to somewhat circumvent this issue by not using useReadQuery but instead use a deferrable version of useSuspenseQuery that defers the query variables in the same way Dominik hinted in his quote above:

import { useDeferredValue } from 'react';
import { equal } from '@wry/equality';
import {
  useSuspenseQuery,
  type DocumentNode,
  type TypedDocumentNode,
  type SuspenseQueryHookOptions,
  type NoInfer,
  type OperationVariables,
} from '@apollo/client';

export function useSuspenseQueryDeferred<
  TData = unknown,
  TVariables extends OperationVariables = OperationVariables,
>(
  query: DocumentNode | TypedDocumentNode<TData, TVariables>,
  options?: SuspenseQueryHookOptions<NoInfer<TData>, NoInfer<TVariables>>
) {
  const variables = useDeferredValue(options?.variables);

  const result = useSuspenseQuery<TData, TVariables>(query, {
    ...options,
    variables,
  });

  const suspending = !equal(variables, options?.variables);

  return { ...result, suspending };
}

And then use it in the component:

function RouteComponent() {
  const search = Route.useSearch();
  const navigate = useNavigate({ from: Route.fullPath });

  // --------------------------- 👇👇👇 -------------------------------------
  const { data, suspending } = useSuspenseQueryDeferred(GET_LANGUAGE, {
    variables: { code: search.code },
    fetchPolicy: "network-only",
  });
  // ------------------------------------------------------------------------

  function handleSubmit(e: FormEvent<HTMLFormElement>) {
    e.preventDefault();

    const data = new FormData(e.currentTarget);
    const code = data.get("code") as string;

    if (code.length !== 2) return;

    // ℹ️ No need for `startTransition` anymore
    navigate({
      search: { code: data.get("code") as string },
      replace: true,
    });
  }

  return (
    <div>
      ...use `suspending` here...
    </div>
  );
}

However this is not as ergonomic DX-wise as we need to repeat the query options exactly the same way as in the preloaded query, and as mentioned in this comment by @jerelmiller there are no guarantees that Apollo won't make two network requests with this approach compared to the useReadQuery approach.

So in summary, is there a way to use useReadQuery with useDeferredValue in a way that we can still get the loading state of the query when the variables of it change, or is there some other approach we can take to stop React from showing the Suspense fallback in situations where we are not able to use startTransition?

PS: I couldn't find any mention of useDeferredValue in the Apollo Client docs so it might be a good idea to add a section about it in the Suspense page.

Link to Reproduction

https://github.com/Temzasse/router-comparison/tree/main/tanstack-router

Reproduction Steps

After cloning the repo locally:

  1. cd tanstack-router
  2. npm install
  3. npm run dev
  4. Open http://localhost:3001/suspense
  5. Open browser devtools
  6. Type eg. ja or fi in the input and submit the form
  7. Notice how the log for isPending state is always false.
  8. You can try to throttle the network to "Slow 4G" to emphasise how we never get the pending state even if the query takes a long time to resolve.

@apollo/client version

3.12.3

@Temzasse Temzasse changed the title How to use useDeferredValue with useReadQuery to stop showing Suspense fallback? How to get loading state when using useDeferredValue with useReadQuery? Dec 16, 2024
@jerelmiller
Copy link
Member

Hey @Temzasse 👋

I'll do my best to dig into your reproduction a bit further when I have time (unless @phryneas beats me to it).

A quick note though, I'm curious if === works for comparing the value returned from useDeferredValue rather than the equals functions. Since you're creating query refs outside of render in the loader function, those objects should be stable so === should theoretically work to detect whether you're rendering the deferred query ref (i.e. the pending state) or not. Could you try that as your comparison rather than equals?

@Temzasse
Copy link
Author

Well I'll be damned. I clearly was way too deep into this topic and couldn't see the obvious simple solution 🙈

Your suggestion works nicely:

const { queryRef } = Route.useLoaderData();
const deferredQueryRef = useDeferredValue(queryRef);
const { data } = useReadQuery(deferredQueryRef);
const isPending = queryRef !== deferredQueryRef; // 🎉🎉🎉

The only minor UX issue is that the isPending is true for a moment when refetching data that is already cached. However that should be easily fixed with something like spin-delay:

import { useSpinDelay } from 'spin-delay';

const isPending = useSpinDelay(queryRef !== deferredQueryRef);

@jerelmiller feel free to close this issue.

Would it be possible to have a mention about this useDeferredValue pattern in the Suspense related docs?

@jerelmiller
Copy link
Member

jerelmiller commented Dec 16, 2024

@Temzasse glad that worked! A bit of a bummer you have to do it that way, but I can understand the tradeoff that TanStack Router made here.

A mention about useDeferredValue in the docs definitely makes sense. Are you thinking we need a mention of useDeferredValue generally, or a specific statement for use with TanStack Router? There are multiple angles we can add here (useDeferredValue with variables and useSuspenseQuery, this use case with queryRef, etc.) so wanted to get your feedback.

@Temzasse
Copy link
Author

Temzasse commented Dec 16, 2024

I'm def not an expert at writing docs but here are my 2 cents:

In think most people don't yet fully understand how startTransition and useDeferredValue work in general and how they are related to Suspense. So I believe it would be great to have some generic introduction and then some examples of common situations where the rough edges of Suspense manifest (namely these kind of refetching situations that reshow the fallback).

Maybe it's just my experience but my challenges with Suspense have always revolved around URL state that drives the query variables. I initially struggled with this same issue with React Query and wrote about it on my blog. I think it's great that you mention startTransition in your docs but often having the pending state co-located with the query and not with the useTransition is more preferable, in which case using useDeferredValue makes more sense imo. So I wouldn't consider useDeferredValue as a "bummer" case necessarily but instead as an alternative pattern that is perfectly valid (altho some people might argue against this 😄).

I totally get that having library/framework specific examples in the docs can be quite challenging to produce and maintain but I think most frameworks, Next.js / Tanstack / Remix / React Router, start to have very similar patterns and hooks that could be generally documented. For example loader functions or useSearch, useSearchParams, useNavigation, useRouter, for updating the URL search params. Highlighting these patterns in your docs would be a great learning resource for React devs that want to level up their game with web fundamentals, like having the URL drive most of the app state (compared to ephemeral component state or by calling refetch() in event handlers manually with new variables).

Hopefully that helps 🤓

@jerelmiller
Copy link
Member

Yep this is very helpful! Appreciate the feedback!

Its always tricky to find the right balance between going into general Suspense concepts vs Apollo Client's specific integration for it. We'd rather rely on the React docs to do most of the teaching on Suspense itself (since those docs are wayyyyy better than anything we could put out). We've leaned toward showing the integration aspect while hoping readers would go to the React docs to learn more about the general Suspense concepts.

We are also trying not to be too prescriptive from our end as we want devs to feel like they have the flexibility to do what works best for them. I think I avoided the useDeferredValue example because I couldn't come up with a good example that doesn't shoehorn the use of it to only a certain case. Definitely not an excuse not to mention at least using useDeferredValue in the docs somewhere though! I'll try to workshop something that shows the two working together.

Thanks again for the discussion! Hopefully this issue helps someone else in the future 🙂

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

@MickJasker
Copy link

I'm encountering a similar issue in my project, but with React Router v7 instead of TanStack. I tried adapting your example, but the behavior isn't quite the same.

In our case, when the search params change, we receive a new queryRef from our loader, which I pass into the useDeferredValue hook. This works fine on the initial render, the component suspends until the data is loaded. However, when a new queryRef is received (after the search query changes), useDeferredValue immediately reflects the new query, rather than keeping the old one.

When this new value is passed to useReadQuery, rendering is blocked until the new data is available, but the suspense fallback doesn't show. This is problematic because we want to display a small indicator showing that the data is being refreshed in the background.

Do you have any insights into what might be different in my approach compared to yours that could be causing this issue?

const query = useLoaderData<PreloadedQueryRef<CampaignOverviewQuery>>();
const deferredQuery = useDeferredValue(query); // is always the same as query even when the query is refetching with new variables

const { data } = useReadQuery(deferredQuery); // will block rerender until new data is loaded

const isRefetching = query !== deferredQuery;
console.log('isRefetching', isRefetching); // is always false here

@jerelmiller
Copy link
Member

@MickJasker React Router wraps route transitions in startTransition (which was the original motivation behind the .toPromise() method on the query ref) so perhaps that is something? I'm actually not entirely sure how useDeferredValue works when its already inside a transition, but my guess that has something to do with it.

@jerelmiller
Copy link
Member

I found this discussion with this explanation:

When the value passed to useDeferredValue changes, it will either return the previous value or the new value depending on the priority of the current render.

You can think of "deferring" a value as similar to throttling or debouncing it.

If the current render is the result of an urgent update, like a user input, React will return the previous value. Then, it will switch to the new value after the current render has completed.

The deferred value has the same priority as updates scheduled by startTransition. Like startTransition updates, deferred values can suspend without triggering an unexpected fallback.

useDeferredValue and startTransition have broadly the same behavior.

If I'm reading this right, I'm guessing useDeferredValue is returning the new value because of the priority of the render already being inside of a startTransition. I could be wrong, but thats how I interpret this.

@MickJasker
Copy link

This is an interesting read and makes a lot of sense to me. Unfortunately, we'll need to look for a different solution. We’ve used toPromise in the past, but we’re looking for a "render as you fetch" approach where parts of our UI stream in progressively. The problem with toPromise is that it blocks navigation until the promise resolves, which isn’t ideal for our use case.

The issue is that React Router can’t track the query’s promise when it’s not being awaited in the loader. A flow that seems logical to me would be for networkStatus (in useReadQuery) to change to loading or refetch when the query changes in useLoader, before Suspense is triggered again. I'm not sure if this is feasible, but it seems like a better fit for what we're trying to achieve.

@jerelmiller
Copy link
Member

A flow that seems logical to me would be for networkStatus (in useReadQuery) to change to loading or refetch when the query changes in useLoader, before Suspense is triggered again.

Unfortunately this is just not possible with the way Suspense works 😞. useDeferredValue/startTransition work by rendering your component with the old state value, then again with the new state value in a background render and if that new state value causes the component to suspend, it will show your old state value until the promise with the new state value resolves. Background renders can get cancelled if new state values are set before the new promise resolves (imagine a user typing another character in search while the previous search is in flight).

useReadQuery works by checking if the promise in the queryRef is pending, and if it is, it will suspend until the value is resolved. Technically the network status is updated in useReadQuery, but because it suspends, you have no way of reading that value. Using queryRef with useDeferredValue/startTransition means that React is rendering your component with the old query ref (which is resolved with NetworkStatus.ready), while rendering the new query ref in the background (which is suspending with NetworkStatus.loading set). We just have no way of connecting these two things in Apollo Client.

This is why something like useTransition with the isPending flag is needed from React to tell you if you're rendering in a transition, because its impossible to determine otherwise what kind of render React is doing with your component. useReadQuery has to suspend in order to work with transitions and to provide the guarantees that it does (i.e. block until loaded). Unfortunately if you're not able to get the old value with useDeferredValue, I'm not entirely sure how to approach this. I think React Router is doing the right thing here because React recommends wrapping navigations in transitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants