apollo-ios icon indicating copy to clipboard operation
apollo-ios copied to clipboard

Generated enum does not seem to conform to JSONDecodable

Open AlexLittlejohn opened this issue 2 years ago • 11 comments

Summary

A generated enum conforming to EnumType fails to deserialize since it doesn't conform to JSONDecodable. This causes the deserialisation to fail with a precondition failure in this function.

  // GraphQLSelectionSetMapper.swift: 32-41

  func accept(customScalar: JSONValue, info: FieldExecutionInfo) throws -> JSONValue? {
    switch info.field.type.namedType {
    case let .customScalar(decodable as any JSONDecodable.Type):
      // This will convert a JSON value to the expected value type,
      // which could be a custom scalar or an enum.
      return try decodable.init(_jsonValue: customScalar)._asAnyHashable
    default:
      preconditionFailure()
    }
  }

Version

1.0.7

Steps to reproduce the behavior

A query with a returned field which is an enum value.

e.g.

// @generated
// This file was automatically generated and should not be edited.

import ApolloAPI

public extension GQL {
    enum RequestStatus: String, EnumType {
        case created = "CREATED"
        case pendingApproval = "PENDING_APPROVAL"
        case approved = "APPROVED"
        case rejected = "REJECTED"
        case booked = "BOOKED"
        // snip: more cases....
    }
}

I've tried adding manual conformance to my enum to allow it to be deserialised with the JSONDecodable protocol. I added this in the same module to which the generated code belongs in my codebase.

extension GQL.RequestStatus: JSONDecodable {
    @inlinable public init(_jsonValue value: JSONValue) throws {
        let rawValue = try RawValue(_jsonValue: value)
        if let tempSelf = Self(rawValue: rawValue) {
            self = tempSelf
        } else {
            throw JSONDecodingError.couldNotConvert(value: value, to: Self.self)
        }
    }
}

This approach did not work though.

Logs

Apollo/GraphQLSelectionSetMapper.swift:39: Fatal error

Anything else?

The object in question at the precondition failure

AlexLittlejohn avatar Feb 16 '23 15:02 AlexLittlejohn

Thanks for the detailed report @AlexLittlejohn! I'm a little confused as to why this is happening. I can see in the debugger that the correct OutputType for the field is being used, which is GraphQLEnum<DMLClient.GQL.RtbRequestStatus>. The generated EnumType itself does not conform to JSONDecodable, but GraphQLEnum<T> does. GraphQLEnum<T> is a wrapper that is used with all enum values in the generated models.

Can you set a breakpoint before the crash and show me we that type of info.field.type.namedType is just to verify? That type should be the GraphQLEnum<DMLClient.GQL.RtbRequestStatus>, which should be able to be cast as JSONDecodable.

AnthonyMDev avatar Feb 17 '23 18:02 AnthonyMDev

As requested, here is a screenshot of the output with a breakpoint on that line.

output

Let me know what other info you may need

AlexLittlejohn avatar Feb 20 '23 12:02 AlexLittlejohn

Thanks! I'll be looking into this more ASAP!

AnthonyMDev avatar Feb 20 '23 20:02 AnthonyMDev

@AlexLittlejohn I'm not able to reproduce this issue yet. In all my tests GraphQLEnum<DMLClient.GQL.RtbRequestStatus> is able to cast to any JSONDecodable just fine.

I'm wondering if there is some odd linking issue in your project that is causing the runtime to not be able to cast this successfully. Can you give me more info about your environment?

What version of the Swift Language and Xcode are you using. What platform are you compiling for? Is this occurring in both release (optimized) and debug (no optimization) builds?

I'm not sure what situation would allow the code to even compile and not understand this type casting. GraphQLEnum has a public conformance to JSONDecodable that lives in ApolloAPI. It makes no sense to me. 😕

Let's keep digging and get to the bottom of this one.

AnthonyMDev avatar Feb 21 '23 19:02 AnthonyMDev

Sure, a few details,

  • Xcode 14.2
  • Swift 5.7
  • iOS only
  • Apollo linked via SPM
  • Compiling for debug (will run a test on prod)

Linked SPM packages to GraphQL client module Screenshot 2023-02-22 at 10 49 31

If you'd like I can share more full code sections with you directly

AlexLittlejohn avatar Feb 22 '23 10:02 AlexLittlejohn

I'm still really stumped by this one. I don't imagine you'd be able to get a reproduction case together for me on this one?

The only other thing I can think of is if you have a protocol named EnumType in your project and the GQL.RequestStatus is conforming to that instead of ours. Could you try changing the generated code to explicitly namespace that conformance?

So try:

public extension GQL {
    enum RequestStatus: String, ApolloAPI.EnumType {
        case created = "CREATED"
        case pendingApproval = "PENDING_APPROVAL"
        case approved = "APPROVED"
        case rejected = "REJECTED"
        case booked = "BOOKED"
        // snip: more cases....
    }
}

We probably should already be generating that with the namespace honestly.

AnthonyMDev avatar Feb 28 '23 20:02 AnthonyMDev

@AnthonyMDev , @calvincestari i'm creating a reproduction case for you. Just trying to trim as much fat as possible

AlexLittlejohn avatar Mar 02 '23 09:03 AlexLittlejohn

@AnthonyMDev , we figured out and resolved the issue.

It was a linker issue rather than a code gen issue. Essentially,Apollo and ApolloAPI was only linked to our generated code module. As shown above, when we ran the network request in a feature module it failed to deserialise the models correctly. However, when I linked the Apollo SPM packages directly to the feature module it resolved the issue.

I'm not sure what the learnings are here except to say that linking is weird 😸

AlexLittlejohn avatar Mar 02 '23 10:03 AlexLittlejohn

This sounds like a very similar linking issue to https://github.com/apollographql/apollo-ios/issues/2686 and https://github.com/apollographql/apollo-ios/issues/2813. I'm not sure what action we could take in these cases.

calvincestari avatar Mar 02 '23 19:03 calvincestari

Thanks so much for following up on this! Glad to hear you are able to move forward, but this is definitely odd behavior. My understanding is that your feature module should not have to be linked against ApolloAPI directly for this to work.

We should definitely take a closer look at this. This could be confusing to a lot of people, and the intention was that you should be able to link only the generated models to ApolloAPI. I'll see if I can reproduce this and come up with a better way to handle this at some future date.

Note to self: Explore using @_alwaysEmitConformanceMetadata to see if we can overcome this linker issue.

AnthonyMDev avatar Mar 03 '23 20:03 AnthonyMDev

I was able to create a reproduction of this now. I tried using @_alwaysEmitConformanceMetadata and that did not solve the issue. Looks like currently, linking to ApolloAPI directly is the only workaround. Hoping we can resolve this better in the future.

AnthonyMDev avatar Mar 31 '23 20:03 AnthonyMDev