GraphQLSwift / Graphiti

The Swift GraphQL Schema framework for macOS and Linux
MIT License
531 stars 67 forks source link

Investigate having support for union #10

Closed williambailey closed 7 years ago

williambailey commented 7 years ago

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 commented 7 years ago

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

paulofaria commented 7 years ago

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.

williambailey commented 7 years ago

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 commented 7 years ago

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

paulofaria commented 7 years ago

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

paulofaria commented 7 years ago

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

williambailey commented 7 years ago

I think so. 😀

codecov-io commented 7 years ago

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% <ø> (ø) :arrow_up:
...aphitiTests/StarWarsTests/StarWarsQueryTests.swift 96.6% <100%> (+0.21%) :arrow_up:
...sts/StarWarsTests/StarWarsIntrospectionTests.swift 97.66% <100%> (+0.1%) :arrow_up:
...sts/GraphitiTests/StarWarsTests/StarWarsData.swift 85% <100%> (+9.32%) :arrow_up:
Sources/Graphiti/Schema/Schema.swift 59.02% <74.35%> (+1.67%) :arrow_up:

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.