Why were GraphQLOperations changed from final to non-final?

[Mordil]

Question

During our migration from 0.36 to 1.3, we noticed that some of our code no longer compiles due to the final modifier being dropped from the generated GraphQL operation classes.

protocol SupportedOfflineMutation {
  init?(variables: GraphQLOperation.Variables)
}

extension UpdateBookingPrimaryGuestMutation: SupportedOfflineMutation {
  public var bookingID: String { self.inputParams.id }
  public var requestType: OfflineRequestType { .standard(self) }
  public var variables: GraphQLOperation.Variables? { self.__variables }

  public convenience init?(variables: GraphQLOperation.Variables) {
     ^^^^^^^^^^^^^^^
        Initializer requirement 'init(variables:)' can only be satisfied by a 'required' initializer in the definition of non-final class 'UpdateBookingPrimaryGuestMutation'
  }

Was this intentional, or a miss in parity during the re-write of the engine?

Would it be reasonable to at least offer this as a codegen option?

[AnthonyMDev]

This was definitely just an oversight. I will add this to our backlog, but if you would like it completed sooner, we are always happy to accept a PR.

Since I don't think we need to add it as a new codegen option, this is actually a pretty simple PR to make. The templates and the unit tests just need to be modified.

[Mordil]

Great to hear - I'll prep a PR tonight.

I think I disagree on the need for making it an option, because people may have use cases where they might want to subclass the operations for whatever reason - but also chiefly because this would be a breaking change to introduce, as people's models will work differently than before with no way of going back to the old behavior.

Perhaps in 2.x the option can be removed, but I think for a backwards-compatible release it would need to be configurable.

[jimisaacs]

We are actually taking advantage of it not being final.

[AnthonyMDev]

Thanks for letting us know that @jimisaacs! The PR #3189 that has been merged makes this a configuration option. It will be in the next release and won't change anything unless you opt in.

[jimisaacs]

Nice, sounds great. Thanks for the context, I just saw this go by on my feed so commented ;)

[calvincestari]

@jimisaacs, could I dig deeper into what you're doing in the subclasses? If it's purely app-side logic then I guess not much for us to do but if there is something we could do in the generated models to better facilitate what you're doing let's explore it.

[jimisaacs]

@calvincestari sure, the use case is because we have query that is fetched in two contexts, in one of those contexts, it passes a particular parameter, while it is not in the other context. That parameter should not be used in the generation of a cache key, and we didn't have a better way to manipulate that process.

So we ended up with a protocol that looks like this:

public protocol CustomCacheOperation {
    func operationForCache() -> Self
}

where operationForCache() returns an instance of an operation that should be used in a custom cache write interceptor, to actually use it in the process of writing to cache. The operation returned is ensured to NOT have that parameter previously mentioned, even though it may or may not have been passed by the client consumer.

This CustomCacheOperation protocol is then conformed to by subclassing the generated query.