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

feat: Add functionality to export doc strings on types #187

Merged
merged 14 commits into from
Feb 9, 2024

Conversation

timephy
Copy link
Contributor

@timephy timephy commented Jan 6, 2024

Overview

I added functionality (and tests) to export doc strings on exported types.

Example

/// Doc comment.
/// Supports new lines.
///
/// Testing
#[derive(TS)]
#[ts(export_to = "tests-out/docs/")]
struct A {
    name: String,
}

is exported as

/**
 * Doc comment.
 * Supports new lines.
 *
 * Testing
 */
export interface A { name: string, }

How

This is achieved by parsing the Vec<Attribute> of an the exported type Item by filtering for "doc" , and then passing that data down to the export layer where generate_decl prepends the doc string before the export definition in TypeScript.

Comments

  • The only thing I am unsure about is how this interacts with the type_def function which is used both for exported types and field types.
  • Only works with "normal doc comments", not with include_str doc comments or other "doc formats".

Related

#52
#102

@bennyhodl
Copy link

Concept ACK. Would love to see this merged

#188

@escritorio-gustavo escritorio-gustavo linked an issue Jan 24, 2024 that may be closed by this pull request
@NyxCode
Copy link
Collaborator

NyxCode commented Jan 24, 2024

Thanks for the PR!
The general structure of the PR looks good, though there are some minor problems I'd like to resolve before merging this.

@escritorio-gustavo
Copy link
Contributor

Hey @NyxCode! What are those minor problems? I can take a look when I have time if you want

@NyxCode
Copy link
Collaborator

NyxCode commented Feb 8, 2024

Hey @NyxCode! What are those minor problems? I can take a look when I have time if you want

oh, seems like i didnt submit the review, sorry.

ts-rs/src/export.rs Outdated Show resolved Hide resolved
ts-rs/src/lib.rs Outdated Show resolved Hide resolved
macros/src/types/mod.rs Outdated Show resolved Hide resolved
@escritorio-gustavo
Copy link
Contributor

oh, seems like i didnt submit the review, sorry.

No problem, I have also done that with a couple PRs lol

Copy link
Contributor

@escritorio-gustavo escritorio-gustavo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the notable changes I made on top of what @NyxCode asked

Comment on lines +121 to +134
attrs
.into_iter()
.filter_map(|a| match a.meta {
Meta::NameValue(ref x) if x.path.is_ident("doc") => Some(x),
_ => None,
})
.map(|attr| match attr.value {
Expr::Lit(ExprLit {
lit: Lit::Str(ref str),
..
}) => Ok(str.value()),
_ => syn_err!(attr.span(); "doc attribute with non literal expression found"),
})
.collect::<Result<Vec<_>>>()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Managed to get rid of those panic!s

@@ -116,6 +116,24 @@ pub fn parse_serde_attrs<'a, A: TryFrom<&'a Attribute, Error = Error>>(
.into_iter()
}

/// Return a vector of all lines of doc comments in the given vector of attributes.
pub fn parse_docs(attrs: &[Attribute]) -> Result<Vec<String>> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always prefer &[T] over &Vec<T>

@timephy
Copy link
Contributor Author

timephy commented Feb 9, 2024

Hi,

First of all, thanks for refactoring the code and solving problems!
Especially that you removed docs from all function calls, and moved it into Attr structs. - This bothered me as well, but I was not sure about how to solve it better at the time.

Note: I made a commit I then realized was unnecessary, because the TS trait already implemented a fallback for the DOCS field. Therefore I removed those changes again.


I believe it is possible to also add docs for struct fields and enum variants... (#188)
Do we want to implement and include this in this PR or implement it somewhen else in another?

@timephy
Copy link
Contributor Author

timephy commented Feb 9, 2024

Okay, I tried to implement docs for fields+variants as well..

But there were several complications that we need to resolve, I'd like to ask for your help doing that / making the right decisions. I have committed these changes to a separate branch and opened a new PR #228 for that.

I suggest implementing this PR first / implementing the docs for fields/variants in that new PR.

Thanks again for the help/coop!

@escritorio-gustavo
Copy link
Contributor

I believe it is possible to also add docs for struct fields and enum variants

I think struct fields shouldn't be too dificult, but we have to make sure to handle #[ts(flatten)] correctly, which always complicates things

Enum variants are a whole other beast though. They are represented as a type union of the different variants and there is no convenient way to add docs for specific type union members in TS.

We could implement #86 and document each variant type, but at the end of the day, we'd do type Enum = VariantA | VariantB, and when using Enum the docs for VariantA and VariantB would not show up anywhere

@escritorio-gustavo
Copy link
Contributor

Do we want to implement and include this in this PR or implement it somewhen else in another?

Definetly another PR, creating #228 was a good call

I suggest implementing this PR first / implementing the docs for fields/variants in that new PR.

Agreed, since all the changes requested have already been made, I will merge this PR now

@escritorio-gustavo escritorio-gustavo merged commit e7a462a into Aleph-Alpha:main Feb 9, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

Add documentation to exported types
4 participants