graphql-js
graphql-js copied to clipboard
feat: disable 'did you mean x' during validation
This PR introduces a new property didYouMean on the ValidationContext, that can be used by validation rules for determining whether suggestions should be added to the error message.
Example Usage:
import { validate } from "graphql";
const errors = validate(schema, documentAST, undefined, {
didYouMean: false
});
I did not add the test yet as I first want to start the discussion on whether this is how this should be implemented.
Related issues:
- https://github.com/graphql/graphql-js/issues/2247
- https://github.com/redwoodjs/redwood/issues/4275
I think this is great!
would suggest option name to be enableSuggestions with default of true, perhaps might eventually default to false
see https://github.com/graphql/graphql-js/issues/2247
in that issue, consensus seems to be around disabling introspection to be one and the same with disabling suggestions, but I think this PR might be about the lower level building blocks that allow that direction
There are also some didYouMean calls in enum parseLiteral, I think we can add an execution option (in parallel to the one in validate) that then has to be passed to parseLiterals?
There are also some didYouMean calls in enum parseLiteral, I think we can add an execution option in parallel to the one till validate that then has to be passed to parseLiterals?
I only focused on the validation rules for now. I am happy to also add this to further parts that require it, once there is a consensus on how we should tackle this!
Side note: we should make validate also take one argument which is an object.
I had a discussion with @benjie about this before in the context of adding contextValue to validation rules. One takeaway was "The currently validate signature is useful for the cache use case, [...]". Please correct me if I interpreted this wrong.
validate(a, b, c)
is easier cachable than
validate({
a,
b,
c,
})
For internal reference: https://tortilla-hq.slack.com/archives/CAY2119MX/p1637243621148400
Though I am not against changing the function signature, however, this might be out of the scope of this PR.
I meant that if your validation result is valid independent of context and variables then you can cache the validation result and the next time you see the query you can know it's valid independent of the new context and variables that it comes along with. The actual signature of the function is unimportant so long as context and variables are not included (caching it based on the list of args vs the extracted list of properties isn't much different).
(Also the GraphQL spec itself makes it very clear that validation does not factor in context or variables - anything that does require that (e.g. coercing the variable values) is done during ExecuteRequest.)