graphql-kit / graphql-voyager

🛰️ Represent any GraphQL API as an interactive graph
https://graphql-kit.com/graphql-voyager/
MIT License
7.81k stars 516 forks source link

GraphQL Voyager not compliant with latest specs, breaks HotChocolate #237

Closed kikaragyozov closed 2 years ago

kikaragyozov commented 2 years ago
voyager.min.js:82 Uncaught (in promise) Error: Name "__AppliedDirective" must not begin with "__", which is reserved by GraphQL introspection.

Name "__DirectiveArgument" must not begin with "__", which is reserved by GraphQL introspection.
    at voyager.min.js:82:186809
    at voyager.min.js:82:186870
    at Vn (voyager.min.js:82:187038)
    at n.schema (voyager.min.js:82:194555)
    at sr (voyager.min.js:82:194678)
    at gr (voyager.min.js:82:196715)
    at t.updateIntrospection (voyager.min.js:82:248696)
    at voyager.min.js:82:248557

This was first brought to the attention of the HotChocolate team in https://github.com/ChilliCream/hotchocolate/issues/5154, but it seems that the __DirectiveArgument is indeed part of the official spec as an official type, but as a DRAFT feature. Could we please have this fixed? Thanks.

LunaticMuch commented 2 years ago

@SpiritBob which version of Voyager are you using? And how are you using it?

kikaragyozov commented 2 years ago

@LunaticMuch I'm using GraphQL.Server.Ui.Voyager which is maintained by GraphQL-DotNet. How can I check the version from the UI? The worker file is delivered via a cdn, so I'll toss a link to that if it helps you in any way.

LunaticMuch commented 2 years ago

Cool. It's not a worker problem per se. My best guess is that while we made an update to the graphql core library, this has not been pushed yet on the CDN/NPMJS so whoever is using voyager still have a version which depends on a wrong graphql library.

Would be able to share the schema/introspection you're using to reproduce the issue?

kikaragyozov commented 2 years ago

For example:

schema {
  query: QueryType
  mutation: MutationType
}

"""
The `@defer` directive may be provided for fragment spreads and inline fragments to inform the executor to delay the execution of the current fragment to indicate deprioritization of the current fragment. A query with `@defer` directive will cause the request to potentially return multiple responses, where non-deferred data is delivered in the initial response and data deferred is delivered in a subsequent response. `@include` and `@skip` take precedence over `@defer`.
"""
directive @defer(
  """
  If this argument label has a value other than null, it will be passed on to the result of this defer directive. This label is intended to give client applications a way to identify to which fragment a deferred result belongs to.
  """
  label: String

  """
  Deferred when true.
  """
  if: Boolean
) on FRAGMENT_SPREAD | INLINE_FRAGMENT

"""
The `@stream` directive may be provided for a field of `List` type so that the backend can leverage technology such as asynchronous iterators to provide a partial list in the initial response, and additional list items in subsequent responses. `@include` and `@skip` take precedence over `@stream`.
"""
directive @stream(
  """
  If this argument label has a value other than null, it will be passed on to the result of this stream directive. This label is intended to give client applications a way to identify to which fragment a streamed result belongs to.
  """
  label: String

  """
  The initial elements that shall be send down to the consumer.
  """
  initialCount: Int! = 0

  """
  Streamed when true.
  """
  if: Boolean
) on FIELD

"""
An Applied Directive is an instances of a directive as applied to a schema element. This type is NOT specified by the graphql specification presently.
"""
type __AppliedDirective {
  name: String!
  args: [__DirectiveArgument!]!
}

"""
Directive arguments can have names and values. The values are in graphql SDL syntax printed as a string. This type is NOT specified by the graphql specification presently.
"""
type __DirectiveArgument {
  name: String!
  value: String!
}

type QueryType {
  """
  Fetches an object given its ID.
  """
  node(
    """
    ID of the object.
    """
    id: ID!
  ): Node

  """
  Lookup nodes by a list of IDs.
  """
  nodes(
    """
    The list of node IDs.
    """
    ids: [ID!]!
  ): [Node]!
  tests: [Test!]!
}

type MutationType {
  doTest(ids: [ID!]!): Boolean
}

"""
The node interface is implemented by entities that have a global unique identifier.
"""
interface Node {
  id: ID!
}

type Test implements Node {
  id: ID!
  name: String!
}
LunaticMuch commented 2 years ago

Ok, I see it. You named a directive with a double __ at the beginning. The error is correct, and it's not voyager, it's the graphql library telling you that it's a reserved prefix. You can check the official specs.

The double underscore is returned by the introspection, but this is an SDL and it can't be there.

kikaragyozov commented 2 years ago

@SpiritBob this is a issue of GraphQL Voyager as they do apparently not support these types. The __DirectiveArgument is a offical introspection type, but it's a DRAFT feature on the spec. I woudl suggest to raise this issue on the graphql voyager repository, as HC is spec compliant in this case

Could you please toss a link to where they specify that it's an official introspection type? Latest working draft doesn't specify it, unless my CTR +F is playing tricks on me. https://spec.graphql.org/draft/

@SpiritBob Its this feature graphql/graphql-spec#300. It's a stage 0 spec draft so hasnt made it into the offical draft specification. The introspection types are returned by the normal introspection aswell, but are not part of the SDL.

LunaticMuch commented 2 years ago

@SpiritBob correct and because it's draft, it's not usable. graphql-js library does not support it. I doubt you can find another supporting it.

You should refrain from using it. If you decide to use it, you need to accept the risk.