-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Fix type of extensions
in protocolErrors
#12321
Conversation
🦋 Changeset detectedLatest commit: c300e0d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Docs preview has no changesThe preview was not built because there were no changes. Build ID: 3c41a81872723f523097e726 |
commit: |
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
size-limit report 📦
|
message: string; | ||
extensions?: GraphQLErrorExtensions[]; | ||
}>; | ||
public protocolErrors: ReadonlyArray<GraphQLFormattedError>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The multipart spec mentions that these errors will "never include locations
or path
". I believe this is why this custom object was used here.
Do we want to use Omit<GraphQLFormattedError, 'location' | 'path'>
(or some variant of this) to convey that, or leave this as-is? FWIW, GraphQLFormattedError
marks these fields as optional anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave it as it is for consistency, as you say they are optional.
e88e841
to
789cabf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me. Good catch!
According to the multipart HTTP subscription protocol, fatal tranport errors follow the GraphQL error format which require
extensions
to be a map as its value instead of an array. This PR fixes this by usingGraphQLFormattedError
as its type.