-
Notifications
You must be signed in to change notification settings - Fork 56
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
GraphQL root level directive syntax #89
Comments
@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") ? |
I think 1 is incorrect variant since it implies that Anyway, I think we should start by discussing how to:
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 |
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 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
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:
@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. |
@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. The closest thing you can do at the moment is to write: schema @import(defs: ["..."]) {
query: MyQuery
} And you definitely can't write: 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:
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 Id be curious to hear if anybody think we are missing something? |
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:
Open questions
@import
,defs:
,from:
The text was updated successfully, but these errors were encountered: