Skip to content
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

Query type and QueryProjectionRoot classes are not generated for client code #589

Open
cjelger opened this issue Aug 17, 2023 · 6 comments

Comments

@cjelger
Copy link

cjelger commented Aug 17, 2023

Hello. I'm evaluating dgs-codegen and when generating client code, it seems it doesn't generate any type nor projection classes for the root Query type. If I want to write a client request using multiple top-level fields under the root type, I'd expect to have classes generated for the root Query type, like Query, QueryProjectionRoot, and QueryGraphQLQuery.

Is this intentional? For example with a simple schema as

type Query {
  foo: String
  bar: String
}

I'd expect to also get all the Query classes to be able to query the two fields at the same time. I can write them by hand, but it's not really nice because I'd have to change them each time a new top-level field is added under Query.

@cjelger cjelger changed the title Query type and QueryProjectRoot classes are not generated Query type and QueryProjectRoot classes are not generated for client code Aug 17, 2023
@srinivasankavitha
Copy link
Contributor

This is by design. We generally make requests for single queries so far and therefore have not implemented a top-level Query type. Could you describe how would you use the client API in that case?

@cjelger
Copy link
Author

cjelger commented Aug 21, 2023

Thank you @srinivasankavitha for your reply.

Well I'd simply do something like

QueryGraphQLQuery query = QueryGraphQLQuery
                        .newRequest()
                        .build();

QueryProjectionRoot root = new QueryProjectionRoot()
                        .foo()
                        .bar();

GraphQLQueryRequest req = new GraphQLQueryRequest(query, root);
String queryString = req.serialize();
... // send request
Query queryData = response.dataAsObject(Query.class);

The root Query, Mutation, and Subscription fields are GraphQL query fields like any other ones (except they are at the root of the schema). If one cannot make multiple queries at the same time, IMHO it's no longer very useful to use GraphQL ;-)

@cjelger cjelger changed the title Query type and QueryProjectRoot classes are not generated for client code Query type and QueryProjectionRoot classes are not generated for client code Aug 21, 2023
@srinivasankavitha
Copy link
Contributor

Thanks for the description. We didn't design for this scenario since this is not something we actively use internally at Netflix. That said, it would involve a breaking change to the existing API to support this, so this is not something we would prioritize at the moment either (also since we don't really need this feature).

@cjelger
Copy link
Author

cjelger commented Aug 22, 2023

Thank you for your reply. I understand that you don't need that feature, but it's a pretty fundamental feature for any GraphQL schema, too bad for us :-(

That said, it would involve a breaking change to the existing API to support this

TBH I don't really understand why it would be a breaking change. Everything else would continue to work exactly as before, it would simply be an extra feature to make queries. You might need to handle the root types in a special manner because of the way you serialize the queries to string, but IMO you don't have to make any breaking change.

@srinivasankavitha
Copy link
Contributor

I think we may be able to get away without making a breaking change, but it will need some more investigation. The example you pointed out is a simple use case. It does get complicated to specify the projection fields on each query field, especially when the query depth is more. But we may be able to handle that differently. In any case, we'll keep the issue open so we can prioritize it in future. I do agree it is a nice feature to have in any case, just not something we can prioritize at the moment.

@lukastaegert
Copy link

lukastaegert commented Jul 8, 2024

To add another perspective, at our client we have established the pattern that mutation return types have a field of type Query. The rationale is that the mutation updates some state and the client can then in the same request get any data that they depend upon in the updated form while decoupling client and server.

A minimal schema could look like this

type Query {
  mutatedValue: String
}

type Mutation {
  performUpdate: PerformUpdateResponse
}

type PerformUpdateResponse {
  query: Query
}

Allowing for queries such as

mutation PerformUpdate {
  performUpdate {
    query {
      mutatedValue
    }
  }
}

At the moment, we cannot directly generate classes for this schema because the class generated for PerformUpdateResponse references a non-existing Query class.
We can get around this by defining the class manually, but of course it would be nicer to have the auto-generated class here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants