obmarg / cynic

A bring your own types GraphQL client library for Rust
https://cynic-rs.dev
Mozilla Public License 2.0
379 stars 46 forks source link

`cynic-querygen` ignores fragment names #1015

Open Bobrokus opened 2 months ago

Bobrokus commented 2 months ago

Input

query getFeedCollection($id: ID!) {
  # https://shopify.dev/docs/api/admin-graphql/2024-07/queries/collection
  collection(id: $id) {
    products {
      nodes {
        ...FeedProductFragment
      }
    }
  }
}

mutation setProduct($id: ID) {
  # https://shopify.dev/docs/api/admin-graphql/2024-07/mutations/productSet
  productSet(input: { id: $id }) {
    # https://shopify.dev/docs/api/admin-graphql/2024-07/objects/Product
    product {
      id
    }
  }
}

# https://shopify.dev/docs/api/admin-graphql/2024-07/objects/Product
fragment FeedProductFragment on Product {
  title
  id
}

Output

#[derive(cynic::QueryVariables, Debug)]
pub struct GetFeedCollectionVariables<'a> {
    pub id: &'a cynic::Id,
}

#[derive(cynic::QueryVariables, Debug)]
pub struct SetProductVariables<'a> {
    pub id: Option<&'a cynic::Id>,
}

#[derive(cynic::QueryFragment, Debug)]
#[cynic(graphql_type = "QueryRoot", variables = "GetFeedCollectionVariables")]
pub struct GetFeedCollection {
    #[arguments(id: $id)]
    pub collection: Option<Collection>,
}

#[derive(cynic::QueryFragment, Debug)]
#[cynic(graphql_type = "Mutation", variables = "SetProductVariables")]
pub struct SetProduct {
    #[arguments(input: { id: $id })]
    pub product_set: Option<ProductSetPayload>,
}

#[derive(cynic::QueryFragment, Debug)]
pub struct ProductSetPayload {
    pub product: Option<Product>,
}

#[derive(cynic::QueryFragment, Debug)]
pub struct Product {
    pub id: cynic::Id,
}

#[derive(cynic::QueryFragment, Debug)]
pub struct Collection {
    pub products: ProductConnection,
}

#[derive(cynic::QueryFragment, Debug)]
pub struct ProductConnection {
    pub nodes: Vec<Product2>,
}

#[derive(cynic::QueryFragment, Debug)]
#[cynic(graphql_type = "Product")]
// Wrong name, expected "FeedProduct"
pub struct Product2 {
    pub title: String,
    pub id: cynic::Id,
}

The problem

Notice Product and Product2 in the result. Product2 should be named after the FeedProductFragment fragment.

Is this a bug or intended behaviour?

API Reference:

collection query productSet mutation Product object

Bobrokus commented 2 months ago

It does the same thing even when I create a fragment for the product in setProduct.productSet.product:

query getFeedCollection($id: ID!) {
  collection(id: $id) {
    products {
      nodes {
        ...FeedProductFragment
      }
    }
  }
}

mutation setProduct($id: ID) {
  productSet(input: { id: $id }) {
    product {
    # Created a fragment for this
      ...SetProductFragment
    }
  }
}

# New fragment
fragment SetProductFragment on Product {
  id
}

fragment FeedProductFragment on Product {
  title
  id
}
obmarg commented 2 months ago

Is this a bug or intended behaviour?

I'm actually not sure. I agree that it's not ideal, but I couldn't tell you whether or not it was intentional or an oversight - was quite a long time ago that I wrote the generator.

Either way: I'd be happy to accept a PR that fixes this, but I'm unlikely to fix myself anytime soon. I mostly view the generator as a helper, so minor issues that are easily fixed by hand aren't that important to me. Unfortunately there's just too many things that are higher priority. But if you want to try and fix please feel free.

Bobrokus commented 2 months ago

Either way: I'd be happy to accept a PR that fixes this, but I'm unlikely to fix myself anytime soon. I mostly view the generator as a helper, so minor issues that are easily fixed by hand aren't that important to me. Unfortunately there's just too many things that are higher priority. But if you want to try and fix please feel free.

Thanks for the reply! I planned on looking deeper into the generator but found it a little too complex for me the first time I checked it. I might give it a second chance.

I agree that it's not a top priority and I think there are many other things that need a lot more attention than this. Just put your time into things that deserve it, I believe in you.

Btw: I've hit a potential brick wall so expect an issue and maybe, hopefully, a PR.

Bobrokus commented 2 months ago

btw how do you generate your branch names?

obmarg commented 2 months ago

btw how do you generate your branch names?

I use jj instead of git. Automatic branch names are one of it's many excellent features.

Bobrokus commented 2 months ago

I use jj instead of git. Automatic branch names are one of it's many excellent features.

Would you recommend it to a beginner?

obmarg commented 2 months ago

I use jj instead of git. Automatic branch names are one of it's many excellent features.

Would you recommend it to a beginner?

I am honestly not sure. I find it easier than git for a lot of things - but I'm quite far from being a beginner so my perspective may be a bit skewed.

Bobrokus commented 2 months ago

I am honestly not sure. I find it easier than git for a lot of things - but I'm quite far from being a beginner so my perspective may be a bit skewed.

Is it compatible with Github Desktop? I think it should if it's compatible with git.

obmarg commented 2 months ago

Is it compatible with Github Desktop? I think it should if it's compatible with git.

Hard to say without trying it. I'd expect most functionality to work, but there might be some rough edges where jj does things that your standard github desktop user usually wouldn't.