The query gets stuck at fetching status when re-fetched data has cycles

[stanislav-halyn]

Describe the bug

The query gets stuck at fetching status when you try to re-fetch data and it has cycles in it.

Your minimal, reproducible example

https://codesandbox.io/p/sandbox/react-query-mfsz4x

Steps to reproduce

• create a query that returns data with cycles in it
• re-fetch the query
• see that the fetchStatus gets stuck at fetching

Expected behavior

The query should change the fetchStatus to idle when re-fetch has been finished.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

• OS: MaxOs
• Browser: Chrome
• Version: 121.0.6167.184 (Official Build) (x86_64)

Tanstack Query adapter

react-query

TanStack Query version

v5.22.2

TypeScript version

No response

Additional context

As far as I understand, when you turn off structuralSharing for the query - the issue will go away.
After some digging, I found that default structuralSharing functionality is to call replaceEqualDeep function. And when there are cycles in the data, this function goes into infinite loop because it recursively calls itself on each data property.

[stanislav-halyn]

I will prepare a PR since the fix seems quite straightforward: add a visited WeakSet to check if we have processed the item already.

[TkDodo]

maybe we need to make it more obvious in the docs, but for structuralSharing to work, data must be JSON-serializable.

if that's not the case, you can:

• turn off structuralSharing by setting it to false
• provide your own method to structuralSharing to share data yourself.

add a visited WeakSet to check if we have processed the item already.

I think this is something you would do in a custom structuralSharing function. It's a conscious decision to only work with JSON serializable data on our end.

[TkDodo]

however, I think the query should go into error state if that happens. I think that might be the real issue ?

[stanislav-halyn]

@TkDodo yeah, I think it should, because it took me some time to figure out what was the issue with the query, since I didn't know my data wasn't JSON-serializable

[NikSimonov]

got stuck with this problem for a long time too, would be good to get error in this case

[TkDodo]

however, I think the query should go into error state if that happens. I think that might be the real issue ?

okay, does someone want to contribute this ? I think a try/catch around here:

query/packages/query-core/src/query.ts

Line 499 in e3240f0

this.setData(data)

• calling onError in the catch should do it

[tigerabrodi]

@TkDodo

Anyone on this?

I think I'm encountering the same issue.

For me it gets stuck in "fetching", this happens consistently.

[TkDodo]

I think you can feel free to work on it. Putting the query in error state won't fix the problem for you though. What you likely want is to set structuralSharing: false for a Query that has circular dependencies in its data.

[RomainLanz]

Hey there! 👋🏻

I’m experiencing a similar issue that might be related to this one.

In my case, some queries remain in the Fetching status even after the queryFn callback has completed. As shown in the video, you can see that I've received, parsed, and returned my data successfully (indicated by the Done log), yet the status doesn’t update.

CleanShot.2024-08-21.at.14.30.39.mp4

After running a profiler on the code, I noticed that the replaceEqualDeep function is taking approximately 6.4 seconds to complete, which might be contributing to the issue.

Here is my query definition, which is relatively simple:

export function useUserList(params?: UserListParams) {
  const { filters = {} } = params ?? {};
  const queryKey = userKeys.list(filters);
  const queryClient = useQueryClient();

  return useQuery({
    queryKey,
    queryFn: async () => {
      const oldData = queryClient.getQueryData<Awaited<ReturnType<typeof fetchUserList>> | undefined>(queryKey);

      const { data, version } = await fetchUserList({
        ...filters,
        Version: oldData?.version ?? 0,
      });

      const mergedData = mergeData(oldData?.data, data);

      return { data: mergedData, version };
    },
  });
}

My fetcher function looks like this:

export async function fetchUserList(filters: UserFilters) {
  const request = GetUserList.clone();

  // ...

  const response = await http.send(request, { version: APIVersion.VIS2009 });
  const data = UserListSchema.parse(response.data);

  return {
    data: indexOnce(data),
    version: response.version,
  };
}

However, I don't see where structuralSharing or cycles might be occurring within my data but when setting structuralSharing to false, the issue disappears.

I'm happy to provide more context or information if needed. Thanks for looking into this!

[RomainLanz]

After revisiting the documentation on structuralSharing, I'm uncertain about its usefulness in a Vue context. The primary goal appears to be avoiding unnecessary data mutation or replacement, but it's unclear how this applies to Vue.

Additionally, I couldn't find any references to structuralSharing in the TanStack Query Vue Adapter documentation.

[TkDodo]

I don't know if structuralSharing is relevant in vue (cc @DamianOsipiuk). @RomainLanz does your issue go away if you turn it off? One idea would be to turn it off per default in vue, but that seems like a breaking change

[RomainLanz]

does your issue go away if you turn it off

Yes, it does.

I will turn it off globally in my QueryClient and report back if I see anything breaking. 👍🏻

[TkDodo]

the PR that adds error management by @robin4a4 looks good but also a bit stale:

• fix(query-core): add error management around this.setData(data) #7642

if someone wants to continue it, please do.

[RomainLanz]

does your issue go away if you turn it off

Yes, it does.

I will turn it off globally in my QueryClient and report back if I see anything breaking. 👍🏻

I didn't have anything breaking, so I will continue testing and push the change to production on Monday and report back again.

Linking related discussion for reference: #1709

[RomainLanz]

I didn't have anything breaking, so I will continue testing and push the change to production on Monday and report back again.

We pushed the change (defaulting to structuralSharing: false) in production on Monday and found no issue for the moment.

[RomainLanz]

I didn't have anything breaking, so I will continue testing and push the change to production on Monday and report back again.

We pushed the change (defaulting to structuralSharing: false) in production on Monday and found no issue for the moment.

We still haven't found any issue.

Should we consider removing structuralSharing by default for Vue users?
Maybe we can move this discussion somewhere else.