feat(execution): add max coercion errors option to execution context #4366
Conversation
cristunaranjo
force-pushed
the
feat/execution-opts-max-error-coercing
branch
from
23775d6 to
531c6a2
Compare
website/pages/api-v16/execution.mdx
Outdated
| @@ -29,16 +29,20 @@ const { execute } = require('graphql'); // CommonJS | |||
| ### execute | |||
|
|
|||
| ```ts | |||
| export function execute( | |||
| export function execute({ | |||
There was a problem hiding this comment.
I think this object syntax wasn't intended
website/pages/api-v16/execution.mdx
Outdated
| #### options | ||
|
|
||
| ##### maxCoercionErrors | ||
|
|
||
| Set the maximum number of errors allowed for coercing (defaults to 50). |
There was a problem hiding this comment.
Can we leave the rest of the docs untouched, the changes seem invalid 😅 like from where I sit the only thing that we need to add is options?: { maxCoercionErrors?: number }; as an argument, not an object-field to the docs
There was a problem hiding this comment.
I think you are wrong. I cannot add options? as a positional argument without adding the other missing arguments (fieldResolver, typeResolver, subscribeFieldResolver). I don't know why the function accepts positional arguments or an object, I guess to avoid a breaking change, but the code looks like prefer the object solution and the documentation should reflect that.
I might be wrong but I think my changes make sense
There was a problem hiding this comment.
From the code:
'graphql@16 dropped long-deprecated support for positional arguments, please pass an object instead.',
There was a problem hiding this comment.
That's fair but the docs should still show both methods rather than this kind of driveby change
There was a problem hiding this comment.
I have reverted the changes to the docs. My only concern now is that this new options are only documented in the code, so not sure how many people will benefit from them...
It would be nice to update the docs for v17.
…4366) This PR add `executionOptions` property to the `ExecutionArgs`. Currently only one option can be configured, as is the one I need, but I have built an object so it can easily be extended in the future. The property allows the configuration of the maximum number of errors (`maxErrors`) for coercion. This allows the current hardcoded limit (`50`) to be reduced to a small number to avoid possible DoS attacks. > Also, it updates the execution docs to reflect this new change. I think the documentation was outdated since functions were using positional arguments and now they only accept an object. No longer updates the docs.
This PR add
executionOptionsproperty to theExecutionArgs. Currently only one option can be configured, as is the one I need, but I have built an object so it can easily be extended in the future.The property allows the configuration of the maximum number of errors (
maxErrors) for coercion. This allows the current hardcoded limit (50) to be reduced to a small number to avoid possible DoS attacks.No longer updates the docs.