Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drops Encodable requirement on GraphQL types #129

Conversation

NeedleInAJayStack
Copy link
Member

@NeedleInAJayStack NeedleInAJayStack commented Nov 10, 2023

This drops unnecessary Codable conformance from GraphQL-injected types. More specifically, output types are never encoded or decoded - only the resulting leaf Scalars are encoded. While this PR removes the restriction that all output types are codable, it still requires Encodability on Scalars and Decodability on Arguments. Decodable conformance is technically removed from Input objects, but typically still enforced through the decodable Argument requirement.

This change is useful if you have a complex, non-codable type that you would like to pass through resolver chains.

@NeedleInAJayStack NeedleInAJayStack requested review from paulofaria and removed request for paulofaria November 10, 2023 22:37
@NeedleInAJayStack NeedleInAJayStack changed the title Drops Encodable requirement on GraphQL types Draft: Drops Encodable requirement on GraphQL types Nov 10, 2023
@NeedleInAJayStack
Copy link
Member Author

Changing to draft. It turns out that the Encodable conformance was the only thing differentiating SyncResolve and SimpleAsyncResolve. Without it, resolvers that return futures match both signatures, meaning that the Field init is ambiguous.

@cshadek
Copy link
Contributor

cshadek commented Nov 10, 2023

Would this make connections of interfaces/unions possible?

@NeedleInAJayStack NeedleInAJayStack force-pushed the feature/drop-type-codable-requirement branch from 28f6bbe to 86a1e69 Compare November 11, 2023 00:27
@NeedleInAJayStack NeedleInAJayStack changed the title Draft: Drops Encodable requirement on GraphQL types Drops Encodable requirement on GraphQL types Nov 11, 2023
@NeedleInAJayStack
Copy link
Member Author

Would this make connections of interfaces/unions possible?

I don't think so. I think that's more a limitation in how the GraphQL type system is implemented than a Swift codability restriction.

@cshadek
Copy link
Contributor

cshadek commented Nov 11, 2023

My understanding is the only reason connections of interfaces/ unions doesn't work is that node needs to be encodable. I've created custom connections where the a swift protocol is the node.

@paulofaria
Copy link
Member

@NeedleInAJayStack have you tried @_disfavoredOverload?

@NeedleInAJayStack NeedleInAJayStack force-pushed the feature/drop-type-codable-requirement branch from 86a1e69 to ffcb6d6 Compare November 11, 2023 21:12
@NeedleInAJayStack
Copy link
Member Author

@NeedleInAJayStack have you tried @_disfavoredOverload?

@paulofaria OMG, you're a genius! I had no idea that existed and it totally fixed the problem. Thanks!

@NeedleInAJayStack
Copy link
Member Author

My understanding is the only reason connections of interfaces/ unions doesn't work is that node needs to be encodable. I've created custom connections where the a swift protocol is the node.

@cshadek Oh okay! Yeah, if that's the case, then this probably will fix it!

@NeedleInAJayStack NeedleInAJayStack force-pushed the feature/drop-type-codable-requirement branch from ffcb6d6 to 5278908 Compare November 11, 2023 21:15
Copy link
Member

@paulofaria paulofaria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is super awesome! I'm just curious as to why a type would not be able to conform to Codable. 😄

@NeedleInAJayStack
Copy link
Member Author

This is super awesome! I'm just curious as to why a type would not be able to conform to Codable. 😄

😆 It's a good question... My primary use case is that I have an object with a property that contains existentials, which makes it pretty hard to decode without a huge typeswitch. I don't really want to expose this property in GraphQL, but I do want to expose things derived from it. For example:

struct A {
  let b: [any B] // Not exposed in GraphQL, but important to keep around.
  var bDescription: String { // Exposed in GraphQL
    return b.map { $0.description }
  }
}

@NeedleInAJayStack NeedleInAJayStack merged commit 61f714d into GraphQLSwift:main Nov 12, 2023
9 checks passed
@paulofaria
Copy link
Member

Gotcha, makes sense! Just out of curiosity, 😄. Are you using an existential because the array is not homogeneous so generics wouldn't work?

@NeedleInAJayStack
Copy link
Member Author

Yeah, exactly. It's a heterogeneous array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants