Skip to content

Support generating Encodable types #728

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Support generating Encodable types #728

wants to merge 4 commits into from

Conversation

gsabran
Copy link

@gsabran gsabran commented Aug 13, 2025 •
edited
Loading

Support generating Encodable types:

  • make DataDict conform to Encodable
  • add an opt-in option in the codegen config to make the types conform to Encodable (there is no code to generate other than adding the protocol conformance) since the implementation is provided by an extension.

Motivation

In some contexts, using the normalized cache adds a lot of overhead, including large cache size. When that's the case, it is desirable to be able to manage the persistence of Apollo types more directly, and Codable is the standard protocol to do so. This PR provides a solution for the serialization, and possibly for the deserialization when including Apollo as a dependency.

Why not Decodable ?

I think types can be decoded from a JSON value using

@_spi(Execution) import Apollo

try GraphQLExecutor(executionSource: NetworkResponseExecutionSource()).execute(
  selectionSet: Type.self,
  on: dataEntry, // JSONObject
  withRootCacheReference: nil,
  variables: nil,
  accumulator: GraphQLSelectionSetMapper<Type>()
)

which in my understanding has the benefit of going through all the Apollo machinery to validate the data, detect fragment conformance etc. If this understanding is correct this is great. However this code depends on Apollo, not ApolloAPI so this is not something the generated code can depend on.

@netlify Netlify
Copy link

netlify bot commented Aug 13, 2025

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

Visit the deploys page to approve it

Name Link
🔨 Latest commit f9fbc47

@gsabran
Copy link
Author

gsabran commented Aug 13, 2025

In addition to feedback about the goal of this change, and its implementation, I was curious if there already exist:

  • snapshot testing of generated code
  • tests that use generated types and validate their behavior (this would be quite useful to validate the encoding/decoding behavior of such generated types)

@AnthonyMDev
Copy link
Contributor

Thanks so much for this PR @gsabran. We've talked about this feature in the past and had decided not to implement it. But it's been requested a number of times, and with a working implementation already proved out, I think it deserves re-consideration.

The team will discuss this and get back to you!

@gsabran
Copy link
Author

gsabran commented Aug 13, 2025

Thank you. Yes I remember reading something about why it was rejected in the past (but I could not find a pointer for this PR). My recollection was that this was because Apollo's normalized cache is the recommended way to do storage, and that when using it there is no need to manage serialization.

I'm working on a project where the normalized cache is not scaling well and I would prefer to have the ability to do this myself.

I'm not entirely sure this implementation works on all cases (would love some feedback on that or how to better test), but one nice aspect is that it required very few changes to the generated code as we get Encodable for free as long as we can conform DataDict to it.

@AnthonyMDev
Copy link
Contributor

The team spoke about this today. We do want to accept an implementation of this, but there are both some things we think could be simplified.

We could simplify the implementation by just making SelectionSet conform to Encodable, then you don't need any changes to the codegen templates. This doesn't need to be an opt-in feature with a codegen option if we approach it that way.

My big concern on the other hand, is that if we ship this, people are going to start asking for a Decodable implementation as well. But that has some unresolved issues that add a lot of complexity. 😅

As you identified, the correct way to do that would be by running the GraphQLExecutor over the data in order to create properly formed and validated models. This is located in Apollo not ApolloAPI, but I think it would be fine to just include the Encodable and Decodable implementations in Apollo as extensions.

However I can't figure out how to overcome the issue of the input variables. When working with any operations that have @include/@skip directives, we need to have the variable values input into the operation in order to properly run GraphQL execution and ensure the values are mapped over properly.

The created DataDict doesn't hold on to the variable values, it just uses them to determine which fragments should be fulfilled. The DataDict includes the private properties fulfilledFragments and deferredFragments which are computed based on these variables (and some other execution logic for deferred fragments).

If we were to include the values of the fulfilledFragments and deferredFragments properties in the encoded response, we would then be able to decode the same models just fine. But that makes the encoded output much more directly tied to our generated models and not just a JSON representation of the values. This approach is probably more aligned with how Codable is meant to work.

If what you want is just a JSON representation to write to a store, your implementation can do that, it just can't properly re-create the generated models in some cases (when using @include/@skip or @defer).

@gsabran
Copy link
Author

gsabran commented Aug 28, 2025

Thanks, all those points make sense. I'll defer to you and the team on what you think are the best trade offs here.

Maybe worth noting: it is possible to pass some configuration flag through the encoder's userInfo, so you could decide that that the params should be included as extra JSON params or not as a default, and let the option for the developper to chose otherwise through such configuration.

I was made aware that the implementation in this PR has issues and needs more work.

I was wondering if within your tests you have something like

  • set of pre-defined graphql definitions that cover desired test cases.
  • Swift types generated from those definitions that are kept in sync as the codegen evolves
  • tests that validate how those generated types behave for different json inputs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gsabran @AnthonyMDev