Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate arguments for delegateToSchema #550

Closed
schickling opened this issue Dec 21, 2017 · 4 comments
Closed

Validate arguments for delegateToSchema #550

schickling opened this issue Dec 21, 2017 · 4 comments

Comments

@schickling
Copy link
Contributor

It's a common scenario to modify the args parameter which is required for delegateToSchema. It can easily happen that you're making a typo here or somehow else don't match the underlying schema for the input types.

Would be great if delegateToSchema would validate the schema of the arguments before delegating to the actual schema.

Related: prisma-labs/prisma-binding#16

@stubailo
Copy link
Contributor

Hmm, is that something we could rely on the underlying schema for? It seems like the direction this would head would be reproducing all of the validation on both ends of the call which would definitely impact performance.

@kbrandwijk
Copy link
Contributor

kbrandwijk commented Dec 21, 2017

@stubailo From the spec: "Typically validation is performed in the context of a request immediately before execution, however a GraphQL service may execute a request without explicitly validating it if that exact same request is known to have been validated before.".

In this case, the request has not been validated before. Also, a call to graphql(schema, query) is validated before execution. It feels like delegateToSchema -> execute is bypassing this normal execution flow, executing the request without any validation.

To me, that's not 'validation on both ends of the call', it's following spec by doing validate -> execute.

@stubailo
Copy link
Contributor

Hmmm, I guess I'm thinking primarily about remote schemas. But yeah that makes sense.

@yaacovCR
Copy link
Collaborator

This is likely addressed by validation that has been present since at least v4, which can optionally be disabled with skipValidation set to true

https://github.com/ardatan/graphql-tools/blob/master/packages/delegate/src/delegateToSchema.ts#L216

Closing this stale issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants