-
Notifications
You must be signed in to change notification settings - Fork 64
Make it clear that other keys are reserved #278
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
Conversation
I don't think this will break anything. Looks good to me! It is kinda conventionalized that all metadata lands on the |
Follow-up question: should I add a test to graphql-http reflecting this? It's now illegal to have extraneous keys in the request body. |
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. |
Hmm, is the "must" then confusing?
If we add more fields in future spec versions, we can just add the fields/relax the compliance tests? |
I think we should add this to the spec. The main graphql spec did not state this explicitly for reserved names (those starting with We could add something like this: Servers receiving a request with additional properties MUST ignore the additional properties when processing the request. |
That must be a bug. All potential field names that start with |
Fixes #271
The
extensions
object exists so that vendors may add their own properties to requests/responses/errors/etc. All top-level keys are reserved by the spec. This PR makes this explicit for JSON-encoded requests.@enisdenjo Will this break any of the existing implementations, other than Relay's persisted queries network layer recommendation?