-
Notifications
You must be signed in to change notification settings - Fork 60
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
Optional query discussion #216
Comments
if it it made optional, might be a good idea to also roll in the change of the “query” term to “document” or “source” i prefer source as is more generic and perhaps could also include hash and extensions will specify this. That would require some further language adjustment… Reference: dotansimha/graphql-yoga#1589 (comment) |
Two thoughts: The term I believe that any modification to this spec in order to support persisted queries should be vague enough to allow servers to grow and to encourage experimentation. It should not be restrictive about how servers or clients should support persisted queries (it should not even mention them, so as not to restrict to that use case). In my opinion, if we are to add anything to this spec that supports persisted queries, something like this would be preferred:
This would allow things like persisted queries, fragments, operation names, variable values, etc. There should be no mention of which other parts of the request these values should be determined from. |
dotansimha/graphql-yoga#1589 (comment) I had some text explaining this stance in greater detail somewhere but I can’t find it on my phone in Dover castle. Might be lost in the PR comments for #175? To be clear though, I definitely think persisted operations should be part of the HTTP spec ultimately; ref #38 |
Now I'm back on my computer I found the discussion: #175 (comment) (emphasis added):
|
IMHO, I like @ghmcadams proposal from #216 (comment). Add something like:
|
Since the spec is focussed on compatibility, without some indication of what that means (how can the missing parameters be determined?) I don't think we can add that as-is. Something with similar intent may be suitable though. Personally I'd just like us to see the use of |
@benjie I agree that compatibility is key. Something is needed to describe a contract for client requests to contain pointers to server persisted information. Though I would like to see something more broad than adding an As stated on opensource.com:
There is a balance. Too specific and innovation is limited. Too broad and compatibility suffers. |
Completely agree. I think the
The server will need a document to execute. One option (the currently specified option) is to give the document explicitly. Another option is to give an identifier for that document, such that the server may retrieve it from some kind of store. What other options do you envisage that specifying this as an option would rule out? |
@benjie For the most part, that's great. The issue I have is that I'm struggling to find a simple solution currently, but I think we can find one together as the conversation moves forward. |
Oh right; just use |
id might not be enough as a property. Apollo uses the extension map to store these information on. In discussions with @IvanGoncharov very early when we started on this spec we discussed to not have any extra fields and store everything extra for certain use-cases on the extension map. But this means |
I think we should keep We don't forbid alternative request formats in the spec. If it turns out there are benefits to standardizing them, we can do that too. However, I believe we should then make those alternative request formats their own, separate thing - distinct from a standard This server supports the following request formats: |
Reference:
#175 (comment)
just pulling this out in case there is not another issue to track
in my view, query should be optional so that persisted operations, automatic or otherwise, could be considered spec-compliant.
The spec language would say something like that “the document to be executed, when sent within the http request, should be contained within the optional query parameter. When not included, the implementation can use an implementation defined method of determining the operation…”
otherwise, spec compliant implementations might be tempted to send empty query parameters to be compliant. See: dotansimha/graphql-yoga#1589 (comment)
Tagging below discussants from those issues:
@enisdenjo
@ardatan
@glasser
@benjie
The text was updated successfully, but these errors were encountered: