Skip to content

Heads up: Deprecation of useQuery and useLazyQuery lifecycle hooks #12352

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

Open
phryneas opened this issue Feb 7, 2025 · 18 comments
Open

Heads up: Deprecation of useQuery and useLazyQuery lifecycle hooks #12352

phryneas opened this issue Feb 7, 2025 · 18 comments

Comments

@phryneas
Copy link
Member

phryneas commented Feb 7, 2025

Deprecation of useQuery and useLazyQuery lifecycle hooks

With the release of Apollo Client 3.13, we will be deprecating the useQuery and useLazyQuery lifecycle hooks onCompleted and onError and will be removing them in Apollo Client 4.0.

These lifecycle hooks have long been the cause of confusion, bugs and frustration for many developers. With this step we are following other tools like React Query in their removal.

We encourage you to read this comprehensive blog post by React Query's maintainer Dominik Dorfmeister which provides some great insight into the pitfalls of these callback APIs. Apollo Client shares many of the same concerns.

While Apollo Client shares similar concerns, there are some additional reasons behind this change.

onCompleted

Conflicting interpretations for onCompleted behavior

Apollo Client uses a normalized cache, which means that there are many different reasons why the data displayed by a component might change:

  • The query initiated by the current hook might return data
  • The same query might be initiated by another hook and update existing data
  • You call fetchMore, refetch, etc.
  • Another query or mutation might be overlapping with the query of the current hook and update some of its data
  • An optimistic update might update some or all of the data of the hook
  • A manual cache update might change the data for the hook

Apollo Client users have provided logical justification for each of these cases for why the onCompleted callback should or should not execute.

Added ambiguity around @defer

With the introduction of the @defer directive in the GraphQL ecosystem, we have yet another source of "updates" which provides further complication.
Should onCompleted run once the initial chunk of data arrives? After all deferred fragments arrived? After each fragment?
While one behavior might make sense to some, others might have vastly different conflicting opinions that are equally valid.

Changes around the behaviour

Adding insult to injury, onCompleted had a bug in versions from versions 3.5 to 3.7 where cache changes in addition to network requests would execute the onCompleted callback. This was fixed in version 3.8 with #10229, but the damage was done.
While the users who initially reported the bug were satisfied, others came to expect the behavior from 3.5 to 3.7 as the correct behavior.

Given this history, we are not confident that we can provide an approach that is intuitive for everyone and doesn't add more confusion among our userbase.

Our recommendation

