Optional query discussion

[yaacovCR]

Reference: https://github.com/graphql/graphql-over-http/pull/175#discussion_r917932366

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: https://github.com/dotansimha/graphql-yoga/pull/1589#discussion_r944641768

Tagging below discussants from those issues: @enisdenjo @ardatan @glasser @benjie

[yaacovCR]

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: https://github.com/dotansimha/graphql-yoga/pull/1589#discussion_r944641768

[ghmcadams]

Two thoughts:

The term query is well established in the GraphQL spec and the community. This would not be the right place to make that change. If the GraphQL were to change the term, this spec will change as well.

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:

Servers MAY allow any request parameter to be omitted, as long as the missing parameter(s) can be determined from other parts of the request.

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.

[benjie]

https://github.com/dotansimha/graphql-yoga/pull/1589#discussion_r945129583

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

[benjie]

Now I'm back on my computer I found the discussion: https://github.com/graphql/graphql-over-http/pull/175#discussion_r895139192 (emphasis added):

The Apollo implementation of automatic persisted queries works by leaving out query in the particular requests [...] If we want to try to make the Apollo APQ protocol not a violation of this spec, we could make query optional and have some sort of note stating that when query is left out, the server must have some sort of extensions-based mechanism for deriving the Document.

I think it's okay for Automatic Persisted Queries to not be an official GraphQL request - the point of the specification is to define a common set of rules that means a compliant client from any vendor can speak to a compliant server from any vendor. APQ is not part of that flow, it's a specific behavior that the client/server can negotiate as an extension, separate from this specification.

Does the spec actually explain anywhere what to do if the query is not provided? I suppose for GETs this falls into the "Server URLs which enable GraphQL requests MAY also be used for other purposes" thing to some degree.

"A {GraphQL-over-HTTP request} is formed of the following parameters" implies that missing "query" means that it's not a "GraphQL over HTTP request"; then we have "When a server receives a {GraphQL-over-HTTP request}, it must return a well‐formed response." but it's not a GraphQL over HTTP request, so it's up to the server how to handle it, either by classifying it as "other purposes" (in this case serving GraphQL but in a way not 100% compatible with the GraphQL-over-HTTP spec), or it can throw an error.

Maybe we could add a non-normative note to make this clearer? Or indicate that there may be more ways to talk GraphQL with a webserver than strictly defined in this spec?

[enisdenjo]

IMHO, I like @ghmcadams proposal from https://github.com/graphql/graphql-over-http/issues/216#issuecomment-1213638627. Add something like:

Servers MAY allow any request parameter to be omitted, as long as the missing parameter(s) can be determined from other parts of the request.

[benjie]

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 id documentId standardized in GraphQL-over-HTTP 1.1 - a string with arbitrary content (agreed between the client and server via whatever mechanism they see fit) that can be used to identify the document to be used instead of the query parameter.

[ghmcadams]

@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 id parameter that points to a query on the server.

As stated on opensource.com:

...standardization provides a foundation on which innovation can build. Think of standardization as a core set of tools and practices you might applied to all products. Innovation can take the form of tools and practices that go above and beyond this standard

There is a balance. Too specific and innovation is limited. Too broad and compatibility suffers.

[benjie]

There is a balance. Too specific and innovation is limited. Too broad and compatibility suffers.

Completely agree. I think the id field balances this nicely; it does not dictate the structure of the field (other than that it's a string) nor how the server should use the string to figure out the document leaving lots of room for innovation. At the moment we have Relay using id or documentId and Apollo using extensions.persistedQuery.sha256Hash for the same thing - this seems too broad, I know this is something I've had to work around in https://github.com/graphile/persisted-operations for example.

Though I would like to see something more broad than adding an id parameter that points to a query on the server.

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?

[ghmcadams]

@benjie For the most part, that's great. The issue I have is that id is so generic but is being proposed for something so specific as query. It kind of closes the door on additional parameters without creating an inconsistent API.

I'm struggling to find a simple solution currently, but I think we can find one together as the conversation moves forward.

[benjie]

Oh right; just use documentId instead 👍

[michaelstaib]

https://github.com/graphql/graphql-over-http/issues/250

[michaelstaib]

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 query must be allowed to be optional.

[spawnia]

I think we should keep query required for the standard GraphQL-over-HTTP request that every compliant MUST support. Making it optional forces the server to implement alternatives. It does not really matter how broadly, open or specific those alternatives are described in the specification or if they are described at all - the mere possibility makes it much harder to implement a spec compliant server. Forcing every spec-compliant server to support persisted queries is a particularly bad idea, as it forces them to be stateful.

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 GraphQL-over-HTTP request. Giving them a distinct name allows talking about them in a meaningful way, e.g.:

This server supports the following request formats: standard, persisted, flabblgarp. It does not support magical requests.