Fix issue where CodableParsingInterceptor always fails#1646
Fix issue where CodableParsingInterceptor always fails#1646sharplet wants to merge 1 commit intoapollographql:mainfrom sharplet:codable-responses
Conversation
In practice, `GraphQLResult.init(from:decoder:)` would always fail when called by `CodableParsingInterceptor`. Its default implementation, when `Data` is not known to conform to `Decodable`, would unconditionally throw `ParseableError.unsupportedInitializer`. Because `CodableParsingInterceptor.interceptAsync` is a generic context where the operation's `Data` may not be `Decodable`, it would always select the default initialiser, even if a `Decodable` implementation is available. Dynamically casting to `Parseable` isn't possible because of it's `Self` requirement, which it inherits from `Decodable`. To work around this, we introduce a "private" base protocol that has no `Self` requirement, and type-erases decoded values to `Any`. A default implementation of this method then allows `CodableParsingInterceptor` to dynamically call into `Parseable.init(from:decoder:)` via `_ParsableBase._decode(from:decoder:)`, then force-cast back to `Operation.Data`.
|
@sharplet: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
|
Yeah, the I'm slightly reluctant to un-break this before we have all that straightened out, since the current state of this helps emphasize that it's not finished. |
That's great to hear, and I'm looking forward to learning about the details.
Yeah I totally understand. I wouldn't want to precipitate a larger-than-necessary breaking of clients that begin to use this, if that's what you're trying to avoid. Just to give you a bit more context on where this patch came from: I'm working on high-level GraphQL API, designed around Codable models. I'm planning to ship with a bare-bones networking client so that you can hit a GraphQL endpoint out of the box, but it's a non-goal for it to have any of the bells and whistles Apollo includes, like normalised or persistent caching. I'd be delighted to be able to ship with support for integrating Apollo's networking stack. |
|
I'm gonna leave this open for the moment and think about what to do when I get back from vacay next week. FWIW I'm leaning towards removing this from the public API - I'd originally hoped to have the new codegen published not too long after I got the network stack redone, but haha nope - until we get a more concrete look at what the final form of codegen looks like. |
|
Ultimately, I came down on the side of removing the Thank you again for this PR - you're certainly more than welcome to grab the |
3 participants
In practice,
GraphQLResult.init(from:decoder:)would always fail when called byCodableParsingInterceptor. Its default implementation, whenDatais not known to conform toDecodable, would unconditionally throwParseableError.unsupportedInitializer. BecauseCodableParsingInterceptor.interceptAsyncis a generic context where the operation'sDatamay not beDecodable, it would always select the default initialiser, even if aDecodableimplementation is available.Dynamically casting to
Parseableisn't possible because of itsSelfrequirement, which it inherits fromDecodable. To work around this, we introduce a "private" (underscored) base protocol that has noSelfrequirement, and type-erases decoded values toAny. A default implementation of this method then allowsCodableParsingInterceptorto dynamically call intoParseable.init(from:decoder:)via_ParsableBase._decode(from:decoder:), then force-cast back toOperation.Data.