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

make -[ApolloStore clearCache] public #518

Merged
merged 3 commits into from
Jul 24, 2019
Merged

make -[ApolloStore clearCache] public #518

merged 3 commits into from
Jul 24, 2019

Conversation

RolandasRazma
Copy link
Contributor

@RolandasRazma RolandasRazma commented Apr 24, 2019

followup to #517

make -[ApolloStore clearCache] public.

At the moment there is no way to clear cache unless you have access to ApolloClient. Having clearCache() on ApolloClient is not exactly correct as multiple ApolloClient can share same cache.

make clearCache() public
@RolandasRazma RolandasRazma deleted the deprecate-ApolloClient-clearCache branch May 31, 2019 13:54
@RolandasRazma RolandasRazma restored the deprecate-ApolloClient-clearCache branch May 31, 2019 13:54
@RolandasRazma RolandasRazma reopened this Jun 5, 2019
@designatednerd
Copy link
Contributor

So I'm definitely in agreement with making clearCache on the Store public for the reasons you've outlined. However, I don't think we should deprecate clearCache on the client - many users will only be using a single client with a single cache, and for that use case leaving it on the client makes sense.

How about adding a note to the documentation that if you're using the same cache across multiple, you should call the method on the store instead?

@RolandasRazma
Copy link
Contributor Author

It's bit confusing if we leave it...

apolloClient.clearCache()
// do I need to call apolloClient.store.clearCache() as well?

@designatednerd
Copy link
Contributor

@RolandasRazma I think if you're only using one client and one cache (which is a fairly good chunk of our users), leaving it as a convenience would probably be better.

Having it exposed separately is helpful for the people who need it, but I think for people who only need the one client and the one cache, it's easier to leave the existing behavior and expose the underlying clear as a benefit for the people who need it.

@RolandasRazma
Copy link
Contributor Author

IMO it brings unnecessary confusion, but it's your call. Removed deprecation.

@RolandasRazma RolandasRazma changed the title make -[ApolloStore clearCache] public and deprecate -[ApolloClient clearCache] make -[ApolloStore clearCache] public Jul 24, 2019
@designatednerd
Copy link
Contributor

I'll update some of the docs once this is merged to try to make it clearer when to use which one - thank you!

Copy link
Contributor

@designatednerd designatednerd left a comment

Choose a reason for hiding this comment

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

Ready to go whenever Travis is

@designatednerd designatednerd added this to the 0.13.1 milestone Jul 24, 2019
@designatednerd designatednerd merged commit e62c1d9 into apollographql:master Jul 24, 2019
@designatednerd designatednerd modified the milestones: 0.13.1, 0.14.0 Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants