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

Error parsing @join__type directive on supergraph #269

Closed
tduong2049 opened this issue Jul 14, 2023 · 6 comments · Fixed by #270
Closed

Error parsing @join__type directive on supergraph #269

tduong2049 opened this issue Jul 14, 2023 · 6 comments · Fixed by #270

Comments

@tduong2049
Copy link

tduong2049 commented Jul 14, 2023

What happened?

I am using genqlient to generate GraphQL operations into Go code. It uses gqlparser to validate a provided graph before generating code.
In this case, I am using a supergraph composed from multiple subgraphs via Rover CLI.

Prior to v2.5.2, gqlparser parses the supergraph and returns no errors.
Upon upgrading to v2.5.2+, I get the following error when running go run github.com/Khan/genqlient:
invalid schema: Argument extension for directive join__type cannot be null.

Possibly related to #258?

What did you expect?

No parsing errors when running: go run github.com/Khan/genqlient on a supergraph.
However, a validation error is returned when parsing @join__type directives.

Minimal graphql.schema and models to reproduce

type AircraftManufacturer
  @join__type(graph: AIRCRAFT, key: "uuid")
{
  uuid: ID!
  name: String!
}

versions

  • go list -m github.com/vektah/gqlparser/v2?
  • go version?
github.com/vektah/gqlparser/v2 v2.5.6
go version go1.20.6 darwin/arm64
@StevenACoffman
Copy link
Collaborator

Oh thanks for noticing this, and for providing the minimal repro. I would really appreciate it if you can try to put up a PR to fix this!

@StevenACoffman
Copy link
Collaborator

@benjaminjkraft Just so you are aware of this issue that effects genqlient

@benjaminjkraft
Copy link
Contributor

Thanks for the cc! From a quick glance it looks like #258 didn't correctly handle default arguments? The directive definition seems to be

directive @join__type(
  graph: Graph!,
  key: FieldSet,
  extension: Boolean = false,
  resolvable: Boolean = true,
  isInterfaceObject: Boolean = false
) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR

so the argument in question (extension) is non-nullable but has a default so still isn't required.

@StevenACoffman
Copy link
Collaborator

@tduong2049 @benjaminjkraft Thanks! I made and merged a PR to fix that, but I'm too pressed for time to add test coverage for it at present. If either of you get a chance, I would appreciate a PR for that.

StevenACoffman added a commit to Khan/genqlient that referenced this issue Jul 16, 2023
This updates both gqlparser and gqlgen dependencies. This was prompted
by an error report in gqlparser.

gqlparser also preserves comments now, so some test cases might need
tweaking here.

Per @tduong2049 in vektah/gqlparser#269 

### What happened?
> I am using [genqlient](https://github.com/Khan/genqlient/) to generate
GraphQL operations into Go code. It uses gqlparser to validate a
provided graph before generating code.
> In this case, I am using a supergraph composed from multiple subgraphs
via Rover CLI.

> Prior to
[v2.5.2](https://github.com/vektah/gqlparser/releases/tag/v2.5.2),
gqlparser parses the supergraph and returns no errors.
> Upon upgrading to v2.5.2+, I get the following error when running `go
run github.com/Khan/genqlient`:
> `invalid schema: Argument extension for directive join__type cannot be
null.`

> Possibly related to #258?

### What did you expect?
No parsing errors when running: `go run github.com/Khan/genqlient` on a
supergraph.
However, a validation error is returned when parsing `@join__type`
directives.

### Minimal graphql.schema and models to reproduce
```
type AircraftManufacturer
  @join__type(graph: AIRCRAFT, key: "uuid")
{
  uuid: ID!
  name: String!
}
```


Signed-off-by: Steve Coffman <[email protected]>

---------

Signed-off-by: Steve Coffman <[email protected]>
@StevenACoffman
Copy link
Collaborator

@tduong2049 I made a new release of gqlparser (v2.5.7) and merged a genqlient PR (but have not drafted a release of genqlient).

This should fix your problem until a new release of genqlient is released:

go get github.com/vektah/[email protected]
go get github.com/Khan/genqlient@05817543f5ab8d34036c70b1402d234db880f8a7
go mod edit -replace  github.com/Khan/genqlient=github.com/Khan/genqlient@05817543f5ab8d34036c70b1402d234db880f8a7
go mod tidy

@benjaminjkraft
Copy link
Contributor

You shouldn't need to update genqlient, nor a replace, it should suffice to just do go get github.com/vektah/[email protected]. (Go doesn't install different (minor) versions of deps-of-deps like node does; if you ask for gqlparser 2.5.7, then that's the only (minor) version of gqlparser (v2) you're getting, and all your deps use it.)

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

Successfully merging a pull request may close this issue.

3 participants