Validation of variable values

[dkbarn]

I'm curious why it's not possible to validate the values of variables using the validate framework.

For example, suppose I have a very simple validator that wants to assert the value for argument name contains only ascii characters, given this schema:

type Query {
    hello(name: String!): String!
}

I can implement this validator and it will work when the incoming request is this:

{
    'query': '{ hello(name: "foo") }'
}

but when the request is the following, I have no way of seeing the value for the VariableNode $myName from inside the validator:

{
    'query': 'query myQuery($myName: String!) { hello(name: $myName) }',
    'variables': {'myName': 'foo'}
}

It looks like it comes down to this bit of the code:
https://github.com/graphql/graphql-js/blob/main/src/graphql.ts#L118-L122

Is there any reason why variableValues cannot also be passed in and made available to the validate framework?

[glasser]

My understanding is that the whole concept behind "validation" is that it is specifically a function of "operation + schema". So for example, you can cache the result of parsing and validation and not run it multiple times for the same operation and schema if only variables are changing. (Apollo Server does this!)

That said, I would love to be able to separate the getVariableValues stage out of execute so that we can report them with different kinds of error codes (and plugin callbacks!) in our request processing pipeline. @IvanGoncharov do you think that sort of change would be a reasonable PR? Would love to brainstorm exactly how to structure it. (We'd similarly want to pull out the "make sure an operation name is provided if needed and that it matches an operation in the document if provided" step.)

[ArchitectKatherineGraham]

I am interested in this as well. From: David Glasser ***@***.***> Sent: Friday, July 16, 2021 1:52 PM To: graphql/graphql-js ***@***.***> Cc: Subscribed ***@***.***> Subject: [Non-DoD Source] Re: [graphql/graphql-js] Validation of variable values (#3169) All active links contained in this email were disabled. Please verify the identity of the sender, and confirm the authenticity of all links contained within the message prior to copying and pasting the address to a Web browser.

…

________________________________ My understanding is that the whole concept behind "validation" is that it is specifically a function of "operation + schema". So for example, you can cache the result of parsing and validation and not run it multiple times for the same operation and schema if only variables are changing. (Apollo Server does this!) That said, I would love to be able to separate the getVariableValues stage out of execute so that we can report them with different kinds of error codes and plugin callbacks in our request processing pipeline. @IvanGoncharov < Caution-https://github.com/IvanGoncharov > do you think that sort of change would be a reasonable PR? Would love to brainstorm exactly how to structure it. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < Caution-#3169 (comment) > , or unsubscribe < Caution-https://github.com/notifications/unsubscribe-auth/AR2SHLHOMJX5CUTGMX5OECDTYBWUZANCNFSM46RO4E4Q > .

[yaacovCR]

@glasser @IvanGoncharov

i think that is accomplished by #3193 where creating an object of the Executor class with execution args throws with GraphQLErrors if those args contain problems like a “valid” but non-executable document or problems with variables. execute in #3193 catches those errors and reports them early, but Apollo could do whatever it wants with them.

#3193 is a larger refactor than you need, to support what you need, graphql js could just export the buildExecutionContext function and the functions that execute based on that. Having said that, another use case for #3193 and the supporting PR stack is a very welcome start to the day.

[glasser]

Sure, #3193 seems quite attractive from this perspective.

(If we needed to have graphql 15 support, we could polyfill Executor and just have the optimal error handling only work with the non-polyfilled version.)

[yaacovCR]

Update: #3302 supersedes #3193.