Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

fix: fetchPolicy support for Subscriptions #2298

Merged

Conversation

MatthieuLemoine
Copy link
Contributor

@MatthieuLemoine MatthieuLemoine commented Aug 22, 2018

Hi! 馃憢

Apollo Client supports fetchPolicy for subscriptions but the fetchPolicy option is not passed to the client in react-apollo.

It can be useful when using no-cache to avoid automatic cache update & handle the update manually using the new onSubscriptionData.

This PR just forwards the fetchPolicy option to apollo-client.

PS: It could be nice to add some kind of note next to a feature in the documentation with the minimum version number needed to use it. Actually, I tried to set fetchPolicy to no-cache because I thought that I could use onSubscriptionData to update the cache but it seems that it was only merged yesterday.

@apollo-cla
Copy link

@MatthieuLemoine: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@hwillson
Copy link
Member

PS: It could be nice to add some kind of note next to a feature in the documentation with the minimum version number needed to use it. Actually, I tried to set fetchPolicy to no-cache because I thought that I could use onSubscriptionData to update the cache but it seems that it was only merged yesterday.

Yes, I agree this is annoying. This is caused by the fact that our docs are auto published after a docs related PR has been merged. This can happen at any point, and isn't tied to a specific release, which is definitely confusing. Honestly I think the best way to address this (and several other repo workflow issues) is to move away from our current master <-> PR branch workflow, to a master <-> devel <-> PR branch workflow. We could then make sure production docs publishing only happens when devel merges into master, which would only happen when we publish a new release. More on this soon.

@WaldoJeffers
Copy link
Contributor

WaldoJeffers commented Aug 23, 2018

@hwillson This is a good idea, maybe the issue is less obvious for react-apollo maintainers, but it has happened to me a lot of times (seeing something on the docs, and digging through apollo-client/react-apollo's code only to find out the feature has not been released yet).

@hwillson hwillson self-assigned this Sep 26, 2018
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.

@hwillson hwillson merged commit e6fe307 into apollographql:master Oct 2, 2018
hwillson added a commit to apollographql/apollo-client that referenced this pull request Oct 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants