apollographql / apollo-kotlin

:rocket:  A strongly-typed, caching GraphQL client for the JVM, Android, and Kotlin multiplatform.
https://www.apollographql.com/docs/kotlin
MIT License
3.73k stars 653 forks source link

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

Open ychescale9 opened 1 month ago

ychescale9 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

ychescale9 commented 1 month ago

https://github.com/graphql/graphql-spec/issues/1106

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

Turned out there's an existing issue: https://github.com/graphql/graphql-spec/issues/568