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.

Sign up for GitHub

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

Jump to bottom

watchQuery defaultOptions are unset when query variables change (specifically nextFetchPolicy) #6839

Closed
Tracked by #8465
jebonfig opened this issue Aug 14, 2020 · 4 comments · Fixed by #8465
Closed
Tracked by #8465

watchQuery defaultOptions are unset when query variables change (specifically nextFetchPolicy) #6839

jebonfig opened this issue Aug 14, 2020 · 4 comments · Fixed by #8465
Assignees
@benjamn
Labels
🐞 bug 🛬 fixed-in-prerelease 🌹 has-reproduction
Milestone
Post 3.0

Comments

@jebonfig
Copy link

Intended outcome:

The defaultOptions.watchQuery.nextFetchPolicy setting should always be applied for the query, even if the query variables change. I am setting it to cache-only to retain the behavior from #6353

Actual outcome:

When the query variables change, the defaultOptions are unset. This results in unexpected network calls after cached data changes.

How to reproduce the issue:

Simple repro here: https://github.com/jebonfig/react-apollo-error-template

Versions

  System:
    OS: macOS High Sierra 10.13.6
  Binaries:
    Node: 11.2.0 - /usr/local/bin/node
    npm: 6.4.1 - /usr/local/bin/npm
  Browsers:
    Chrome: 84.0.4147.125
    Safari: 13.1.2
  npmPackages:
    @apollo/client: ^3.1.3 => 3.1.3
@benjamn benjamn added this to the Post 3.0 milestone Aug 17, 2020
@benjamn benjamn self-assigned this Aug 17, 2020
@benjamn
Copy link
Member

benjamn commented Aug 17, 2020

This does seem important, and not just for nextFetchPolicy (though see #6833 (comment) for some other ideas about that).

@benjamn benjamn mentioned this issue Aug 17, 2020
Merged
@dylanwulf
Copy link
Contributor

@benjamn Hi, I hate to be the person to post a "+1 me too", but this issue has been out here for 8 months now and is still a problem. It looks like @jebonfig has updated their reproduction to use the new nextFetchPolicy function, but it still gets unset whenever the variables change.
Also just want to say thank you for all your hard work; I've seen how many issues get posted on this repo every day and it must be difficult keeping up with all of them.

@dylanwulf dylanwulf mentioned this issue Jun 28, 2021
Closed
benjamn added a commit that referenced this issue Jul 7, 2021
@benjamn
Avoid deleting options.nextFetchPolicy after applying it.
7698757 
Despite my vague suggestion in the removed comments that deleting
options.nextFetchPolicy somehow helped make the fetchPolicy transition
idempotent (a concern that only matters when nextFetchPolicy is a
function), leaving options.nextFetchPolicy intact should also be
safe/idempotent in virtually all cases, including every case where
nextFetchPolicy is just a string, rather than a function.

More importantly, leaving options.nextFetchPolicy intact allows it to be
applied more than once, which seems to fix issue #6839.
@benjamn benjamn mentioned this issue Jul 7, 2021
Merged
3 tasks
benjamn added a commit that referenced this issue Jul 8, 2021
@benjamn
Avoid deleting options.nextFetchPolicy after applying it.
9e6f7fd 
Despite my vague suggestion in the removed comments that deleting
options.nextFetchPolicy somehow helped make the fetchPolicy transition
idempotent (a concern that only matters when nextFetchPolicy is a
function), leaving options.nextFetchPolicy intact should also be
safe/idempotent in virtually all cases, including every case where
nextFetchPolicy is just a string, rather than a function.

More importantly, leaving options.nextFetchPolicy intact allows it to be
applied more than once, which seems to fix issue #6839.
benjamn added a commit that referenced this issue Jul 9, 2021
@benjamn
Avoid deleting options.nextFetchPolicy after applying it.
add15f5 
Despite my vague suggestion in the removed comments that deleting
options.nextFetchPolicy somehow helped make the fetchPolicy transition
idempotent (a concern that only matters when nextFetchPolicy is a
function), leaving options.nextFetchPolicy intact should also be
safe/idempotent in virtually all cases, including every case where
nextFetchPolicy is just a string, rather than a function.

More importantly, leaving options.nextFetchPolicy intact allows it to be
applied more than once, which seems to fix issue #6839.
@benjamn
Copy link
Member

benjamn commented Jul 9, 2021

@jebonfig Please try @apollo/client@3.4.0-rc.18 when you have a chance. Sorry for the wait!

@benjamn benjamn linked a pull request Jul 9, 2021 that will close this issue
Improve processing of options.nextFetchPolicy #8465
Merged
3 tasks
@jebonfig
Copy link
Author

Thanks @benjamn! This is fixed on 3.4.0.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@benjamn benjamn

Labels
🐞 bug 🛬 fixed-in-prerelease 🌹 has-reproduction
Projects
None yet
Milestone
Post 3.0
Development

Successfully merging a pull request may close this issue.

Improve processing of options.nextFetchPolicy apollographql/apollo-client
3 participants
@benjamn @jebonfig @dylanwulf