Open tomhoule opened 6 years ago
I've personally never touched the validation code so I can't speak to the feasibility or difficulty, but to me it sounds like a great idea.
Cool, I'm in vacations now but I'll try to find time to attempt this when I'm back.
@tomhoule Did look into this? I have the exact need you mentioned of building a CLI tool to validate queries against a schema. If you didn't spend time on it I might wanna give it a shot 😄 But I make no promises.
I haven't had time for open source work recently, sadly, so I haven't done anything on that front. Feel free to go ahead :)
I have started looking into this but since I'm new to this project I have a few questions:
juniper::execute
starts off by validating the variables and query against juniper's representation of a schema (RootNode<QueryT, MutationT, S>
). So pulling out a function that only does the validation is quite easy, however it requires a juniper::RootNode
type. "graphql-client" doesn't have such a type because it uses "graphql-parser". So is the AST mapping that @tomhoule mentioned mapping from graphql_parser::schema::Document
to juniper::RootNode
?juniper::ast
, but most of that module is currently private. Should it be made public or moved somewhere else? The existing validation code also depends heavily on juniper::ast
.Is the end goal to have functions like these
// For use in juniper, with better name
fn validate_for_juniper(
query: &'a str,
variables: HashMap<String, _>,
root_node: &'a RootNode<QueryT, MutationT, S>,
) -> Result<(), Vec<ValidationError>> {
// ...
}
// For use in graphql-client
fn validate(
query: &'a str,
variables: HashMap<String, _>,
schema: &'a str,
) -> Result<(), Vec<ValidationError>> {
let root_node = schema_to_root_node(schema);
validate_for_juniper(query, variables, root_node)
}
For your points 1 and 2, that is what I imagined yes. In graphql-client, we could either: 1. parse the query twice (one for the graphql-parser AST, another for the juniper AST to perform validation with), 2. make juniper's query parser public and use that (it would be a bit of work though), 3. define a mapping from graphql_parser's AST to juniper's query AST.
Solution 1 is obviously slow but I think it would be acceptable, since it all happens at compile time.
Whatever the final solution, the query and schema roots will have to be exposed, I think.
I'm interested in being able to validate separately from execution, I'm happy to work on this if no one is currently working on it
@allancalix we're not against this, go ahead!
Regarding the solution, I don't see reasons to move the validation into a separate crate. But making a nice public API for using it - quite good to go 👍
Sorry it's been a while, I ended up going in a different direction because at the time there was no way to initialize a schema from SDL files which I was looking to do. Going to unassign myself
I am working on improving the graphql-client crate and one place where it could use better ergonomics is query validation. Since it is defined by the spec and already implemented by Juniper, I feel like it would be wasted work, and it would be vastly preferable to share one high quality Rust crate for validation of a GraphQL query against a schema.
Of course this would be using the juniper ASTs for the query and the schema, but I think mappings could be defined without too much work. Juniper is the only crate where performance matters, since graphql-client validates queries at compile-time, so the conversion overhead/parsing twice is acceptable there.
This is not the only use-case for such a crate, it could be useful on its own with a very thin wrapper CLI. One use-case I have at work would also be to warn when a query is using deprecated fields.
Since I am not very familiar with the Juniper codebase, I may be missing difficulties. Do you think this is desirable/possible? I would be happy to make a PR or help in any other way.