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

Allow empty object type _extensions_ in schema #1106

Open
ychescale9 opened this issue Jul 19, 2024 · 10 comments
Open

Allow empty object type _extensions_ in schema #1106

ychescale9 opened this issue Jul 19, 2024 · 10 comments

Comments

@ychescale9
Copy link

ychescale9 commented Jul 19, 2024

I have been told by @martinbonnin that the following isn't supposed to work according to the spec:

extend type Query {

}

extend type Mutation {

}

However being able to have empty type definitions is useful sometimes where a local "stub" schema needs to be changed between empty and non-empty frequently as part of the dev workflow.

Specifically the use case for us is that we have a local stub.graphqls schema alongside the remote schema we download from the backend. Our devs sometimes add types to the stubs.graphqls while waiting for the changes to be deployed to the real schema (we try to avoid manually tempering the real schema). So about half of the time the content of the extend type Query is empty.

A similar use case can be found here.

@benjie
Copy link
Member

benjie commented Jul 19, 2024

Duplicate of #568 - please re-post any new interesting feedback to that thread.

Also related: #236

@benjie benjie closed this as not planned Won't fix, can't repro, duplicate, stale Jul 19, 2024
@martinbonnin
Copy link
Contributor

martinbonnin commented Jul 19, 2024

Thanks for opening this @ychescale9!

@benjie I'd argue this issue is slightly different from #568: #568 is about empty types and this issue is about empty exensions.

#568 is probably a more fundamental change to the GraphQL type system. I see this issue as mainly about tooling. It doesn't change how the types are represented. Unions are still a thing and objects are required to have one or more fields. Just for convenience, we would allow extensions to be empty. That is a much smaller lift than #568.

Can we keep it open with Allow empty object and interface extensions as a title?

@benjie benjie reopened this Jul 19, 2024
@benjie benjie changed the title Allow empty object type in schema Allow empty object type _extensions_ in schema Jul 19, 2024
@benjie
Copy link
Member

benjie commented Jul 19, 2024

We already allow extend type Query to exist. Can you not simply omit the {} when there's nothing in the braces?

I'm pretty sure this parses, though I haven't checked:

type Query {
  meaningOfLife: Int
}
extend type Query

@martinbonnin
Copy link
Contributor

martinbonnin commented Jul 19, 2024

Agreed, extend type Query should reasonably work.

I think the intent here is facilitating copy/pasting full lines. I see this a bit as trailing comma quality of life improvement. Doesn't really change the language but makes it easier to work with the documents on a daily basis.

It also minimizes the diff if you were to commit those files, etc... It's not essential but I would support the change unless there are strong reasons not to do so that I'm not thinking about today.

@ychescale9
Copy link
Author

@martinbonnin FYI with Apollo Kotlin just having extend type Query throws: Unexpected token: 'name: extend'.

@martinbonnin
Copy link
Contributor

Yikes, let me double check. Just to make sure: is there anything before that extend type Query? If it's not expecting extend, it probably means the parser is left hanging on the previous expression.

@ychescale9
Copy link
Author

Yikes, let me double check. Just to make sure: is there anything before that extend type Query? If it's not expecting extend, it probably means the parser is left hanging on the previous expression.

Just tested again: this is the entire document:

extend type Query

Result:

path/to/stub.graphqls: (2, 1): Unexpected token: 'EOF'
  ----------------------------------------------------
  [1]:extend type Query
  [2]:

  ----------------------------------------------------

@martinbonnin
Copy link
Contributor

Turns out extend type Query not parsing is working as expected.

See https://spec.graphql.org/draft/#ObjectTypeExtension, the spec mandates that the extension adds at least a directive, a field or an interface.

@benjie
Copy link
Member

benjie commented Jul 19, 2024

That makes sense; extend type Query on it's own would be as meaningless (no-op) as extend type Query {}. Trailing commas are an interesting analogy; all commas are ignored in GraphQL - type,Query,{,a:,Int!,,},, is valid I think - but braces are significant so we can't apply the same kind of logic alas. The question really is: is there sufficient utility in this suggestion to justify making every GraphQL implementation support it?

@martinbonnin
Copy link
Contributor

is there sufficient utility in this suggestion to justify making every GraphQL implementation support it?

That's the billion dollar question. My initial thoughts are that parsers are written by few people (dozens?) but used by many more (thousands? millions?). So the price to pay for the maintainers is amortized on a lot of potential users.

Now it all depends how many users would actually benefit from this feature... #tradeoffs

martinbonnin added a commit to apollographql/apollo-kotlin that referenced this issue Jul 19, 2024
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

No branches or pull requests

4 participants
@benjie @martinbonnin @ychescale9 and others