search

movio / bramble

A federated GraphQL API gateway
https://movio.github.io/bramble/
MIT License
497 stars 55 forks source link

should bramble complain about built-in directives declared in spec? #168

Closed benzolium closed 1 year ago

benzolium commented 2 years ago

I've recently encountered a bug when was trying to federate a service which contained specifiedBy directive. It is in latest gql spec, but i've got an error:

Undefined directive specifiedBy.

Should I declare such directives in downstream services? Or it is a bug?

pkqk commented 2 years ago

Hi @benzolium we're using the graphql-go library to parse GraphQL. I did a quick search of the library and can't find any support for the directive yet. We're at v1.3.0 but it looks like the v1.4.0 release also doesn't include it either.

As a quick fix you could declare the directive in your services, but is a bug that we're not supporting the new builtin directive.

benzolium commented 2 years ago

@pkqk thanks for the reply! I've opened an issue there.

Also thanks for a quick fix idea - I've just done that exact thing. 🙂

pkqk commented 2 years ago

@benzolium I see you've already got support merged in to the gophers library 💯 , I'll try out the head and we'll update when they release.

benzolium commented 1 year ago

@pkqk the package has been finally released. But actually I can't write a test to reproduce the error. The only thing which was failing was vektah/gqlparser.

It was updated with specifiedby directive long time ago: https://github.com/vektah/gqlparser/releases/tag/v2.3.0. Bramble is on 2.2.0 now so it's also worth updating.