graphql
State that additional properties are ignored
Servers don't need to validate this, and in fact they should not since a client from a future spec version may send additional keys, which legacy servers should just ignore.
I think we should add this to the spec. The main graphql spec did not state this explicitly for reserved names (those starting with __), and now any current GraphQL UI applications built on Apollo's graphql library crash if they see additional reserved names in the introspection result. (I consider this a bug but 🤷♂️ ). They should be built to ignore additional features added to the spec, and the spec should state this.
We could add something like this:
Servers receiving a request with additional properties MUST ignore the additional properties when processing the request.
Originally posted by @Shane32 in https://github.com/graphql/graphql-over-http/issues/278#issuecomment-1912398114
I feel that it will hamper further extensions to the protocol if option 2 is selected.
Well, we are basically stating that CLIENT implementations that are compatible with this version of the spec MUST not use other root props, and SERVER implementations MUST ignore additional props, in case of future expansion.
If we simply change the MUST to SHOULD, it still doesn't state why it's should vs must, being that the previous sentence stated that all other props are reserved. And doesn't state specifically that other keywords should be ignored.
I'd be fine changing to MUST to SHOULD but I'd still keep the additional sentence for clarification.
If I understand what you're saying correctly then I disagree with you @enisdenjo.
I'm writing this without yet reading the PR changes, because I want to get my thoughts straight first.
All other top-level keys in a request are reserved for future usage (note: which includes official extensions such as persisted operations) and thus clients implementing this version of the specification MUST NOT send additional keys at this top level - instead they may add extra information to extensions.
All other top-level keys in a request are reserved for future usage, but we will do our best to ensure that this future usage degrades gracefully, and thus servers that implement this version of the specification MUST IGNORE all other top-level keys.
Note: I've put this as a "must" for now, but we could loosen it to a "should" at a later time if need be.
In future editions (or with future extensions such as persisted operations, query batching, requesting the response in a different style/format/etc, doing something akin to ETag, adding some kind of smart patching, etc etc) clients may send additional keys; and these new clients may send requests to existing (legacy) servers. These servers cannot understand those keys, but because we want the future client to work with legacy servers, it's essential that legacy servers just ignore the additional keys such that the request degrades gracefully. Should we make a request that would no longer make sense without the additional keys, then we must do so by explicitly breaking the request format for legacy servers - for example removing the query field such that it becomes invalid and adding an id field instead (persisted operations).
Note that rejecting queries with additional keys would be the opposite of ignoring them. Instead, we could write tests that add additional keys of various types and assert that the behavior of the server is unchanged. Note, however, that these keys should be nonsensical, since a server may support future behaviours that our test suite does not yet understand (e.g. they may accept id as part of persisted operations) - I suggest you use UUIDs or other random strings as the keys you test.
