Skip to content
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

Add 'extensions' to request #976

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

benjie
Copy link
Member

@benjie benjie commented Aug 1, 2022

It's common in the ecosystem to use extensions on a GraphQL request to share additional information with the server; for example Apollo Persisted Queries uses it to indicate the hash of the document to execute.

Currently the GraphQL Spec specifies an "extensions" field on Response but not on Request; I'm trying to determine if this is a purely transport-level concern (i.e. should be specified in GraphQL-over-HTTP, etc), or if it should be specified in the spec proper. IMO the spec should have at least a note indicating the use of the "extensions" field.

The specification of the format of a request is somewhat fuzzier than the format of a response; this is not unexpected because GraphQL is so flexible in how it is consumed, and persisted operations/etc should be spec compliant without having to be specified therein. So the request doesn't really speak of specific keys, more here's the information we need to execute, and "extensions" is not amongst that information. Nonetheless, standardising extensions as a key for expansion would be beneficial to the community.

It's worth noting that GraphQL.js does not include extensions amongst the arguments to graphql(): https://github.com/graphql/graphql-js/blob/699ec58547c34bfeef866a2a4458615d39b16964/src/graphql.ts#L20-L68 . Nor does express-graphql look at extensions on a request as far as I can tell.

@netlify
Copy link

netlify bot commented Aug 1, 2022

Deploy Preview for graphql-spec-draft ready!

Name Link
🔨 Latest commit 3a88989
🔍 Latest deploy log https://app.netlify.com/sites/graphql-spec-draft/deploys/675b245e75755600084ee2d1
😎 Deploy Preview https://deploy-preview-976--graphql-spec-draft.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@benjie benjie added the 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) label Mar 7, 2024
@benjie
Copy link
Member Author

benjie commented Mar 7, 2024

Conclusion from the latest working group meeting is that we should be more explicit about all of the things in request. This was also one of the options laid out by Lee back in 2022:

https://github.com/graphql/graphql-wg/blob/623a7bc464406509e0cf41c847e4e4322d577764/notes/2022/2022-09-01.md?plain=1#L146-L148

Lee: Benjie seems like you have two options: 1) extension may exist, be aware
of it 2) improve section and do a better job of defininging the input to an
execution.

@benjie benjie added 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) and removed 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) labels Mar 7, 2024
@benjie
Copy link
Member Author

benjie commented Mar 7, 2024

(It was agreed that this was something that we probably should have in the spec though. Although not explicitly stated as such, I've moved it to RFC1.)

@benjie benjie force-pushed the request-extensions branch from 5cd2861 to c536c15 Compare July 1, 2024 16:30
@benjie benjie added 📄 Draft (RFC 2) RFC Stage 2 (See CONTRIBUTING.md) and removed 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) labels Dec 12, 2024
@benjie
Copy link
Member Author

benjie commented Dec 12, 2024

@leebyron from Dec 2024 WG meeting:

I might just add a little comment inline in the PR with wordsmithing just to make it really clear that the purpose of this is to avoid implementers adding top level fields that might conflict in the future.

RFC2 was approved.

@benjie
Copy link
Member Author

benjie commented Dec 12, 2024

I've reworded this to attempt to capture Lee's point.

Copy link

@calvincestari calvincestari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice clarification to the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 Draft (RFC 2) RFC Stage 2 (See CONTRIBUTING.md)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants