urql-graphql / urql

The highly customizable and versatile GraphQL client with which you add on features like normalized caching as you grow.
https://urql.dev/goto/docs
MIT License
8.54k stars 444 forks source link

fix(client): eager unblocking of in-flight operations #3573

Closed JoviDeCroock closed 2 months ago

JoviDeCroock commented 2 months ago

Resolves #3565

Summary

In the initial issue we were faced with the following problem, imagine a query that returns a list with values [Todo:1, Todo:2] we use this list to return components that in turn has its own query to fetch the details of this Todo. Both this list and the details have a shared parent field let's name it Query.author, this is important for graph-cache to re-dispatch operations.

This diagram of this looks like the following

Operation List
Completed Operation List
Operation Todo:1
Operation Todo:2
Completed Operation Todo:1
Operation Todo:2
Completed Operation Todo:2
Completed Operation Todo:2

Notice that we re-dispatch Todo:2 even though it is already in-flight, this is problematic as in the cache it has not resolved to a response yet and hence we send off multiple network requests. This issue becomes more and more apparent given more queries being in-flight. The lines causing this issue can be found here since there is no operation queued we will always remove the operation from our in-flight operations.

The solution here is to change this logic to only remove dispatched on a queued operation that either is already in dispatched or is network-only.

I did notice that https://github.com/urql-graphql/urql/pull/3564 already makes this issue less severe due to dispatching less operations from graphcache.

The above fix however re-introduces the issue we faced in https://github.com/urql-graphql/urql/issues/3254 this due to our optimistic mutation re-triggering the operation and resulting in stale and then our real data re-triggering the query but it already having been dispatched. This because we have similar logic in the onPush part of our operation. When it's stale we'll check the queue whether there is an operation in there, however there won't be as we are dealing with an optimistic mutation re-trigger, to fix this I added a check whether the stale result has a next payload coming, if not it gets deleted form dispatched either way.

I think the above stale flag fix is the root cause that needed to be addressed in #3254 but I am not a 100% sure as I got a bit overloaded while leveraging console.log in all of these reproductions 😅

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: b82e18477da7e683c42d4ecbcffcdfdcef9a6246

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ---------- | ----- | | @urql/core | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR