-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Prevent @client @export from firing unnecessary network requests #5946
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
hwillson
force-pushed
the
prevent-extra-client-export-requests
branch
3 times, most recently
from
February 14, 2020 19:48
f2783bd
to
d6d4641
Compare
benjamn
approved these changes
Feb 14, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, with one small opportunity to save some bytes.
PR #4604 fixed an issue where changes made to `@client @export` based variables didn't always result in re-running the query that was using the variable. Unfortunately, these changes are a bit too aggressive. They're triggering a `refetch` when an `@client @export` based variable changes (and a few additional conditions are met), and while using the `ObservableQuery.refetch()` method does fix the original issue, it leads to a situation where the query being re-run is always fired over the network, even if the updated query could have been resolved from the cache. `ObservableQuery.refetch()` forces queries to use a network based fetch policy, which means if the query was originally fired with a `cache-first` policy (for example), and has matching data in the cache after taking into consideration the new variable, that data won't be used and instead an extra unnecessary network request will fire. This commit addresses the issue by leveraging `ObservableQuery.setVariables()` instead of automatically refetching. `setVariables` will attempt to resolve the updated query from the cache first, then only fire it over the network if required.
hwillson
force-pushed
the
prevent-extra-client-export-requests
branch
from
February 15, 2020 18:30
d6d4641
to
86a1b25
Compare
hwillson
added a commit
that referenced
this pull request
Feb 23, 2020
The changes made in #5946 helped address an issue where queries with `@client @export` variables were making unnecessary network requests. While those changes work, they're currently preventing watched queries using `@client @export` from updating the `@export`ed variable, as new data is received. This commit fixes that issue.
hwillson
added a commit
that referenced
this pull request
Feb 23, 2020
The changes made in #5946 helped address an issue where queries with `@client @export` variables were making unnecessary network requests. While those changes work, they're currently preventing watched queries using `@client @export` from updating the `@export`ed variable, as new data is received. This commit fixes that issue.
hwillson
added a commit
that referenced
this pull request
Feb 24, 2020
The changes made in #5946 helped address an issue where queries with `@client @export` variables were making unnecessary network requests. While those changes work, they're currently preventing watched queries using `@client @export` from updating the `@export`ed variable, as new data is received. This commit fixes that issue.
hwillson
added a commit
that referenced
this pull request
Feb 25, 2020
The changes made in #5946 helped address an issue where queries with `@client @export` variables were making unnecessary network requests. While those changes work, they're currently preventing watched queries using `@client @export` from updating the `@export`ed variable, as new data is received. This commit fixes that issue.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
PR #4604 fixed an issue where changes made to
@client @export
based variables didn't always result in re-running the query that was using the variable. Unfortunately, these changes are a bit too aggressive. They're triggering arefetch
when an@client @export
based variable changes (and a few additional conditions are met), and while using theObservableQuery.refetch()
method does fix the original issue, it leads to a situation where the query being re-run is always fired over the network, even if the updated query could have been resolved from the cache.ObservableQuery.refetch()
forces queries to use a network based fetch policy, which means if the query was originally fired with acache-first
policy (for example), and has matching data in the cache after taking into consideration the new variable, that data won't be used and instead an extra unnecessary network request will fire.This commit addresses the issue by leveraging
ObservableQuery.setVariables()
instead of automatically refetching.setVariables
will attempt to resolve the updated query from the cache first, then only fire it over the network if required.