As we've received questions about onCompleted through various issues and our community forum, the solutions we propose often involve moving away from onCompleted entirely. Many of the cases where we see onCompleted used
involve some kind of state syncing which is a highly discouraged pattern (see the section on "State
Syncing"
in Dominik's blog post for an example.) We have recommended against using the the use of these callbacks for quite some time now for one reason or another and believe its time for us to sunset these APIs.

Bugs

The final straw that made us come to this decision was this bug report:
With the current implementation, in some circumstances, the onCompleted and onError callbacks can be stale by one render. Unfortunately there is no perfect solution that can prevent that from happening in a manner that won't introduce new bugs, for example when using suspense in your App.

React's useEffectEvent hook might solve this problem for us, but that hook is still experimental.
Even when it is available, it won't be backported to the old React versions which means we cannot provide a working solution for our entire userbase.

This isn't the first issue that's been opened in regards to timing issues with onCompleted. Once again, this is one of those cases where varying logical opinions make it impossible to determine the correct behavior. React itself does
not guarantee stability on the timing of renders between major versions which further complicates this issue as upgrading React versions might subtly change the timing of when onCompleted executes.

With the current available primitives, fixing this might be possible in a very hacky way, but given everything else and the fact that we discourage its use, we want to move everybody off these callbacks instead of pushing additional bundle
size on all our users.

What to use instead

Once again, we recommend reading the blog article by Dominik Dorfmeister which provides a lot of answers on what to use in place of these callbacks.

In short:

  • For derived state, use useMemo
  • If you want to reset state in child components, use key
  • If you want to (re)set or modify local component state as a reaction to the hook result changing, you can actually call the setState function of useState during component render, so you can use this to compare new results with old results and modify state as a consequence.
    See this example in the React docs.
    Keep in mind that this is a very rare use case and you should usually go with useMemo.
  • If you are interested when an in-flight query is finished, keep an eye on networkStatus
  • To synchronize things outside of React with your received data, use useEffect.

onError

onError is too localized

The onError callback will only execute when a query executed by the current hook returns an error.
While that seems fine, remember: if you have two components calling the useQuery hook with the same query, but different onError callbacks, only one or the other will execute - depending on the order the components were rendered in, and also influenced by prior cache contents.

Bugs

onError suffers from the same potential timing issue described for onCompleted.

What to use instead

Your component is probably the wrong place to handle errors like this, and you should probably do it in a more centralized place such as an onError link, or if you are using the suspenseful hooks, in an ErrorBoundary that is parent to
all components potentially calling your query. Future versions of the client will switch to throwing errors by default which further negates the need for the onError callback.

useMutation is not affected by this deprecation

useMutation doesn't suffer from the same problems laid out here so these callbacks are unaffected. While we can't guarantee that we won't deprecate these callbacks in a future version, this deprecation is focused on useQuery and
useLazyQuery.

@dacevedo12
Copy link

Is a global error handling approach feasible for large apps?

I think in practice this will push people towards adopting suspense + error boundaries

@jerelmiller
Copy link
Member

@dacevedo12 global error handling can be achieved through the error link already. Are you looking for something other than this?

@dacevedo12
Copy link

dacevedo12 commented Feb 21, 2025

I'm concerned about the feasibility (and maintainability) of having a single giant error link handling all possible errors in a large app.

The same error can mean different things or be expected to have different handling depending on the view it's on

I see value in a localized onError

@jerelmiller
Copy link
Member

jerelmiller commented Feb 21, 2025

@dacevedo12 I'm curious, what would onError provide that reading the error property returned from useQuery wouldn't? Are there certain things you can do with onError that you can't do by reading that error property? That property will continue to exist to allow you to handle localized errors, we're just removing the callback itself.

@georeith
Copy link

georeith commented Feb 25, 2025

@dacevedo12 I'm curious, what would onError provide that reading the error property returned from useQuery wouldn't? Are there certain things you can do with onError that you can't do by reading that error property? That property will continue to exist to allow you to handle localized errors, we're just removing the callback itself.

Is React guaranteed not to batch errors received in quick succession? E.g., if I call the same mutation twice in a render with different values, and the responses error at the same time and I listen to these errors with a useEffect am I guaranteed to receive each error in a separate effect or could one be lost?

@jerelmiller
Copy link
Member

@georeith we aren't removing onError from useMutation, so at least in the example you gave, this shouldn't be an issue. Errors from mutations aren't propagated to queries.

The only case I could maybe see on the useQuery side would be two refetch calls in quick succession, but even then query deduplication usually kicks in there. You'd have to disable query deduplication in order for there to be a chance at this case. Even so, you still have access to the promise returned from each of these calls which will resolve/reject and get you access to those individual errors from each call. This isn't a pattern we recommend though anyways.

I'm curious though, what kind of work are you doing in onError that you're concerned might be affected by this change?

@georeith
Copy link

georeith commented Feb 25, 2025

@jerelmiller my bad I didn't notice that, gave a bad example.

I do have a case of this for query, we have a paginated table view, thats loaded by one query.

But each row in this table also displays live changing data (list of people who are in video calls and what the status of the video call is).

We recieve webhook events that tell us a specific row has changed and then use a shared lazy query to fetch that singular row and update the cache for the affected row.

If two rows change at once then there is a slim but possible chance our error handling code or cache update code won't run for one of them right (if we depend on the returned error or data instead of onError and onCompleted)?

I guess we could work around it by having a separate lazy query per row component.

@phryneas
Copy link
Member Author

phryneas commented Feb 28, 2025

I guess we could work around it by having a separate lazy query per row component.

@georeith Just as a suggestion: don't use a lazy query here, but call client.query. The point of hooks is to synchronize with your component and rerender them on state change. If you don't rely on the loading, data or error properties of that hook call you are not really synchronizing anything with your component, and calling client.query directly is a better fit.

That said, both client.query as well as the useLazyQuery execute function return a promise. You don't need an onError callback in those cases, you can just client.query(...).catch(onError) in those cases.

@Tyresius92
Copy link

I'm a bit confused on this one. I read the blog post, and I understand the reasons for this deprecation, but I'm unclear on what the alternatives are, especially for a case like this, where we explicitly need state syncing:

const myQuery = gql`
  query MyQuery {
    currentUser {
      id
      name
    }
  }
`

const MyComponent = () => {
  const [name, setName] = useState('');

  const { loading } = useQuery(myQuery, {
    onCompleted: (response) => setName(response?.currentUser?.name ?? '')
  });

  const onSave = () => { ... }

  return (
    <div>
      <input value={name} onChange={e => setName(e.target.value)} disabled={loading} />
      <button onClick={onSave} disabled={loading}>Save</button>
    </div>
  )  
}

Derived state (e.g. const name = data.currentUser.name) does not work in this case, since I explicitly need the value to exist in state since it's an input value.

I can call setName in a useEffect, but the blog post warns against that as well.

What would the recommended pattern for something like this be?

@phryneas
Copy link
Member Author

@Tyresius92 useEffect would even be okay in a case like this, but probably a setState call during render would be better if you want to avoid an additional rerender.

See also the recommendation from above:

If you want to (re)set or modify local component state as a reaction to the hook result changing, you can actually call the setState function of useState during component render, so you can use this to compare new results with old results and modify state as a consequence.
See this example in the React docs.

const [name, setName] = useState("");

const { data, loading } = useQuery(myQuery);
const [lastSeenName, setLastSeenName] = useState(data?.currentUser?.name);
if (lastSeenName !== data?.currentUser?.name) {
  setName(data?.currentUser?.name);
  setLastSeenName(data?.currentUser?.name);
}

@mogelbrod
Copy link

mogelbrod commented Apr 24, 2025

If you are interested when an in-flight query is finished, keep an eye on networkStatus

Most of my company's usage of onCompleted consists of conditionally (sometimes) triggering an effect when the query finishes, and often enough the effect doesn't result in a re-render. Relying on networkStatus would imply enabling notifyOnNetworkStatusChange: true for the query, which in turn significantly increases the number of re-renders of the component in question. This issue has been mentioned before, ex: #5531 (comment), #9151 (comment).

What are our options if we wish to avoid these unnecessary re-renders? Would it be possible to introduce a onNetworkStatusChange(query: { networkStatus, data, ... }) or similar callback that lets the component subscribe to changes without forcing re-renders? @phryneas

@phryneas
Copy link
Member Author

Most of my company's usage of onCompleted consists of conditionally (sometimes) triggering an effect when the query finishes

Given how ambiguous onComplete currently triggers, this might be exactly the right behaviour in your case, but cause bugs for others, or it might even be incorrect behaviour in your application and you just didn't notice those subtle bugs yet. I would really not recommend to rely on onComplete here.

Relying on networkStatus would imply enabling notifyOnNetworkStatusChange: true for the query, which in turn significantly increases the number of re-renders

Generally we will set notifyOnNetworkStatusChange: true as default in the 4.0 release of Apollo Client.
You can of course opt out of it, but many users have been confused by loading not changing appropriately.

As for more rerenders, there are a lot of ways to prevent full child trees from rerendering even if a parent component renders, like returning a child tree created with useMemo, wrapping child components in React.memo or using the new React Compiler.

Especially with the React Compiler, there is a mindset shift here towards something that should have been the case all the time:
Rerenders should be very cheap, so the number of rerenders (excluding excessive rerenders, but that's not the case with the change of a network status) should not be something to optimize for.
We will be running Apollo Client itself through the React Compiler, so you will be getting the minimal amount of updates from our hooks, but it won't skip updates that might be relevant for a reliant component - for that, you'll have to run the compiler on your code base (or manually optimize).

@mogelbrod
Copy link

mogelbrod commented Apr 24, 2025

Appreciate the quick response @phryneas!

Given how ambiguous onComplete currently triggers, this might be exactly the right behaviour in your case, but cause bugs for others, or it might even be incorrect behaviour in your application and you just didn't notice those subtle bugs yet. I would really not recommend to rely on onComplete here.

Just to clarify: I understand the reasoning for removing onComplete, and agree that many cases of onComplete can be be rewritten as useMemos/useEffect calls.

I do however see plenty of cases where devs want to react to the network status changing without necessarily re-rendering the component, and if there was a onNetworkStatusChange callback this would be trivial. The reason I think it's a good fit is that it doesn't introduce any new terminology or semantics, while letting consumers react to network status changes. These are more clearly defined, and thus should avoid the confusion of onCompleted.

Generally we will set notifyOnNetworkStatusChange: true as default in the 4.0 release of Apollo Client. You can of course opt out of it, but many users have been confused by loading not changing appropriately.

As for more rerenders, there are a lot of ways to prevent full child trees from rerendering even if a parent component renders, like returning a child tree created with useMemo, wrapping child components in React.memo or using the new React Compiler.

It will be some time before we can adopt React compiler in our codebase. It would be unfortunate if we until then had to resort to manual memoization (probably the least enjoyable part of working with react) to avoid redundant re-renders 🙁

@phryneas
Copy link
Member Author

phryneas commented Apr 24, 2025

and if there was a onNetworkStatusChange callback this would be trivial.

Adding it would be far from trivial, unfortunately.
If you set notifyOnNetworkStatusChange: false, the Apollo Client core will emit completely different events than on notifyOnNetworkStatusChange: true.
There's no way for us to implement a version of onNetworkStatusChange that would behave fully correctly if you would not also turn on notifyOnNetworkStatusChange at the same time.
We would recreate the same ambiguous behaviour we want to get rid of by deprecating onCompleted.

@jerelmiller
Copy link
Member

@mogelbrod mind going into more detail on this?

Most of my company's usage of onCompleted consists of conditionally (sometimes) triggering an effect when the query finishes, and often enough the effect doesn't result in a re-render

Would you be willing to give us some code samples of the kind of side effects you're triggering here?

@mogelbrod
Copy link

mogelbrod commented Apr 25, 2025

Adding it would be far from trivial, unfortunately. If you set notifyOnNetworkStatusChange: false, the Apollo Client core will emit completely different events than on notifyOnNetworkStatusChange: true. There's no way for us to implement a version of onNetworkStatusChange that would behave fully correctly if you would not also turn on notifyOnNetworkStatusChange at the same time. We would recreate the same ambiguous behaviour we want to get rid of by deprecating onCompleted.

Gotcha, that's a great argument against adding it 😄 I was hoping it would be relatively simple given that notifyOnNetworkStatusChange already existed, so thanks for elaborating on why it isn't!

Would you be willing to give us some code samples of the kind of side effects you're triggering here?

We've got an abstraction over useQuery() called useQueryTtl() that adds basic TTL support on a query level, conditionally upgrading cache-first to cache-and-network depending on the query staleness. In this method a onCompleted() callback keeps track of when the query finishes and schedules the next refresh (which requires no re-rendering). We'll be experimenting with replacing parts of this logic with pollInterval+skipPollAttempt, but it's not clear if it's possible. I'll happily share the code for the entire hook if you're interested. Building this functionality into a cache layer or apollo link would've been preferable, but I've yet to find a way to accomplish this since neither has access to all information that's necessary to function.

Other usage in our code base mostly consist of:

  • Analytics/tracking off specific queries, in which case we don't need notifyOnNetworkStatusChange. These could be converted to using a "TrackingLink" approach configured by context, with the downside of making it harder for devs to use and understand it
  • Sometimes calling some setState() function if the data received has a specific format

Come to think about it, wouldn't it be possible to essentially implement onCompleted in user-land by passing a callback in the query context, and then adding a ApolloLink that calls context.onCompleted after an outgoing request finishes? Rough example:

const query = useQuery(MyQuery, {
  context: {
    onCompleted: (data) => console.log('onCompleted', data)
  },
})

@jerelmiller
Copy link
Member

adds basic TTL support on a query level

FYI we do have cache TTL on our short list of thing we want to tackle after 4.0 is released, so definitely keep watch for this! We've heard about TTL quite a bit recently and think its time we get something in there 🙂 (I know this doesn't help you until the feature lands, but just wanted you to be aware its coming sooner than later)

Building this functionality into a cache layer or apollo link would've been preferable, but I've yet to find a way to accomplish this since neither has access to all information that's necessary to function

I'm curious what information you'd need in the link chain to make this work as you're expecting. I want to point you to #12576 which just landed in 4.0.0-alpha.11. This adds a client property to operation so you can access the client (and the cache) that kicked off the request to the link chain. Would having access to the client/cache in the link chain be enough to do what you'd need?

const link = new ApolloLink((operation, forward) => {
  const client = operation.client;
  // ...
})

Analytics/tracking off specific queries

I'm not sure entirely what kind of analytics you're tracking, but I do want to mention that there is a possibility of bugs given the current implementation of onCompleted. You'd only be able to accurately track data from the initial response since that has a guaranteed networkStatus change, but cache changes, refetches, etc. can't be accurately tracked without notifyOnNetworkStatusChange which may or may not be a detriment here.

Additionally if your callback relies on values outside the function, you might have stale closures and this could lead to subtle bugs or inaccuracies. I would double check this to be sure 🙂

Sometimes calling some setState() function if the data received has a specific format

I'd be cautious of doing this because its very easy for that state value to get out of sync with data. If a cache update causes the hook to rerender, that state value will be stale and out-of-sync. If that bit of data is at all critical to your view, you'll end up with UI bugs showing a stale value. We recommend derived state for these types of data transformations, that way whenever the value changes, whether it be a refetch, cache update, etc. you'll always be in sync with the correct value.

wouldn't it be possible to essentially implement onCompleted in user-land by passing a callback in the query context

Actually yes! This is very possible. The link implementation would look something like this (NOTE: I haven't tested these, but this should give you a rough idea):

3.x (which uses zen-observable)

import { Observable, type FetchResult } from "@apollo/client";

const onCompletedLink = new ApolloLink((operation, forward) => {
  return new Observable((observer) => {
    let result: FetchResult;

    const subscription = forward(operation).subscribe({
      next: (value) => {
        result = value;
        observer.next(value);
      },
      error: observer.error.bind(observer),
      complete: () => {
        operation.getContext().onCompleted?.(result.data);
        observer.complete();
      }
    });
    return () => subscription.unsubscribe();
  });
});

4.x (which uses RxJS)

import type { FetchResult } from "@apollo/client";
import { tap } from "rxjs";

const onCompletedLink = new ApolloLink((operation, forward) => {
  let result: FetchResult;
  return forward(operation).pipe(
    tap({
      next: (value) => {
        result = value;
      },
      complete: () => operation.getContext().onCompleted?.(result.data)
    })
  );
});

@jerelmiller
Copy link
Member

Quick note on that code above ☝. There is one flaw with the link approach which is that if you use query deduplication (enabled by default), there is a possibility that onCompleted might not be called since requests would be batched. context is not combined with in-flight requests, so only the request that kicked off the fetch would be notified. You'd need code in Apollo Client core in order to handle deduplicated requests, otherwise you'd need to disable that feature in order for this to reliably be called 100% of the time.

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

6 participants