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

Making ApolloStore extensible #3461

Open
lincolnq opened this issue Oct 16, 2024 · 4 comments
Open

Making ApolloStore extensible #3461

lincolnq opened this issue Oct 16, 2024 · 4 comments
Labels
feature New addition or enhancement to existing solutions

Comments

@lincolnq
Copy link

lincolnq commented Oct 16, 2024

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

  • Mark ApolloStore open and make more of its functions public/open
    • 4-argument ReadTransaction.readObject
  • Make its implementation dependencies public (gated with @_spi(Execution) as appropriate):
    • DispatchQueue extensions (performAsyncIfNeeded)
    • GraphQLDependencyTracker
  • Maybe even: define ApolloStore's public interface as a protocol called ApolloStore. Rename the current implementation to NormalizedCacheApolloStore and instantiate it wherever ApolloStore is currently instantiated.

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

@lincolnq lincolnq added the feature New addition or enhancement to existing solutions label Oct 16, 2024
@calvincestari
Copy link
Member

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//issues/3319, https://github.com//issues/892, https://github.com//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
Copy link
Member

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

@lincolnq
Copy link
Author

lincolnq commented Oct 16, 2024

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
Copy link
Author

Ok, here's the PR: apollographql/apollo-ios-dev#509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New addition or enhancement to existing solutions
Projects
None yet
Development

No branches or pull requests

2 participants