Repository navigation
Add syntax for empty objects - #1245
martinbonnin wants to merge 3 commits into
Conversation
| An Object type may define no fields. This may be written with an empty set of | ||
| braces, or by omitting the braces entirely; the following two definitions are | ||
| equivalent: | ||
|
|
||
| ```graphql example | ||
| type Empty {} | ||
| ``` | ||
|
|
||
| ```graphql example | ||
| type Empty | ||
| ``` |
There was a problem hiding this comment.
@benjie pointed out in #1249 (comment) that this declaration is incomplete so I think we should remove the second example here too.
There was a problem hiding this comment.
@DaleSeo I think both syntaxes should be valid. As a matter of facts, it's already possible to do example 2 today: https://spec.graphql.org/draft/#ObjectTypeDefinition
type Empty is valid.
See also #598 (comment)
There was a problem hiding this comment.
@martinbonnin True, it parses today, but it fails validation because it must define one or more fields. So, the question is whether it should start passing as an empty type.
Interestingly, Lee calls interface Foo "an incompletely defined interface" in the comment you linked, which lines up with @benjie's take. I'm fine either way, but I'd like all four PRs to reach the same conclusion. Maybe we should settle this with the WG?
There was a problem hiding this comment.
the question is whether it should start passing as an empty type.
My preference is that it should. I'd be very surprised if type Empty {} was valid but type Empty wasn't.
Lee calls interface Foo "an incompletely defined interface" in the comment you linked, which lines up with @benjie's take
👍 Before empty types, interface Foo was an incompletely defined interface. With empty types, it becomes an empty interface, equivalent to interface Foo {}.
There was a problem hiding this comment.
Yeah Lee explicitly doesn't want this, and though I didn't agree at first, having thought about it, I do agree.
I explicitly do not want two ways of defining an empty object. Thus, I used our existing syntax:
type User^ User with no fields. This keeps type User {} as invalid, so there's only one way of defining an empty object type.
However, type User (no braces) has always been an extension point - an indication of things to come later. It's a placeholder, and people who, until now, use it in SDL are expecting to complete the type later via extend type.
We should keep this meaning, and there should remain exactly one way to define an empty object type. Without this, users might accidentally start putting empty types into their schema, and there's no syntactic way to tell a linter "I explicitly mean for this to be an empty type" vs "this is a placeholder I shall complete later". I wish we'd not used type User to model this, and instead used declare type User, but that ship has long sailed.
So:
type User # This is a STUB - it needs to be `extend`-ed to become complete
extend type User {} # User is now a complete type with no fields
type Empty {} # Empty is a complete type with no fieldsThere was a problem hiding this comment.
Fair enough. I've updated that pull request to reflect that. Let me know how that sounds and I'll update the interfaces PR symmetrically.
There was a problem hiding this comment.
FWIW, I've asked internally if we have any evidence people use this pattern. Would be nice to have data to back this.
There was a problem hiding this comment.
I’ve been discussing this with Dale in Discord and effectively union is where this argument falls down. There’s no way to define a union that contains no types separately to a union “placeholder” that’s waiting for extension. We could accept that union is the odd duck here and push ahead with empty braces, or we could consistently say that empty types are effectively unadorned, as I do in the parent PR as it stands. The latter has the advantage of not being a syntax change, and thus not breaking any existing parsers. I’m leaning towards the latter, but we can discuss at the WG on Thursday.
There was a problem hiding this comment.
👍 I put this PR back to its initial state where type Empty is a shorthand for type Empty {} in case we want it. Some robots might end up producing {} by removing all the field definitions.
Follow up from #1228 (comment)
Ping @benjie