graphql-rust / graphql-client

Typed, correct GraphQL requests and responses in Rust
Apache License 2.0
1.14k stars 155 forks source link

Generate common types for named types in schema #356

Open nirvdrum opened 3 years ago

nirvdrum commented 3 years ago

I'm working with an API that uses the GraphQL-standard errors field for critical failures and its own UserError type for validation errors. Every operation in this API can respond with an inner errors field. As an example, here's an extracted portion of the IDL:

type UserError {
  message: String!
  path: [String!]
}

type UpdateEnvironmentPayload {
  clientMutationId: String
  environment: Environment
  errors: [UserError!]!
  viewer: Viewer!
}

type UpdateParameterPayload {
  clientMutationId: String
  errors: [UserError!]!
  parameter: Parameter
  viewer: Viewer!
}

While the schema indicates that both UpdateEnvironmentPayload#errors and UpdateParameterPayload#errors share the same type definition, graphql-client generates two distinct types for them. Consequently, writing code that can work over any error is really tedious; each case needs to extract the fields and generate a common error type that I define.

tomhoule commented 3 years ago

I haven't worked on the crate in a long time, but I think it would only generate separate types if you have two different queries? There are other issues about sharing more generated code between queries.

nirvdrum commented 3 years ago

They're multiple queries, yes. Here, they're multiple mutations and each one is used depending on some user action.

I can appreciate the separated types in general. But, when sourcing from the same schema, I think it makes sense to generate a shared type for any user-defined types. As mentioned, it's making work with the schema rather awkward in a way that I haven't run into with other languages. If there's buy-in on the idea, I can take a crack at the code generator. I just don't want to spend the time on it if it turns out there's some general project philosophy around generating new structs for everything.

tomhoule commented 3 years ago

There is a lot of complexity involved with generating fewer types (where do these types live? how are they named? are we really sure they have the same selection sets?), so I think it's definitely better to have a fleshed out design in mind before implementing.

nirvdrum commented 3 years ago

For the time being, I've had some success with this:

#[derive(Debug)]
pub struct UserError {
    pub message: String,
    pub path: Option<Vec<String>>,
}

macro_rules! to_user_errors {
    ($errors:expr) => {{
        $errors
            .into_iter()
            .map(|error| crate::graphql::UserError {
                message: error.message,
                path: error.path,
            })
            .collect::<Vec<crate::graphql::UserError>>()
    }};
}

I don't think we need to be particularly novel with the approach to code generation. Libraries for other languages have solved this issue and I'd just turn to one of them. A common approach I've seen is to provide a mapping from a user-defined type in the schema to a struct/interface/class/module in the target language. In this case, we could even circumvent the common type generation and make it incumbent on the caller to supply it if also providing a type mapping.

felipesere commented 11 months ago

I'm hitting the same "issue" with shared fragments. By virtue of being a shared fragment, they have the same structure. But at the moment they share nothing and turning some of these structures into my own domain types is really tedious.