formatError: put all extensions inside 'extensions' property#1284
formatError: put all extensions inside 'extensions' property#1284leebyron merged 2 commits intographql:masterfrom
Conversation
IvanGoncharov
force-pushed
the
errorExtensions
branch
from
2d57c8d to
67a498f
Compare
leebyron
left a comment
leebyron
left a comment
There was a problem hiding this comment.
This is great - just some small suggestions inline.
We'll also need some kind of adoption strategy to help users with the breaking change
| message: error.message || 'An unknown error occurred.', | ||
| locations: error.locations, | ||
| path: error.path, | ||
| extensions: error.extensions, |
There was a problem hiding this comment.
The extensions key should only exist if there are extensions, so perhaps this should if (error.extensions) and return different shapes
| // Extensions | ||
| +[key: string]: mixed, | ||
| }; | ||
| +extensions: { [key: string]: mixed } | void, |
There was a problem hiding this comment.
Based on spec description, this should be:
+extensions?: { [key: string]: mixed }
67a498f to
8bca471
Compare
|
@leebyron Thanks for review 🎉
Fixed in 80ca852. I tried dozens of ways how to initialize optional non-undefined readonly property and it's the simplest one I found.
import { formatError as baseFormatError, /* ... */ } from 'graphql';
{
// other options
formatError(error) {
const { extensions, ...rest } = baseFormatError(error);
return { ...extensions, ...rest };
},
} |
IvanGoncharov
force-pushed
the
errorExtensions
branch
from
8bca471 to
752b28a
Compare
|
Nice - Let's get #1305 landed first and I'll let you rebase over that, otherwise I really like this change. |
IvanGoncharov
force-pushed
the
errorExtensions
branch
from
752b28a to
41295a3
Compare
|
@leebyron I rebased this PR on top of master. |
|
Great work on this! |
3 participants
Mirrors graphql/graphql-spec#407
This is breaking change, however,
extensionsis the relatively new feature that was added in0.12.0.Also, it could be easily workaround by providing
formatErrorproperty to a server:https://github.com/graphql/express-graphql#options
https://github.com/apollographql/apollo-server#options
Unintended changes: I didn't want to add
extensions: undefinedto every error inside tests so I refactored them.