juniper icon indicating copy to clipboard operation
juniper copied to clipboard

Make parsing publicly accessible

Open mirosval opened this issue 5 years ago • 3 comments

Partial fix for #726 Reference #773

I've added parse as a public method on the GraphQLRequest type. I've documented and exposed Document, Spanning and ParseError to allow working with the resulting type.

mirosval avatar Oct 06 '20 19:10 mirosval

I think we could kill two birds with one stone if instead of adding a parse method to GraphQLRequest, we just added a method to get the query. This would address part of https://github.com/graphql-rust/juniper/issues/750.

I think it's also worth it to look into eliminating the schema / scalar template parameter requirement from parsing before making Document public so that we don't have to make a backwards-incompatible change later. If you'd like, I can probably take some time to investigate this further.

ccbrown avatar Oct 06 '20 20:10 ccbrown

Thanks @ccbrown! I'm not sure I understand where you're suggesting to add the method. Are you suggesting that:

  1. We move the parse method elsewhere (as a free fn) AND
  2. Add a simple accessor method to get the query out of GraphQLRequest?

Regarding the Scalar in Document, how about I create another PR, address that and then we can come back to this one and revise it?

mirosval avatar Oct 07 '20 06:10 mirosval

Are you suggesting that:

  1. We move the parse method elsewhere (as a free fn) AND
  2. Add a simple accessor method to get the query out of GraphQLRequest?

Just 2. Then delete the parse method because we could just use the existing parse_document_source function with GraphQLRequest.

Regarding the Scalar in Document, how about I create another PR, address that and then we can come back to this one and revise it?

Sounds good to me. 👍

ccbrown avatar Oct 07 '20 06:10 ccbrown