possible `GraphQL::Query` refactor

[xuorig]

Right now it's possible for an instance of GraphQL::Query to be in an invalid state. For example Take this query:

document = GraphQL.parse('{ some { query } }')

query = GraphQL::Query.new(
  query: "{ another { query } }"
  document: document
)

This ends up being valid, { some { query } } being executed.

Right now the API for GraphQL::Query is a bit confusing. A query can be provided through:

• the second argument
• the query kwarg
• a pre parsed graphql document

The outcome is not obvious to the user 😁

In the spirit of making it hard to do the wrong thing, what would you think if we provided an interface that exposes the two ways of building queries through their own separate ways?

Maybe Query.new can only take document, and then we can also expose Query.from_query_string / Query.from_document ?

A quick fix would be to make the runtime check for query && document (I remember we had that a while ago, but might have been lost?) but I have a feeling we can probably make it better with something like what i proposed! (Although it might be a slight breaking change 😁 (1.7.0 ? 🐒 )

[rmosolgo]

Thanks for the suggestion, I think it would be great to prevent that nonsensical behavior with two inputs.

But, I think if we're going to make a breaking change to GraphQL::Query, I might like to throw a few more changes in there :) The original implementation did lazy processing for parsing & validating and I've jumped through a lot of hoops to preserve that. But if we want to rework the internals, maybe we could change that behavior too!

Tomorrow or next week I will try to take a look at the other refactors I have in mind and follow up here.

[rmosolgo]

I just spent some time trying to clean up some of the weird lazy preparation stuff, but it basically doesn't work, because you need an uninitialized state during before_query, but you also only want to call before_query on valid queries, which means you have to do preparation before that.

So, since the main issue here was resolved in #957, I'll close this one for now ...

[rmosolgo]

oh, here's that experiment: https://github.com/rmosolgo/graphql-ruby/compare/eager-query-processing