Support for changes in the codegen template

[martin-muller]

Use case

Thank you for the work on Apollo 1.0. We've been trying to update Apollo in our project to 1.0 and had some issues with the generated models.

In our project, we make heavy use of setters and inits on our GQL-generated models and this functionality missing in 1.0.x is prohibiting us from updating. We've considered changing our ways but decided not to, we don't see any benefit for our project and use cases.

Our use-cases:

1. inits
   • on fragments and selection sets for SwiftUI previews
2. setters
   • we often use the generated models in our views and logic directly, so considering we have a selectable list, we want to be able to flip isSelected flag on the generated model
3. cache
   • we have generic functions that mutate existing cache data or insert whole query data to the cache, and to do that, they require setters and inits on the models, some usages might be possible with the local cache mutation at the expense of convenience at point of use, but others, I believe, wouldn't be possible at all

Describe the solution you'd like

We've forked the apollo-ios project and re-added support for inits and setters, but I think such functionality might be useful to others and I'd like to ask for opinion about possible API's:

1. Flags on ApolloCodegenConfiguration.OutputOptions
2. Directives

query Cats @apollo_client_ios_init { // <- Codegen would generate an init for Cats's query data and all its children
  cats {
    name
    isHungry @apollo_client_ios_setter // <-  Codegen would generate setter for this field
  }
}

3. Allow specifying templates through ApolloCodegenConfiguration
   • Exposing the template building blocks and allowing to specify templates would allow adding setters and inits as well as adding imports and many useful changes without the need to fork

I think any of the options would be a great addition, and I'd be happy to give the implementation a go if you can see a place for this in the apollo-ios project.

[AnthonyMDev]

Thanks for your thoughts!

Initializers

I am currently working on generating initializers for the models. This is a lot more complicated than you would think, but it's in progress! You can check out #2551 to track this. It's planned for the 1.1 release.

Caching

The need to be able to create objects to add to the cache is one of the primary reasons why we are adding initializers to the generated models. This was definitely an oversight in 1.0 and we're working on correcting it for 1.1. But you are right that for your caching needs, some things may not be possible currently. Please hold tight, the generated local cache mutations will have initializers soon!

However, we still don't want to make the network operation models writable to the cache for a number of reasons. So you will need to migrate your cache mutations to use the new local cache mutations.

Mutable Models

We don't intend on making the generated models mutable (other than the local cache mutations). As noted in the docs here, we feel like it can lead to confusion. Though I do understand why this is appealing to you, especially when migrating an existing codebase that is already taking advantage of this from the 0.x version.

You are definitely welcome to continue with a fork of the codegen engine! Part of why we re-built it in Swift was to enable Swift developers to do this. But I would recommend you wrap our generated model in a view model that adds additional local fields, such as your isSelected field. You can use @dynamicMemberLookup to make this nice and clean! For example:

@dynamicMemberLookup struct MyViewModel {
  let _model: MyQuery.Data.MyModel
  var isSelected: Bool

  init(_ model: MyQuery.Data.MyModel) {
    self._model = model
    self._isSelected = model.isSelected
  }

  subscript<T>(dynamicMember keyPath: KeyPath<MyQuery.Data.MyModel, T>) -> T {
       _model[keyPath: keyPath]
  }
}

The subscript passes through all the other fields from the model that you don't specifically provide a new definition for on your ViewModel. If you need more information on @dynamicMemberLookup, check it out here!

This more clearly delineates the difference between a temporary, in-memory view model and the underlying data model which represents the immutable response data from an operation.

[martin-muller]

Great to see the progress on inits!

Im aware of the reasoning in docs, but don't believe it's applicable to our team and project.

If our cache mutation models were mutable, mutating them outside of a ReadWriteTransaction wouldn't persist any changes to the cache.

I wonder where does this assumption comes from, was this an issue in the past? I've never encountered someone mutating an object outside of a save context/closure or not followed by some save mechanism and expecting the data to be saved in a cache or db.

Additionally, mutable data models require nearly double the generated code. By maintaining immutable models, we avoid this confusion and reduce our generated code.

The amount of generated code will depend on project's needs, if I need to mutate every single field, the 1.0 codegen will increase the amount of our generated code. Additionally, mutable models are generated from scratch while adding setters requires one line per field, so adding just a few local cache mutations can erase the lines saved by not generating setters.

Looking at the trade-offs, I'm struggling to see why this solution was chosen without providing an alternative.

You can use @dynamicMemberLookup to make this nice and clean!

Sure we can do that, but why complicate things that can be easy 🙂. It also further decreases the amount of lines saved by not generating setters.

This more clearly delineates the difference between a temporary, in-memory view model and the underlying data model which represents the immutable response data from an operation.

The clarity of code is in the eyes of beholder. I think there are cases where it would be beneficial to not have any differences between models, but that's no longer possible because the 1.0 asserts there is a difference.

I think the move to Swift templates is great, but I also think the need to fork is pretty big barrier for entry. Is allowing user defined templates without forking not a priority, or is it something you just don't want to allow?

[yonaskolb]

I would echo everything @martin-muller said regarding setters and real world flexibility. I haven't quite understood the reasoning for removing the setters either. Having said that, very much appreciate all the work that's gone into this 1.0 update 🙏

[AnthonyMDev]

It sounds like most of the feedback in this issue has been addressed with generated initializers.

As for the requests for mutable models, we continue to have internal conversations about this as a feature. We may revisit this again in the future, but for the time being, what we've decided is this:

You can currently make a Named Fragment mutable with @apollo_ios_local_cache_mutation and then use that fragment inside of an immutable operation. This should allow you to access that fragment and mutate the fields in it.

We think that this should serve users purposes for now. Rather than making an option for making operations completely mutable, we are considering iterating on that functionality with an improved API and documenting it with usage examples as the correct way to handle this. It allows you to selectively make mutating parts of an operation possible without using the sledgehammer of making everything mutable all the time.

Going forward, we probably want to make a different directive name that is more descriptive of what that does, and maybe make some better API around how you can access and mutate those fields. But for the time being, using this pattern does allow you to workaround this "limitation" (in quotes because it was an intended limitation).

Closing this issue and making a new one to track that future feature work. #3011