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 struct fields and enum variants #229

Merged
merged 10 commits into from
Feb 12, 2024

Conversation

timephy
Copy link
Contributor

@timephy timephy commented Feb 10, 2024

This is an "extension" of #187 and tries to also export doc strings not only on "full types" but also on the fields/variants of structs/enums.

This is also a new PR as a replacement for #228, because that PR had a "wrong commit base".

Example

#[derive(TS)]
#[ts(export_to = "tests-out/docs/")]
struct A {
    /// Doc of field
    ///
    /// Testing
    name: String,
}

is exported as (with format feature)

export type A = {
  /**
   * Doc of field
   *
   * Testing
   */
  name: string;
};

and

enum F {
    /// Doc of variant
    ///
    /// Testing
    VarA,
    /// Doc of variant
    ///
    /// Testing
    VarB(),
    /// Doc of variant
    ///
    /// Testing
    VarC {
        /// Doc of field of variant
        ///
        /// Testing
        variant_vield: i32,
    },
}

is exported as (with format feature)

export type F = "VarA" | { "VarB": never[] } | {
  "VarC": {
    /**
     * Doc of field of variant
     *
     * Testing
     */
    variant_vield: number;
  };
};

Notes

These are the "problems" that have to be thought of.

Following notes are a combination of my own observations + input from @escritorio-gustavo.

JSDocs are only shown when starting in a new line (solved)

image image

Inserted doc strings always have a leading newline to solve this.

JSDocs are not applicable to unions (enums)

See microsoft/tsdoc#164

It seems like this is currently not possible, not even depending on the tagging-kind of enums.

@timephy
Copy link
Contributor Author

timephy commented Feb 10, 2024

I would like to ask for your help @escritorio-gustavo to solve these issues 🙏

  • I am unsure about how or even if we should handle docs for newtypes and tuples...
  • You may also please have a look how this implementation behaves when using flatten, which I did not look into yet
  • I have copied the function format_docs because I needed it inside the macros module, but it is defined inside the ts-rs module, and therefore not importable..

Should we create a new utils crate?

I believe there is no way to refactor the usage of format_docs from inside ts-rs to inside macros for the doc generation for "full types", because the doc string needs to be in front of typescript's "export", which is generated from inside ts-ts... But maybe you can find a way - although we can of course just keep it this way.

image

@timephy
Copy link
Contributor Author

timephy commented Feb 10, 2024

(the commit message "exporting docs on struct fields not works" should of course be "now works", lol)

@escritorio-gustavo
Copy link
Contributor

I am unsure about how or even if we should handle docs for newtypes and tuples...

Good question, I assume they currently don't export the docs right? I think we can skip documenting fields for these types as documenting the field of a newtype is kinda pointless and I'm not sure if TSDoc even renders tuple member documentation

You may also please have a look how this implementation behaves when using flatten, which I did not look into yet

Sure, I'll write some tests and push them to the PR

I have copied the function format_docs because I needed it inside the macros module, but it is defined inside the ts-rs module, and therefore not importable..
Should we create a new utils crate?

That could work, but we need to check with @NyxCode.

It's really annoying that we can't just move the function to macros and export it. Maybe we could turn format_docs into a proc_macro to do this? Seems like overkill but could also be an option

@@ -29,6 +29,7 @@ pub(crate) fn newtype(
skip,
optional,
flatten,
docs: _, // TODO: Not sure what to do here with docs yet
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually think discarding is the right thing to do, documenting the field of a newtype seems pointless to me, just document the type itself

@escritorio-gustavo
Copy link
Contributor

I think we can make parse_docs return the properly formatted docs as a Result<String> and eliminate format_docs entirely

@escritorio-gustavo
Copy link
Contributor

I think this is now ready for merging. Thank you so much for your work on this and #187 @timephy!

@escritorio-gustavo escritorio-gustavo merged commit c709437 into Aleph-Alpha:main Feb 12, 2024
4 checks passed
@timephy
Copy link
Contributor Author

timephy commented Feb 14, 2024

Wonderful! Thanks for all the help! 🤝🏽

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

Successfully merging this pull request may close these issues.

2 participants