ardatan/graphql-import

GraphQL root level directive syntax

Opened this issue · 7 comments

In the last GraphQL working group meeting we discussed to enable GraphQL directives on a document/root level. This would allow for the following syntax which avoids the common "why is it commented out" confusion around graphql-import:

@import(defs: [Post, Comment] from: "./other-types.graphql")
@import(defs: Message from: "./msg.graphql")

type User {
  id: ID!
}

Open questions

  • Final syntax: @import, defs:, from:

@leebyron @IvanGoncharov do you have an opinion on whether it should be

@import(defs: [Post, Comment] from: "./other-types.graphql")

or 2)

@import(defs: ["Post", "Comment"] from: "./other-types.graphql")

?

do you have an opinion on whether it should be

I think 1 is incorrect variant since it implies that Post, Comment is values of some kind of enum and you can't define @import directive without defining this enum first.

Anyway, I think we should start by discussing how to:

enable GraphQL directives on a document/root level

It has its own set of problems, e.g. you can't have multiple directives at the same location: http://facebook.github.io/graphql/October2016/#sec-Directives-Are-Unique-Per-Location

@schickling I would suggest open an issue at facebook/graphql repo regarding document level directives and get feedback from the entire graphql community.

I actually think that 2. is better for the same reason you mentioned.

Otherwise, where is the enum definition coming from?

As Ivan explained, I don't think either will work. GraphQL only support one directive with a given name. I don't see this changing anytime soon.

As such, a single @import directive must support multiple imports to make this work.

Using multi-line string

@import(defs: """Post, Comment from "./other-types.graphql"
                 Message from "./msg.graphql"""")

or

@import(defs: """
  Post, Comment from "./other-types.graphql"
  Message from "./msg.graphql"
""")

This would be even nicer if GraphQL would support the following syntax:

@import("""
  Post, Comment from "./other-types.graphql"
  Message from "./msg.graphql"
""")

This could be achieved in two ways

  1. Introduce a general concept of positional arguments
  2. Allow (potentially just for directives) omitting the argument name when there is just one argument available.

Both approaches have severe drawbacks. 1. forces every single server to specify the order of arguments. 2. would make it a breaking change to introduce a second nullable field.

If approach 2. is limited to directives or maybe even an explicit part of the Directive type, this might be a worthy tradeoff. In my experience there are many cases where it is beneficial to be able to have a directive with a single unnamed argument:

  • @import("some import specification") instead of @import(defs: "some import specification")
  • @default("some default value") instead of @default(value: "some default value")
  • @calculated("some expression") instead of @calculated(expression: "some expression")

@IvanGoncharov I would love your thoughts on this!

list of strings

@import(defs: ["Post, Comment from ./other-types.graphql", "Message from ./msg.graphql"])

list of input objects

@import(defs: [
  {defs: "Post, Comment", from: "./other-types.graphql"}
  {defs: "Message", from ./msg.graphql"}])

I think the multiline string is the only option that is not too heavy in notation. Nameless arguments would be nice, but we don't need to wait for this change to land before moving forward.

As Ivan explained, I don't think either will work. GraphQL only support one directive with a given name. I don't see this changing anytime soon.

@sorenbs It was just an example of why you need to discuss the possibility of document level directive first because it's the main problem right now.
You can put directives only in allowed places (search for directives in this section).

The closest thing you can do at the moment is to write:

schema @import(defs: ["..."]) {
  query: MyQuery
}

And you definitely can't write:
image
https://astexplorer.net/#/gist/d1e3ad590b4995d7d7b307d3619a3106/27e0723d47ccae8c6b0bd6c9c0a6c7dc6c34d463

So it's chicken or egg kind of problem and I think before spending time developing syntax you need to push support of Document level directives in the spec which I think is a really big problem by itself.

@IvanGoncharov - I completely agree and volunteered to drive top-level directives at the latest working group https://github.com/graphql/graphql-wg/blob/master/notes/2018-02-01.md#discuss-subscriptions-for-live-queries

This issue serves as motivation for the feature :-)

After more internal discussion we reached the conclusion that it is not worth pursuing replacing the current comment based syntax with a directive based syntax.

The main points against this change:

  1. The change requires at least two changes to the GraphQL spec, meaning a realistic timeline is 6+ months.
  2. Even with the changes to the GraphQL spec, the resulting syntax is still more complicated than the current comment based syntax
  3. GraphQL SDL might eventually gain a native import keyword making the entire exercise moot.

Especially given point 3, we would rather not force the community to go through the transition twice. We believe the current comment based syntax is sufficient to give us all the learnings we need before we can propose a native import keyword.

Id be curious to hear if anybody think we are missing something?