Remove local cache mutation type case setter

[calvincestari]

Local cache mutations are mutable and the properties have both get and set accessors generated. This is fine for scalar and entity type properties but causes a problem with type conditions.

The problem is that the setter for the type condition is only changing the internal Dictionary (_data) value but none of the metadata (_fulfilledFragments and _deferredFragments) needed during execution. The result is that if a selection set is instantiated and then the type condition setter overwrites the underlying data, committing that data to the cache often results in errors when execution is performed.

Is this a breaking change?
The correct way to initialize a selection set through a type condition is to use the asRootEntityType property which correctly sets the underlying DataDict which in turn handles the internal Dictionary storage and the associated execution metadata. This exists already and is documented.

The existing type case setters are causing unintended runtime errors and were not intended to be used this way therefore I do not think removing them should be considered a breaking change.

[Iron-Ham]

Currently exploring updating our version of Apollo – and this seems like a rough change for us.

Using a small mutation in our codebase as an example. For context to other readers – we have n schemas that we support, where each can be thought of as a time-frozen version of the current schema. We generate our own existential protocols unifying all Apollo outputs.

Before:

        let mutation = client.makeMutableNotificationThreadCommonFieldsFragmentTransaction(
            cacheKey: "NotificationThread:\(id)",
            variables: ["avatarSize": InboxCell.iconSize()]) { data in
                data.isArchived = isDone
                data.isUnread = !isRead
                data.isSaved = isSaved
                if let state = subscribeState?.toType {
                    data.listType.asSubscribableType?.viewerSubscriptionType = state
                    let isSubscribed = SubscriptionStateType.subscribed(notification: data.subscriptionStatusType)
                    data.subscriptionStatusType = .case(isSubscribed ? .unsubscribed : .threadSubscribed)
                }
            }
        client.readWriteStore(body: mutation)

After:

        let mutation = client.makeMutableNotificationThreadCommonFieldsFragmentTransaction(
            cacheKey: "NotificationThread:\(id)",
            variables: ["avatarSize": InboxCell.iconSize()]) { data in
                data.isArchived = isDone
                data.isUnread = !isRead
                data.isSaved = isSaved
                let isSubscribed = SubscriptionStateType.subscribed(notification: data.subscriptionStatusType)
                data.subscriptionStatusType = isSubscribed ? .unsubscribed : .threadSubscribed
                if let subscriptionState = subscribeState?.toType,
                    let newList = listType(for: subscriptionState, originalList: data.listType) {
                    data.listType = newList
                }
            }
        client.readWriteStore(body: mutation)

        func listType(for subscriptionState: SubscriptionStateType, originalList: any MutableNotificationThreadCommonFieldsFragmentListType) -> (any MutableNotificationThreadCommonFieldsFragmentListType)? {
            switch graphQLClient.schemaIdentifier {
            case .dotcom:
                if let asSubscribableType = originalList.asSubscribableType, let subscribeState = subscriptionState.graphQLDotcomType {
                    GraphQLDotcom.MutableNotificationThreadCommonFieldsFragment.List.AsSubscribable(
                        __typename: asSubscribableType.__typename ?? "",
                        id: asSubscribableType.id,
                        viewerSubscription: .case(subscribeState)
                    ).asRootEntityType
                } else {
                    nil
                }
            case .ghes38:
                if let asSubscribableType = originalList.asSubscribableType, let subscribeState = subscriptionState.graphQLGHES38Type {
                    GraphQLGHES38.MutableNotificationThreadCommonFieldsFragment.List.AsSubscribable(
                        __typename: asSubscribableType.__typename ?? "",
                        id: asSubscribableType.id,
                        viewerSubscription: .case(subscribeState)
                    ).asRootEntityType
                } else {
                    nil
                }
            case .ghes310:
                if let asSubscribableType = originalList.asSubscribableType, let subscribeState = subscriptionState.graphQLGHES310Type {
                    GraphQLGHES310.MutableNotificationThreadCommonFieldsFragment.List.AsSubscribable(
                        __typename: asSubscribableType.__typename ?? "",
                        id: asSubscribableType.id,
                        viewerSubscription: .case(subscribeState)
                    ).asRootEntityType
                } else {
                    nil
                }
            case .ghes312:
                if let asSubscribableType = originalList.asSubscribableType, let subscribeState = subscriptionState.graphQLGHES312Type {
                    GraphQLGHES312.MutableNotificationThreadCommonFieldsFragment.List.AsSubscribable(
                        __typename: asSubscribableType.__typename ?? "",
                        id: asSubscribableType.id,
                        viewerSubscription: .case(subscribeState)
                    ).asRootEntityType
                } else {
                    nil
                }
            }
        }

I think there's a few things highlighted in the code above – even in a case where you aren't operating over existentials (as we are), and that's that there's a certain ambiguity to the change being made now. The intent is to change a single value viewerSubscription. However, we have to instantiate a new object – and even passing values from the existing object results in needing to nil-coalesce. It doesn't scale well, especially if you have multiple nested types of this nature in the same declaration.

For others that don't operate over existentials, an extension like this would prove sufficient:

extension ApolloAPI.MutableSelectionSet {
    public func with<T>(_ keyPath: WritableKeyPath<Self, T>, _ value: T) -> Self {
      var copy = self
      copy[keyPath: keyPath] = value
      return copy
    }
}

// usage:
let modifiedList = data.list.asSubscribable?
    .with(\.viewerSubscription, .case(state))
    .with(\.id, "123") // chainable
    .asRootEntityType
if let modifiedList {
    data.list = modifiedList
}

But in our case, the Self requirement in place there would stop us dead in our tracks; we'd need to specify a concrete type at some point (whereas in the past, we did not).

[calvincestari]

Hey @Iron-Ham, @AnthonyMDev and I are going to be talking about this stuff this morning. He filled me in yesterday about the problems it created for you.

[Iron-Ham]

Currently running this extension in my 1.7.1 Apollo codebase (haven't updated to a more recent version like 1.18 yet as there's the secondary issue of mis-matched typealias mutability that's tough to workaround).

extension ApolloAPI.MutableSelectionSet {

  @discardableResult
  public mutating func mutateIfPresent<T: ApolloAPI.InlineFragment>(
    _ kp: KeyPath<Self, T?>,
    _ body: (inout T) -> Void
  ) -> Bool {
    guard var fragment = self[keyPath: kp] else {
      return false
    }

    body(&fragment)
    self.__data = fragment.__data
    return true
  }

}

extension ApolloAPI.MutableSelectionSet where Fragments: FragmentContainer {
  @discardableResult
  public mutating func mutateIfPresent<T: ApolloAPI.Fragment>(
    _ kp: KeyPath<Self, T?>,
    _ body: (inout T) -> Void
  ) -> Bool {
    guard var fragment = self[keyPath: kp] else {
      return false
    }

    body(&fragment)
    self.__data = fragment.__data
    return true
  }

}

With this extension, I've been able to generate custom setters for these inline fragments, and thus workaround this issue entirely without needing to abuse the Swift type system.