[WIP] React Query Optimizations - Maintaining Object References

[kevupton]

Issue:

Right now it is impossible to use the object returned by the useQuery in dependency arrays for react. This is because each rerender we are recreating the object and then returning a new instance.

Suggestion

We give the user the ability to turn off optimistic results, by using an optimistic: false flag on the options.

const query = useQuery({ optimistic: false });
useEffect(() => {
  // do something in here
}, [query]);

Additional Optimization Improvement

We can also improve performance by caching the options object so that we do not recreate it each time, as the options._defaulted prevents it from be defaulted again.

useQueryOptions

import {useQueryOptions} from '@tanstack/react-query';

export const useAccountOptions = (id: string) => {
  return useQueryOptions(
    () => ({
      enabled: !!id,
      queryKey: ['account', id],
      queryFn: async () => ...,
    }),
    [id]  // only update the options object whenever the id changes.
  );
};

Usage:

const options = useAccountOptions(id);
const query = useQuery(options);

Notes:

useQueryOptions means that the options object is more accessible and can be used by useQueries much more easily, or passed around and used as needed.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
query	⬜️ Ignored (Inspect)	Visit Preview		Sep 9, 2023 1:29pm

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b8b042cd5ae80c5dc898feebaed0aaabd154e4f6:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[TkDodo]

without optimistic results, you'll get the first render cycle in a state that has isFetching: false. Also, you will not get notified about observer option changes because observer.setOptions assumes updates have been propagated optimistically already:

https://github.com/TanStack/query/blob/fe0ae802d8e7521ba6d55239e7daf03ec0dd20fa/packages/react-query/src/useBaseQuery.ts#L89-L93

I would generally say that if you want referential stability, you should probably be using data returned from useQuery, as it is structurally shared out of the box:

const { data } = useQuery(...)

useEffect(() => {
  // do something in here with data
}, [data]);

[kevupton]

@TkDodo I ran some tests and the first render cycle does appear to be fetching:

Using this code:

const useTest = () => {
  const query = useQueryOptions(() => ({
    queryKey: ["test"],
    queryFn: async () => {
      await new Promise((res) => setTimeout(res, 30000));
      return true;
    },
  }));

  return useQuery(query);
};

I believe the reason is that we are already optimistically creating the results when we initially create the QueryObserver.

Here the constructor of the QueryObserver calls https://github.com/TanStack/query/blob/dec9ed34da3ea23b9d109d860ac4e6048339c70c/packages/query-core/src/queryObserver.ts#L94

Which eventually leads to: https://github.com/TanStack/query/blob/dec9ed34da3ea23b9d109d860ac4e6048339c70c/packages/query-core/src/queryObserver.ts#L597

From what I can see is doing the same thing as calling https://github.com/TanStack/query/blob/dec9ed34da3ea23b9d109d860ac4e6048339c70c/packages/react-query/src/useBaseQuery.ts#L76

as that method also calls https://github.com/TanStack/query/blob/dec9ed34da3ea23b9d109d860ac4e6048339c70c/packages/query-core/src/queryObserver.ts#L243

So the only reason I thought to leave the optimistic results there, was in the scenario after the very first render, as I am not sure about the unknown possibilities the getOptimisticResults would influence after the first render.

But I could definitely be missing something as there are a lot of conditions happening here... 🤔

[codecov-commenter]

Codecov Report

Patch coverage: 87.67% and project coverage change: +0.16 :tada:

Comparison is base (ed7d9f8) 91.90% compared to head (c464301) 92.06%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5165      +/-   ##
==========================================
+ Coverage   91.90%   92.06%   +0.16%     
==========================================
  Files         111      113       +2     
  Lines        4188     4248      +60     
  Branches     1083     1103      +20     
==========================================
+ Hits         3849     3911      +62     
+ Misses        318      316       -2     
  Partials       21       21              

Impacted Files	Coverage Δ
packages/query-core/src/query.ts	99.03% <ø> (ø)
...kages/react-query-devtools/src/styledComponents.ts	92.85% <ø> (ø)
packages/react-query/src/errorBoundaryUtils.ts	100.00% <ø> (ø)
packages/react-query/src/reactBatchedUpdates.ts	100.00% <ø> (ø)
packages/react-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/react-query/src/useIsFetching.ts	92.30% <ø> (ø)
packages/react-query/src/useIsMutating.ts	92.30% <ø> (ø)
packages/react-query/src/useMutation.ts	96.15% <ø> (ø)
packages/react-query/src/useQuery.ts	100.00% <ø> (ø)
packages/react-query/src/useSyncExternalStore.ts	100.00% <ø> (ø)
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[TkDodo]

could we instead just structurally share the optimistically created result with the current result? Something like, in getOptimisticResult:

- return this.createResult(query, options)
+ return replaceData(
+   this.#currentResult,
+   this.createResult(query, options),
+   options,
+ )

This should lead to currentResult being returned in case there are no changes. We'd need to keep the useMemo on the trackResult call though

[TkDodo]

So the only reason I thought to leave the optimistic results there, was in the scenario after the very first render, as I am not sure about the unknown possibilities the getOptimisticResults would influence after the first render.

So I went ahead and just used the return value of useSyncExternalStore in useBaseQuery, thus removing the call to getOptimisticResult altogether, which is something that I would generally really appreciate.

However, it leads to 14 test failures out of 128 of the useQuery.test.tsx test suite.

Probably, in one of those failing test lies the answer to why we can't easily do this if you want to look at this a bit further 😅

[TkDodo]

right, the reason is probably that setOptions is only called in the constructor and then again in an effect. So if your query changes queryKeys, without the call to getOptimisticResult, it would have one render cycle where the options are from an old observer.

[kevupton]

yeh ok awesome, will take a look into this one more and see what we can do 🙏

[nx-cloud[bot]]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 2075610a141a2f455fac5fb3312643ab111bb26b. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

🟥 Failed Commands
nx affected --targets=test:eslint,test:lib,test:types,test:build

✅ Successfully ran 1 target

• nx affected --targets=test:lib --base=18ba7c0c3f506773371844407a6bb0f5a69eb955

---

Sent with 💌 from NxCloud.

[kevupton]

@incepter @TkDodo Let me know what you guys think so far. There are a few larger changes here, but in doing them, I have been able to reduce the number of rendering and state updates, plus able to maintain a object equality when there have been no changes.

I have put a pause on fixing tests and other dependencies until there is some signal that this is something you guys are interested in.

As my concern is that the amount of changes mean this will be unlikely to be merged. But maybe there is a simpler way inside some of the changes I have made. Open to suggestions, ideas and feedback. 🙏

Best to have a look at QueryClient and useBaseQuery inside react-query.

[TkDodo]

@kevupton sorry for letting this one slide. I'm generally interested in getting rid of getOptimisticUpdate, if we can do it in a backwards compatible way. We've released v5 in the meantime - would you want to take another stab at it? I can see there are quite some conflicts, probably mainly because we switched to private class fields

[TkDodo]

I think it would be easier to start from scratch here. Let me know if you're interested in completing this.

[kevupton]

@TkDodo No problem, apologies for the delayed response. I agree would it be best to go from scratch. I will have a go again and see how I go 🙏