Skip to content

Make parsing publicly accessible - #776

Open
mirosval wants to merge 4 commits into
graphql-rust:masterfrom
mirosval:make-parsing-publicly-accessible
Open

mirosval wants to merge 4 commits into
graphql-rust:masterfrom
mirosval:make-parsing-publicly-accessible

Conversation

@mirosval

@mirosval mirosval commented Oct 6, 2020

Copy link
Copy Markdown

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.

Adds a parse method on GraphQLRequest
Exposes and documents types that make that possible: Document, Spanning and ParseError
adds a very simple example that does not use any frameworks and just demonstrates how the library works on its own
@ccbrown

ccbrown commented Oct 6, 2020 •

Copy link
Copy Markdown
Contributor

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 #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.

@mirosval

mirosval commented Oct 7, 2020

Copy link
Copy Markdown
Author

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?

@ccbrown

ccbrown commented Oct 7, 2020

Copy link
Copy Markdown
Contributor

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. 👍

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement of existing features or bugfix k::api Related to API (application interface)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants