-
-
Notifications
You must be signed in to change notification settings - Fork 454
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
fix(client): eager unblocking of in-flight operations #3573
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: b82e184 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
JoviDeCroock
changed the title
present fix for subsequent operations
fix(client): eager unblocking of in-flight operations
Apr 24, 2024
JoviDeCroock
force-pushed
the
fix-subsequent-operations
branch
from
April 24, 2024 06:29
1bc42ba
to
e992f40
Compare
JoviDeCroock
commented
Apr 24, 2024
JoviDeCroock
force-pushed
the
fix-subsequent-operations
branch
3 times, most recently
from
April 24, 2024 08:21
4e88864
to
fec9546
Compare
JoviDeCroock
force-pushed
the
fix-subsequent-operations
branch
from
April 24, 2024 08:26
fec9546
to
8b070db
Compare
JoviDeCroock
commented
Apr 24, 2024
kitten
reviewed
Apr 24, 2024
kitten
approved these changes
Apr 24, 2024
Co-authored-by: Phil Pluckthun <phil@kitten.sh>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 thisTodo
. Both this list and the details have a shared parent field let's name itQuery.author
, this is important for graph-cache to re-dispatch operations.This diagram of this looks like the following
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 indispatched
or isnetwork-only
.The above fix however re-introduces the issue we faced in #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 theonPush
part of ouroperation
. 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 leveragingconsole.log
in all of these reproductions 😅