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(introspectComments): Enhanced introduction comments using tsdocs #1207

Conversation

juanmiguelbesada
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #1097

What is the new behavior?

With this PR comments can be analysed with @microsoft/tsdoc parser to generate enhanced OpenApi spec based on comments rather than using Decorators.

In this PR only adds support for summary and description blocks, but we can keep improving it to add @examples, @params or whatever we need.

This PR keeps BC (while adding a package is not a bc). The new behaviour only works when setting controllerKeyOfComment to null.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@kamilmysliwiec
Copy link
Member

Thanks for your contribution!

With this PR comments can be analysed with @microsoft/tsdoc parser to generate enhanced OpenApi spec based on comments rather than using Decorators.

I'm trying to understand what benefits this change would bring us. I'm guessing the final goal would be to get rid of the current way we extract examples etc from the comment block (using regular expressions) and just use this package instead?

@juanmiguelbesada
Copy link
Contributor Author

I'm trying to understand what benefits this change would bring us. I'm guessing the final goal would be to get rid of the current way we extract examples etc from the comment block (using regular expressions) and just use this package instead?

The main goal is to get rid of decorators as much as possible to provide doc info. Not just examples or summary.
At this moment for example, if you want to provide a description for a operation, you have to use @ApiOperation just for including a description, when it is something that, maybe, you already have done in your comments.

Example:

/**
 * This is what I spect to be the operation summary
 * 
 * @remarks
 * And we already have this operation really well documented in our comments, because we generate more documentation 
 * than just Swagger. AKA Compodoc
 */
@Get("/foo")
@ApiOperation({ description: "And we already have this operation really well documented in our comments, because we generate more documentation than just Swagger. AKA Compodoc" })
public fooAction() {}

vs

/**
 * This is what I spect to be the operation summary
 *
 * @remarks
 * And we already have this operation really well documented in our comments, because we generate more documentation 
 * than just Swagger. AKA Compodoc
 */
@Get("/foo")
public fooAction() {}

IMHO is more easy to use tsdoc than playing with regex

@xuxusheng
Copy link

Look forward to this feature 💪

@exxocism
Copy link

exxocism commented Jul 8, 2022

YES 👍

@OscarMeier
Copy link

OscarMeier commented Sep 1, 2022

It would be really great if it also took @throws and @summary into account:

/**
 * Finds one cat.
 *
 * @summary Get one cat
 * 
 * @throws {BadRequestException} If wrong or missing parameters
 * @throws {NotFoundException} If not found
 *
 * @param {FindOneParamsDto} params
 * @param {LocaleQueryDto} query
 * @returns {Observable<Cat>} The corresponding cat
 */

would get parsed to:

@ApiOperation({
  description: 'Finds one cat.' // already implemented
  summary: 'Get one cat', // new
})
@ApiBadRequestResponse({
  description: 'If wrong or missing parameters', // new
})
@ApiNotFoundResponse({
  description: 'If not found', // new
})
// ApiOkResponse, ApiParam, ApiQuery and ApiBody get already generated by the swagger plugin.
@Get('cat/:id')
findOneCat(
  @Param() params: FindOneParamsDto,
  @Query() query: LocaleQueryDto,
): Observable<Cat> {}

Additionally it would be great if it supported @example definitions in DTOs starting with a newline to be compatible with compodoc:

/**
 * @example
 * "John"
 */
 name: string;

@thawankeane
Copy link

Any update of this?

I loved @OscarMeier suggestion, but apparently this PR is currently paused :(

Should we threat those features into a new issue (or PR)?

@juanmiguelbesada
Copy link
Contributor Author

juanmiguelbesada commented Sep 18, 2022

@kamilmysliwiec Do you still find this interesting? I can back to work on this and add the features @OscarMeier mentioned...

@kamilmysliwiec
Copy link
Member

Sounds great @juanmiguelbesada!

@kenerwin88
Copy link

I want this so bad! :'(

@LeonieKr
Copy link

This sounds so great, I really want this too! 🙏

@Uelb
Copy link

Uelb commented Dec 21, 2022

Looking at the code on this PR, it seems like it's ready for the first part of the feature, that is to accept the summary section as the api operation summary and the remarks section as api operation description ?
@juanmiguelbesada is there something else you need to make it mergable ? I'd be glad to help if that is the case, as I would love to use this feature.

@juanmiguelbesada
Copy link
Contributor Author

Looking at the code on this PR, it seems like it's ready for the first part of the feature, that is to accept the summary section as the api operation summary and the remarks section as api operation description ? @juanmiguelbesada is there something else you need to make it mergable ? I'd be glad to help if that is the case, as I would love to use this feature.

Sorry folks but couldn't get time to work on this yet.

@Uelb First thing will be to rebase master because this branch is pretty old. Then we could:

  1. Merge and work in additional features in other PR
  2. Implement @OscarMeier suggested features

My vote is to finish this up and then work in additional features in a different PR

@Uelb
Copy link

Uelb commented Jan 7, 2023

@juanmiguelbesada Alright, rebasing without knowing the codebase can be tricky, would it be easier in this case to just restart and copy paste the code that is still interesting ? There is a huge number of different commits, I tried to perform the rebase but after 10 minutes, I think it's not worth it. What do you think ? Should I try and reproduce the first feature using part of your code (the summary part) ?

@emretepedev
Copy link

I'm looking forward to this feature!

@kamilmysliwiec
Copy link
Member

Let's track this here #2822

@juanmiguelbesada juanmiguelbesada deleted the features/enhanced-controller-comments branch February 8, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants