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

Investigate having support for union #10

Merged
merged 3 commits into from
May 27, 2017

Conversation

williambailey
Copy link
Contributor

@williambailey williambailey commented May 11, 2017

Following up my own question in #9. I'm currently trying to think of the cleanest way to add union support to Graphiti.

This PR enables union support by way of a marker protocol. Usage is something like the following (trying to stick to the StarWars example)...

// define union marker protocol
protocol DroidAndPlanetUnion {}

// associate other members of the union
extension Droid: DroidAndPlanetUnion {}
extension Planet: DroidAndPlanetUnion {}

// building the unions
let starWarsSchema = try! Schema<NoRoot, NoContext> { schema in
    // ...
    try schema.union(type: DroidAndPlanetUnion.self) { union in
        union.description = "an unholy union of droids and planets"
        union.types = [ Droid.self, Planet.self ]
    }
    // alternatively if you don't need a description.
    try schema.union(type: DroidAndPlanetUnion.self, members: [ Droid.self, Planet.self ])
    // ...
}

I wasn't able to figure out a way to validate that the types provided in union.types implement the DroidAndPlanetUnion protocol.

Tests etc. can follow should we agree on the way forward.

@williambailey
Copy link
Contributor Author

Build failure is related to needing to first declare the init function on GraphQLUnionType public in the GraphQL package.

@paulofaria
Copy link
Member

have you thought about any other option? I think we could go with that if we're not able to come up with something else.

public final class UnionTypeBuilder<Type> {
public var description: String? = nil
public var resolveType: GraphQLTypeResolve? = nil
public var types: [Any.Type] = []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried using [Type.Type] here (and in the union<Type> function members arguments) in an attempt to make it a bit more type safe however I was then unable to actually produce any code that would compile without errors like... Cannot convert value of type ... to expected argument type '[_.Type]'

@williambailey
Copy link
Contributor Author

I toyed with using enums to define the union but this resulted in more complication both with the implementation and on the call side. The marker protocol approach seemed the most elegant. Open to other suggestions however.

@paulofaria
Copy link
Member

CI is not passing due to some ACL issues.. can you please fix it? 😊

@paulofaria
Copy link
Member

paulofaria commented May 24, 2017

By the way, merged and tagged GraphQL PR. Let's see if this alone fixes it.

@paulofaria
Copy link
Member

NICE™. CI is passing. think it's ready to merge?

@williambailey
Copy link
Contributor Author

I think so. 😀

@paulofaria paulofaria merged commit 5db60d7 into GraphQLSwift:master May 27, 2017
@codecov-io
Copy link

Codecov Report

Merging #10 into master will increase coverage by 0.64%.
The diff coverage is 91.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
+ Coverage   82.69%   83.34%   +0.64%     
==========================================
  Files          13       13              
  Lines        1745     1855     +110     
==========================================
+ Hits         1443     1546     +103     
- Misses        302      309       +7
Impacted Files Coverage Δ
...s/GraphitiTests/StarWarsTests/StarWarsSchema.swift 80% <ø> (ø) ⬆️
...aphitiTests/StarWarsTests/StarWarsQueryTests.swift 96.6% <100%> (+0.21%) ⬆️
...sts/StarWarsTests/StarWarsIntrospectionTests.swift 97.66% <100%> (+0.1%) ⬆️
...sts/GraphitiTests/StarWarsTests/StarWarsData.swift 85% <100%> (+9.32%) ⬆️
Sources/Graphiti/Schema/Schema.swift 59.02% <74.35%> (+1.67%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b50a6d...d7c0829. Read the comment docs.

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