-
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
Change usages of the GraphQLError
type to GraphQLFormattedError
.
#11789
Conversation
🦋 Changeset detectedLatest commit: d7fa5fe 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 |
Funny enough, nothing here fails for me locally. I'll have to take a closer look. |
@phryneas is this something we want to move forward with? If so, what is left to be done? |
Thanks for bringing this up! |
src/errors/__tests__/ApolloError.ts
Outdated
|
||
expect(graphQLError).toBeInstanceOf(GraphQLError); | ||
// test equality of enumberable fields. non-enumerable fields will differ | ||
expect({ ...graphQLError }).toStrictEqual({ ...original }); |
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.
After writing this test I'm less convinced that this is the path forward - it's not straightforward to deserialize a serialized GraphQLError
as the serialized variant is missing a lot of information and the constructor doesn't even accept locations
.
size-limit report 📦
|
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
GraphQLError
type to 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.
Thanks for making this change!
See: #11034. This was extracted from that one without the experimental package upgrade. Hmm looks like we'll have to do some refactoring due to apollographql/apollo-client#11789.
Fixes #11787
This would be one direction of going at this.
The other way of going about it would be to change our usage of the
GraphQLError
type to theGraphQLErrorFromResponse
type that I created here, with everything exceptmessage
being optional.We'll have to discuss that internally - putting it on the agenda for next meeting.