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

Supports "no-cache" fetchPolicy with pollInterval #10020

Merged

Conversation

MrDoomBringer
Copy link
Contributor

Fixes #9691
Fixes #6187

Adds support for the "no-cache" fetchPolicy to pollInterval.

Most fetchPolicies don't make sense to use when polling, as users probably don't want to be polling the cache directly. As such, currently polling will overwrite the fetchPolicy with "network-only".
However, "no-cache" is also useful for when the user wants to control whether or not their results are written to the cache, so this PR goes ahead and adds a check to allow that policy through, and a relevant test.

The check this adds looks similar to a fetchPolicy sieve in refetch(), and it might be worth looking into how to make that functionality a bit more generalized/cleaner.

I also went ahead and added a pollFetchPolicy type as a way to codify and clarify the rationale for this change, please let me know if I should change anything here!

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with some non-blocking suggestions for possible simplifications. Thanks @MrDoomBringer!

src/core/ObservableQuery.ts Outdated Show resolved Hide resolved
src/core/ObservableQuery.ts Outdated Show resolved Hide resolved
src/core/watchQueryOptions.ts Outdated Show resolved Hide resolved
Co-authored-by: Ben Asher <benasher44@gmail.com>
@jpvajda
Copy link
Contributor

jpvajda commented Sep 1, 2022

@MrDoomBringer @hwillson 👋 is there anything preventing us from merging this change in now?

Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great - thanks @MrDoomBringer!

@hwillson
Copy link
Member

hwillson commented Sep 1, 2022

@jpvajda I'm 👍 with merging. Thanks!

@MrDoomBringer MrDoomBringer merged commit fcd0452 into apollographql:release-3.7 Sep 1, 2022
@jpvajda
Copy link
Contributor

jpvajda commented Sep 1, 2022

Fixes #9691
Fixes #6187

viliket added a commit to viliket/live-trains-finland that referenced this pull request Nov 19, 2022
- Update apollo client to latest version to fix bug where "no-cache" does not work when pollInterval is set - see apollographql/apollo-client#10020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants