apollographql / apollo-ios

📱  A strongly-typed, caching GraphQL client for iOS, written in Swift.
https://www.apollographql.com/docs/ios/
MIT License
3.89k stars 730 forks source link

Making ApolloStore extensible #3461

Open lincolnq opened 1 month ago

lincolnq commented 1 month ago

Use case

Goal: Make it easier to customize behavior of the Apollo normalized cache.

Currently, ApolloStore is a concrete class depended-on by lots of Apollo internals. It's not marked open, nor is it a protocol, so in practice there is no way without modifying Apollo code for applications to customize the store behavior.

[context: My specific use-case is one that has been touched-on/requested a number of places (#3319, #892, #3216: make the Apollo cache accept missing values for nullable fields as null, rather than failing to parse them.) While Apollo team has resisted adding a flag, if ApolloStore were extensible I could implement the behavior I want myself without forking all of apollo-ios.]

I would be fine if it were necessary to use @_spi(Execution) to override some ApolloStore behavior, flagging to end-users that the APIs may change. I just want to be able to override stuff without forking the entire repo.

Describe the solution you'd like

I can work on this but wanted to check with the team about whether it is likely to be accepted.

calvincestari commented 1 month ago

HI @lincolnq 👋🏻 - we have discussed the extensibility of ApolloStore and I don't think we'd be opposed to opening it up BUT that will not achieve what you want here.

[context: My specific use-case is one that has been touched-on/requested a number of places (https://github.com/apollographql/apollo-ios/issues/3319, https://github.com/apollographql/apollo-ios/issues/892, https://github.com/apollographql/apollo-ios/issues/3216: make the Apollo cache accept missing values for nullable fields as null, rather than failing to parse them.) While Apollo team has resisted adding a flag, if ApolloStore were extensible I could implement the behavior I want myself without forking all of apollo-ios.]

It is not the cache, nor the store implementation, that is rejecting missing response values. You can test this yourself by using a request cache policy to completely ignore the cache. The operation will still fail if there are missing response values. The logic for this behaviour lives within our GraphQL executor and it is built to be compliant with the GraphQL specification.

calvincestari commented 1 month ago

What was the implementation you were thinking you could achieve this? What additional logic would your store have?

lincolnq commented 1 month ago

Ok thanks - I'm working on a PR, but I do have a local demo that does what I want overriding ApolloStore to specify handleMissingValues: .allowForOptionalFields on the load operation. To be clear: what I want is a relatively narrow subset of that which was mentioned in the linked issues (specifically loading allowing missing values to be filled by nulls when loading from the cache, not the server).

My local demo requires importing Apollo as @testable @_spi(Execution) in order to override the relevant functionality, and also requires replacing the CacheReadInterceptor with a custom one that uses my own proxied ApolloStore.

lincolnq commented 1 month ago

Ok, here's the PR: https://github.com/apollographql/apollo-ios-dev/pull/509