Repository navigation
Conversation
8c40697 to
21a2a8c
Compare
|
Pulled from this great comment (emphasis mine):
Whilst I agree with the above, I also think that the GraphQL spec generally avoids dictating best practices and instead focuses on compatibility. I think "don't use marker interfaces" is a best practice that people should generally adhere to, and I think allowing empty interfaces does cause confusion of interface vs union as raised by Caleb, but I think this problem lives in the space of "best practices" rather than "hard requirements" and there are benefits to allowing for empty interfaces - both in terms of future expansion and prototyping (these two things are both "Searchable", but I don't know what that means yet... but when I do it will be an interface contract rather than a union) and in terms of subschema extractions (both of these implement the Searchable interface... [not included]). I think it warrants debate. |
martinbonnin
left a comment
There was a problem hiding this comment.
+1000
For the record, I'm also very OK with marker interfaces. Java/Kotlin have had them pretty much forever
| - UnionMemberTypes | NamedType | ||
| - = `|`? NamedType | ||
|
|
||
| GraphQL Unions represent an object that could be one of a list of GraphQL Object |
There was a problem hiding this comment.
| GraphQL Unions represent an object that could be one of a set of GraphQL Object |
|
|
||
| Union types have the potential to be invalid if incorrectly defined. | ||
|
|
||
| 1. A Union type must include one or more unique member types. |
There was a problem hiding this comment.
Let's not lose the fact they are unique - ensure that it's declared as a "set" somewhere maybe?
There was a problem hiding this comment.
I wonder how the direction suggested in #1249 (comment) applies here. Unions have no braces, so there is no way to tell an empty union from an incomplete one: union Empty would be the only form, and it becomes valid once this rule is removed. Is that fine for unions, or did you have something like union Empty = in mind?
|
Another concrete example of why we need empty unions comes from the Schema Search RFC(graphql/ai-wg#187). When a search matches a field that returns a union but none of its members, the slice has to drop both the union and the field that returns it, since unions need at least one member today. With this change, the slice can keep I think the practical impact on interfaces is smaller. A slice already drops an |
|
|
||
| 1. An Interface type must define one or more fields. | ||
| 2. For each field of an Interface type: | ||
| 1. For each field of an Interface type: |
There was a problem hiding this comment.
Same as my note on #1228: replace the rule rather than delete it, so interface Empty {} is valid and a braceless interface Empty stays invalid.
1. An Interface type must define a set of fields. The set may be empty.
|
|
||
| Union types have the potential to be invalid if incorrectly defined. | ||
|
|
||
| 1. A Union type must include one or more unique member types. |
There was a problem hiding this comment.
I wonder how the direction suggested in #1249 (comment) applies here. Unions have no braces, so there is no way to tell an empty union from an incomplete one: union Empty would be the only form, and it becomes valid once this rule is removed. Is that fine for unions, or did you have something like union Empty = in mind?
Follow up to:
This is again to help subschemas remain valid. Empty unions is fine (we already allow interfaces with no implementations, which is kind of the mirror), but empty interfaces makes it so people may want to use interfaces where unions should currently be used... Relevant:
Incidentally I'm okay with deprecating unions... And adding
@oneOfoutput types instead.