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

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

Closed
stanislav-halyn opened this issue Feb 22, 2024 · 15 comments · Fixed by #7966
Closed

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

stanislav-halyn opened this issue Feb 22, 2024 · 15 comments · Fixed by #7966
Labels
help wanted Extra attention is needed

Comments

@stanislav-halyn
Copy link

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
Copy link
Author

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
Copy link
Collaborator

TkDodo commented Feb 24, 2024

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

Screenshot 2024-02-24 at 11 24 34

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 TkDodo closed this as not planned Won't fix, can't repro, duplicate, stale Feb 24, 2024
@TkDodo
Copy link
Collaborator

TkDodo commented Feb 24, 2024

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

@stanislav-halyn
Copy link
Author

@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
Copy link

NikSimonov commented May 17, 2024

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

@TkDodo
Copy link
Collaborator

TkDodo commented May 17, 2024

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:

this.setData(data)

  • calling onError in the catch should do it

@TkDodo TkDodo reopened this May 17, 2024
@tigerabrodi
Copy link

tigerabrodi commented Jun 18, 2024

@TkDodo

Anyone on this?

I think I'm encountering the same issue.

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

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 18, 2024

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
Copy link

RomainLanz commented Aug 21, 2024

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
Copy link

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
Copy link
Collaborator

TkDodo commented Aug 21, 2024

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
Copy link

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
Copy link
Collaborator

TkDodo commented Aug 21, 2024

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

if someone wants to continue it, please do.

@RomainLanz
Copy link

RomainLanz commented Aug 22, 2024

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
5 participants