-
Notifications
You must be signed in to change notification settings - Fork 19
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
[Feature Request] Ability to set operationName
(name
) through queryDocument
#30
Comments
I'd don't mind doing this and issuing a PR. I just need to know that this is a direction that you (@jamesmacaulay) want to go in. |
@sunwukonga I didn't provide a way to create named operations with the builder DSL because I couldn't think of a situation where you'd need a named operation in a single-operation Document. I considered expanding the DSL to allow for the creation of multi-operation documents, but decided against it because it would have involved adding a lot more complexity to the API and didn't seem worth it. Can I ask what your use case is for this? Is there a reason why you need to be able to create named operations? |
I am integrating Haskell's graphql-api with elm-graphql. Up untill last night, graphql-api did not accept anonymous queries that are prefixed with I have to admit that I'm a little uncertain why an operation would need to be named at all. By convention defined in the spec under #operation-names
You are correct, if there is only a single operation, there is no need to specify an operation name. However, simply restricting documents to a single operation seems a better and saner choice (which you took), given the lack of utility of the case. The above implies that mutations need naming, but again, there is no need if there is only one operation. Similarly, the need in conjunction with variables is only realized in the context of multiple operations. However, operation names and multiple operations per document are defended at graphql github on the strength of future expansions of the spec, circa. 2015, (summarized as):
1, 2, and 3 seem unnecessary. That means, to me at least, that operation names are not really about selecting operations unambiguously (although they can if you're silly enough to include more than one operation per document). Therefore, the only utility for operation names (at this time) is when returning error messages, in which case they become a valuable feedback mechanism for tracking down bugs, regardless of the number of operations in the document. Ultimately, it seems that multiple operations per document and the means to specify a particular one, seems to have been a grand architecture choice that hasn't panned out (...yet). In conclusion, since operation names are only truly useful for tracking errors, they have utility in every query, not just in ones with the arbitrarily and unnecessarily introduced case of multiple operations in a single request. From above:
With mutations (and possibly subscriptions) ^^^^^^, an operation name seems to be expected by servers, thanks to the spec, though not because of any concrete need for them. In the context of "dynamic variables", they are also expected (again without need). |
Strictly from the spec #operation-names, and interpreted succinctly:
|
@sunwukonga okay, you've convinced me that it's worth providing at least the ability to name your operations. I will add functions for that. However, about this statement:
I believe you've interpreted the documentation you were reading incorrectly. The "Learn GraphQL" docs are a great learning resource, but they aren't as precise as the actual spec. If you look at the section of the spec on operations and the corresponding section on validation of operations, you'll see that the operation name is always optional as long as an anonymous operation only ever occurs on its own in a single-operation document. In fact, the primary example of an operation given by the spec is a mutation with no name: mutation {
likeStory(storyID: 12345) {
story {
likeCount
}
}
} |
This is one of those times I'm happy to be wrong; I was literally wondering how facebook engineers could be so imprecise and illogical :). The graphql.org on operation names is not just badly worded, it's wrong. I had, up to this point, made the automatic assumption that they were one-to-one. Thanks, this makes me even more sure that what I'm PR-ing on the server side with graphql-api is correct. I'll keep my eyes firmly on http://facebook.github.io/graphql/October2016/ in future. Would you like me to rearrange these functions to provide a named version and the corresponding anon version that uses it? Which do you prefer:
I think that option 2 conveys the sense that |
Resolved with #31, I will publish a minor version release today. Thanks for bringing this up, @sunwukonga! |
At the moment I cannot see any mechanism for setting the operationName of a
Query
orMutation
.queryDocument
is declared insrc/GraphQL/Request/Builder.elm
as:I propose that this function should be named
anonymousQueryDocument
and anotherqueryDocument
function created with signature:where
The
OperationName
should be, alphanumeric with underscores, and not starting with a number /[_A-Za-z][_0-9A-Za-z]*/ I think this check can be deferred to a query validation step.anonymousQueryDocument
could then be written as:Breaking change obviously. The alternative is a new function, maybe
namedQueryDocument
instead of moving the old behaviour toanonymousQueryDocument
. All of the above also applies tomutationDocument
.The text was updated successfully, but these errors were encountered: