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 an open extensible class, and its previously-internal dependencies public #509

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lincolnq
Copy link

(This is in reference to apollographql/apollo-ios#3461, which I filed earlier today.)

The goal of this PR is: 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.

With this PR, it is still necessary to use @_spi(Execution) for overriding particularly fiddly bits of ApolloStore behavior. I think this is probably a good thing to help users understand that they're digging deep into the internals, while still allowing us to customize the behavior we want.

Feedback is welcome. In particular I'm not sure whether to do what Android does and make ApolloStore a protocol and rename the current ApolloStore to DefaultApolloStore. I can make that change too if desired.


Feel free to skip the below, but here's some more context about the why:

My specific use-case is one that has been touched-on/requested a number of places (apollographql/apollo-ios#3319, apollographql/apollo-ios#892, apollographql/apollo-ios#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.

This example is what I was trying to write, impossible (as far as I can tell) without making ApolloStore extensible. It's just an ApolloStore that passes .allowForOptionalFields to the GraphQLSelectionSetMapper during load:

import Foundation
import ApolloAPI
@_spi(Execution) import Apollo

public class MyApolloStore: ApolloStore {
    override open func load<Operation: GraphQLOperation>(
        _ operation: Operation,
        callbackQueue: DispatchQueue? = nil,
        resultHandler: @escaping GraphQLResultHandler<Operation.Data>
    ) {
        withinReadTransaction({ transaction in
            let (data, dependentKeys) = try transaction.readObject(
                ofType: Operation.Data.self,
                withKey: CacheReference.rootCacheReference(for: Operation.operationType).key,
                variables: operation.__variables,
                accumulator: zip(GraphQLSelectionSetMapper<Operation.Data>(handleMissingValues: .allowForOptionalFields),
                                 GraphQLDependencyTracker())
            )

            return GraphQLResult(
                data: data,
                extensions: nil,
                errors: nil,
                source: .cache,
                dependentKeys: dependentKeys
            )
        }, callbackQueue: nil, completion: resultHandler)
    }
}

Copy link

netlify bot commented Oct 16, 2024

👷 Deploy request for eclectic-pie-88a2ba pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 5b2f32b

Copy link

netlify bot commented Oct 16, 2024

👷 Deploy request for apollo-ios-docc pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 5b2f32b

@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented Oct 16, 2024

✅ Docs Preview Ready

No new or changed pages found.

@AnthonyMDev
Copy link
Contributor

Thanks for this PR! I haven't had time to review this in detail yet, but I wanted to let you know that I'm working on Apollo 2.0 to support the Swift 6 Concurrency Model, and one of the big features I'm currently planning on designing is this.

Once I have a better idea of the design, I'll be updating the 2.0 RFC with more information. I will definitely be looking at this PR in greater detail for inspiration as I iterate on the design for this in 2.0.

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.

3 participants