Upgrade to graphql 16 #686
Conversation
| extensions: { | ||
| value: err.extensions || undefined | ||
| } | ||
| }) |
There was a problem hiding this comment.
I got this error error.toJSON is not a function
That's because they changed the formatError impl to simply do return error.toJSON();
I thought it was safer to move to new GraphQLError
Should I go back to Object.create?
Doing Object.create(GraphQLError.prototype, would also solve the issue
| @@ -1,126 +0,0 @@ | |||
| 'use strict' | |||
There was a problem hiding this comment.
Any particular reason on why this file has been deleted? I mean, just to make sure it wasn't by mistake 😬
There was a problem hiding this comment.
it's dependencies are not yet compatible with graphql 16
Discussion here --> #628 (comment)
There was a problem hiding this comment.
rather than delete, isn't this something we can upgrade to work with graphql@16? In any case we should think about a better way other than deleting. Even if they are not compatible, I assume they will be at some point, meaning that we may want to keep the example around in some way
|
what's the plan with this PR? I assume that once we make mercurius compatible with graphql@16 we break compatibility with 15. Do we release a new semver major version where we introduce this breaking change? /cc @mcollina |
| @@ -129,6 +129,7 @@ function getStubTypes (schemaDefinitions, isGateway) { | |||
|
|
|||
| function gatherDirectives (type) { | |||
| let directives = [] | |||
| /* istanbul ignore else seems that extensionASTNodes is always provided in graphql16+, we are keeping this check for safety */ | |||
There was a problem hiding this comment.
what is this comment doing?
There was a problem hiding this comment.
In graphql 15 extensionASTNodes could be either null or Array
Now it is only Array but I thought that was safer to keep the check.
I've added this comment to get the 100% coverage and explain why I didn't get rid of it.
There was a problem hiding this comment.
ok so this comment mixes an istanbul command with human-readable information.
I would recommend we keep the two separated, meaning, 2 comments
fixes #628