-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
merge ast trees #1428
Comments
@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? |
@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). It is a simple example, there are many more deep and complex situations where one needs to properly deeply merge (combine) several AST trees |
@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. Would be great to have something like that in |
I have thought about this too quite a bit. Here is an approach that could work IMO:
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. |
@ivome did you get any further with this? |
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. |
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. |
Hey @IvanGoncharov , is anyone currently working on this? If not, I can give it a try! |
We should probably migrate the method from GraphiQL: https://github.com/graphql/graphiql/blob/8fbac78ae43a2b245aad0203cb8269f38e0c105f/packages/graphiql-toolkit/src/graphql-helpers/merge-ast.ts#L109 |
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)
The text was updated successfully, but these errors were encountered: