graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.04k stars 2.02k forks source link

merge ast trees #1428

Open terion-name opened 6 years ago

terion-name commented 6 years ago

Feature request

Another feature that is extremely useful and utils are lacking it: merging ast trees.

Primary use case: merging two queries (selections) to dynamically construct queries. There are plenty of cases where it is needed, mainly in libraries and in cases of graphql to graphql communication.

PS It would be great if somebody can say how to do this properly now (simple deep merge doesn't wotk properly)

IvanGoncharov commented 6 years ago

@terion-name Merging is pretty easy by itself if you don't need to handle conflicts:

{
  foo(arg: 1)
}
{
  foo(arg: 2)
}

Would be merged into:

{
  foo(arg: 1)
  foo(arg: 2)
}

Which is invalid query constructed from two valid ones.

So do you need conflict resolution or simply merging ASTs would be enough?

terion-name commented 6 years ago

@IvanGoncharov common scenario: I have an application that utilizes a backend that has graphql interface (it could be a remote api, a service alike prisma or postgraphile). I have a type that has calculated fields. Let's assume Home { avgRating }. avgRating is calculated from ratings { stars }. So to calculate avgRating I need to have ratings { stars } in request that is passed to remote backend. So I need to ensure this selection. When a client queries Home { avgRating } I need to transform it to Home { avgRating ratings { stars } }. So in a resolver I have info.fieldNodes[0].selectionSet (an AST tree), I can construct a desired selection set with { ratings { stars } } (also AST tree) and I need to merge them recursively to properly combine selections.

It is a simple example, there are many more deep and complex situations where one needs to properly deeply merge (combine) several AST trees

IvanGoncharov commented 6 years ago

@terion-name It's pretty complex to handle all corner cases even in such small example, e.g. user sens following query:

{
  Home {
    avgRating
    ratings: someOtherField
  }
}

Or he provide different parameters to the same field:

{
   Home {
     avgRating
     ratings(first: 5) {
       stars
     }
   }
}

In both cases, you can't simply merge and you need to do some tricks with field aliases. AFAIK, graphql-tools and graphql-bindings have some functionality to do that.

Would be great to have something like that in graphql-js but it should cover all edge cases and have an extensive set of test cases to be merged.

ivome commented 6 years ago

I have thought about this too quite a bit.

Here is an approach that could work IMO:

  1. Merge the AST trees of the query while automatically aliasing fields that have naming conflicts. In this step you would have to store the mapping of the queried fields to the original query fields per query to later be able to construct the response objects.
  2. Run the resulting query against the GraphQL server or existing functionality of the graphql-js library.
  3. Transform the result object back to an array of the responses of all individual queries and return.

Interface could look something like this:

type MergeableQueryDefinition = {
   document: DocumentNode,
   operationName?: string,
   variableValues?: ?{ [variable: string]: mixed }
}

executeMergedQueries(
  schema: GraphQLSchema,
  documents: Array<MergeableQueryDefinition>,
  rootValue?: mixed,
  contextValue?: mixed,
): MaybePromise<Array<ExecutionResult>>

I am also interested in this kind of functionality.

timoshisa commented 2 years ago

@ivome did you get any further with this?

tom-sherman commented 1 year ago

Coming back around in 2023, did anyone get to an implementation of this?

I think it would be super useful with React Server Components combined with DataLoader: you could do many graphql queries throughout the component tree and they'd all get merged into a single graphql request for that render.

development-vargamarcel commented 5 months ago

Just putting this here, admittedly without knowing enough: lodash's .merge() could maybe solve this, maybe .mergeWith(), could provide the ability to make it perfect. Before knowing the existance of ast (i know..., i am still a noob) , i used my own object notation for gql operations and just merged them the way I pointed above.