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

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

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

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

phryneas opened this issue Feb 7, 2025 · 8 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.

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

4 participants