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

Apollo 4 AST - add a flag to relax parsing and allow empty extend type #6061

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

Comments

@ychescale9
Copy link
Contributor

Version

4.0.0-rc.1

Summary

Prior to apollo 4 it's possible to have empty extend type Query|Mutation definitions:

# Stub queries
extend type Query {

}

# Stub mutations
extend type Mutation {

}

With Apollo 4, compiling the module with these empty type definitions (the module has both the schema.graphqls from remote and a local stub.graphqls) fails with the following:

Execution failed for task ':my-schema:generateMyServiceApolloCodegenSchema'.
> e: /path/to/my-schema/src/main/graphql/stub.graphqls: (8, 1): Expected Token$Name, found '}'.
  ----------------------------------------------------
  [7]:
  [8]:}
  [9]:
  ----------------------------------------------------

Steps to reproduce the behavior

With Apollo 4, add a stub.graphqls file in a module:

# Stub queries
extend type Query {

}

# Stub mutations
extend type Mutation {

}

in build.gradle.kts:

apollo {
    service("myService) {
        packageName.set("com.example.graphql")
        generateApolloMetadata.set(true)
    }
}

Run ./gradlew my-schema:generateMyServiceApolloCodegenSchema.

Logs

No response

@martinbonnin
Copy link
Contributor

martinbonnin commented Jul 19, 2024

Hi 👋

Thanks for the detailed issue and reproducer!

This actually "works as designed"... GraphQL lists are non-empty by default:

A subscript suffix “Symbollist” is shorthand for a list of one or more of that symbol, 
represented as an additional recursive production.

See also ObjectTypeExtension:

 FieldsDefinition: {FieldDefinition (list)}

I'm not opposed to adding parsing options to relax the syntax if there's a clear use case.

Ideally this is changed in the spec itself. There are a few other places where having empty lists could be useful. Latest to date from @StylianosGakis is empty selection sets if you just want to know if a composite object is null without selecting anything:

query GetFoo {
  # this is not valid GraphQL but could be useful to get the nullability of foo
  foo {}
}

@ychescale9
Copy link
Contributor Author

Thanks for the explanation!

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 workaround that seems to work is to add a _: Boolean when there are no extended types, though it's slightly less convenient than before😀.

@martinbonnin
Copy link
Contributor

@ychescale9 want to open an issue in the GraphQL spec repo so we have tracking for this use case?

@ychescale9
Copy link
Contributor Author

ychescale9 commented Jul 19, 2024

graphql/graphql-spec#1106

@martinbonnin I'm not sure if my terminologies are right so feel free to chime in 😄

Turned out there's an existing issue: graphql/graphql-spec#568

@martinbonnin martinbonnin changed the title Apollo 4 - empty extend type no longer allowed Apollo 4 AST - add a flag to relax parsing and allow empty extend type Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants