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

Use rustdoc comments as graphql desc? #62

Closed
mwilliammyers opened this issue May 9, 2020 · 17 comments
Closed

Use rustdoc comments as graphql desc? #62

mwilliammyers opened this issue May 9, 2020 · 17 comments
Labels
enhancement New feature or request

Comments

@mwilliammyers
Copy link
Contributor

mwilliammyers commented May 9, 2020

I am looking to replace juniper with this library and I was looking through the docs and it looks like all the examples use proc macro attributes to add a description. Is it currently (or planned) possible to parse rustdoc comments for the graphql description?

@mwilliammyers mwilliammyers changed the title Parse rustdoc comments? Use rustdoc comments as graphql desc? May 9, 2020
@sunli829
Copy link
Collaborator

sunli829 commented May 9, 2020

I'm not sure rust's procedural macros support this, but I'll add them if possible. 😁

@sunli829
Copy link
Collaborator

sunli829 commented May 9, 2020

I have confirmed the possibility and I will add it tomorrow. It is a very good suggestion, thank you.😀

@sunli829
Copy link
Collaborator

sunli829 commented May 10, 2020

I am very excited. Repeating the description twice is too annoying. It is very convenient to get the desc directly from rustdoc.

@sunli829 sunli829 pinned this issue May 10, 2020
@sunli829
Copy link
Collaborator

I have completed this feature and updated the example code of starwars. 😁

https://github.com/async-graphql/examples/tree/master/models/starwars

@sunli829
Copy link
Collaborator

Please upgrade to version 1.11.0, which already contains this feature.😁

@sunli829 sunli829 unpinned this issue May 11, 2020
@sunli829 sunli829 added the enhancement New feature or request label May 11, 2020
@Weasy666
Copy link
Contributor

@sunli829 is it also possible to use the structs rustdocs? See the example below.

#[derive(Deserialize, Debug)]
/// A company is a legal entity representing an association of people, whether natural,
/// legal or a mixture of both, with a specific objective/goal.
pub struct Company {}

#[Object]
impl Company {
    /// Globally unique identifier of a company.
    pub async fn id(&self) -> &ID {
        todo!()
    }
    /// The company name.
    pub async fn name(&self) -> String {
        todo!()
    }
}

Docs are currently only generated if i add the rustdocs to impl Company, but that would make it necessary to duplicate the docs on the struct and on the impl, because docs on the impl are not shown in the IDE.

And another question, is it possible to add docs to a generated ...Connection and a ...Edge? And...is it possible to modify the docs of scalars that are delivered with async-graphql, for example, can i as a user of the crate modify the docs of DateTime?

@Kestrer
Copy link
Collaborator

Kestrer commented Oct 26, 2020

I suppose for the first one we could add a derive macro Description or something that derives a trait

trait Description {
    fn description(&self) -> &'static str;
}

And we could either have #[Object(use_type_description)] where it uses that trait, or the other way around where no docs on the impl assumes that it implements Description, and you'd have to write #[Object(no_docs)] to disable that.

For the second one we could either:

  • Make it easier to make type wrappers
  • Provide a method on the schema which overrides type descriptions (e.g. Schema::build(q, m, s).override_description::<DateTime>("custom datetime docs"))
  • Use the inventory crate and provide a macro like
    override_docs! {
        /// custom datetime docs
        DateTime
    }

@Weasy666
Copy link
Contributor

Hm...for the second one, i think i would like the macro override_docs! best, if it also changes the docs that pop up in the IDE.

The first one, the trait, i think that won't work with IDE docs. I would really like to not need to replicate the docs from the struct to the impl. To make it more clear, i will reuse but shorten the previous example:

With the following code, i get the docs shown by the IDE as a popup, see screenshot, but not in graphql.

#[derive(Deserialize, Debug)]
/// A company is a legal entity representing an association of people, whether natural,
/// legal or a mixture of both, with a specific objective/goal.
pub struct Company {}

#[Object]
impl Company {
    /// Globally unique identifier of a company.
    pub async fn id(&self) -> &ID {
        todo!()
    }
}

grafik

When i put the docs on the impl, then they are shown in graphql, but not in the IDE.

#[derive(Deserialize, Debug)]
pub struct Company {}

#[Object]
/// A company is a legal entity representing an association of people, whether natural,
/// legal or a mixture of both, with a specific objective/goal.
impl Company {
    /// Globally unique identifier of a company.
    pub async fn id(&self) -> &ID {
        todo!()
    }
}

grafik

Ideally, docs that are on the struct should be used for graphql too.

@Kestrer
Copy link
Collaborator

Kestrer commented Oct 26, 2020

So what I meant for the first one is that you'd write:

#[derive(Deserialize, Debug, async_graphql::Description)]
/// A company is a legal entity representing an association of people, whether natural,
/// legal or a mixture of both, with a specific objective/goal.
pub struct Company {}

#[Object(use_type_description)]
impl Company {
    /// Globally unique identifier of a company.
    pub async fn id(&self) -> &ID {
        todo!()
    }
}

And in the expansion it would write:

description: Some(<Self as ::async_graphql::Description>::description()>),

if it also changes the docs that pop up in the IDE.

No, it won't - there is no way to change the actual Rustdoc attributes of another item. All that macro does is to add that doc string to a global list of doc strings for that type, so when you register the type in the schema it uses the global list of doc strings. I don't really like that solution because inventory is a bit of a hack.

@Weasy666
Copy link
Contributor

Ah...i misunderstood you. I am not really deep into the howto's of macros, so maybe the my question is somewhat silly. Currently, the Object macro reads the rustdocs of wherever it is put on? Can a macro read the rustdocs of another struct or is it restricted to where it is placed?

No, it won't - there is no way to change the actual Rustdoc attributes of another item.
Then i think scalarwrapper is the way to go. If you can simply wrap the priovided DateTime, maybe like this

#[derive(ScalarWrapper)]
/// My super doc comment.
pub struct MyDateTime(DateTime);

then one would easily get both, the IDE popup when working with that struct and the GraphQL docs. Ideally, the macro should use the name of the inner type for the graphql type, or at least make renaming possible. Maybe even two different wrapper macros, for a specific purpose.
A ScalarDocsWrapper for just changing the docs, which then uses the name of the inner type for generation.
And a ScalarWrapper that allows renaming and also changing the docs. Dunno if that really makes sense. Just some thoughts.

@Kestrer
Copy link
Collaborator

Kestrer commented Oct 26, 2020

Currently, the Object macro reads the rustdocs of wherever it is put on?

Yes

Can a macro read the rustdocs of another struct or is it restricted to where it is placed?

No, this is why we have to add another derive macro and use the trait to communicate between the two.

I don't really like the ScalarWrapper name, but I don't have any other suggestions. @sunli829, what do you think?.

@sunli829
Copy link
Collaborator

sunli829 commented Oct 26, 2020

I think this looks better. 😂

#[derive(Deserialize, Debug, async_graphql::Description)]
/// A company is a legal entity representing an association of people, whether natural,
/// legal or a mixture of both, with a specific objective/goal.
pub struct Company {}

#[Object(use_type_description)]
impl Company {
    /// Globally unique identifier of a company.
    pub async fn id(&self) -> &ID {
        todo!()
    }
}

I think there is no need to add a wrapper type just to be able to shown the document in the IDE, which will cause more trouble.

@Weasy666
Copy link
Contributor

@Koxiaet Thanks for clarifying!

@sunli829 The wrapper would be for overriding the docs of scalars that exist inside of async-graphql, like ID or DateTime.

@sunli829
Copy link
Collaborator

@sunli829 The wrapper would be for overriding the docs of scalars that exist inside of async-graphql, like ID or DateTime.

Ah, this seems to be useful.

@sunli829
Copy link
Collaborator

sunli829 commented Oct 26, 2020

Let me think about this wapper, I first add the use_type_description attribute.

@Weasy666
Copy link
Contributor

Oh...i think we missed one, what about the generated ...Connection and a ...Edge types? Is it possible to add GraphQL docs to them?

@mwilliammyers
Copy link
Contributor Author

mwilliammyers commented Oct 27, 2020

While we are at it 😄 : I have been meaning to verify this as of v2.0.0, but if I recall correctly, graphql docs were not generated for enum variants and custom scalars. Is that still true?

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

No branches or pull requests

4 participants