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

auto generate description from comments #585

Merged
merged 5 commits into from
Aug 20, 2020

Conversation

qbcbyb
Copy link

@qbcbyb qbcbyb commented Feb 24, 2020

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?

When enable the plugin by modified nest-cli.json , there is no default description and example in ApiProperty and ApiOperation

What is the new behavior?

Auto generate description and (example or examples) from comment, just like this:

export abstract class Audit {
  /** test on createdAt */
  @CreateDateColumn()
  createdAt;

  // test on updatedAt1
  // test on updatedAt2
  @UpdateDateColumn()
  updatedAt;

  /**
   * test
   * version 
   * @example '0.0.1'
   * @memberof Audit
   */
  @VersionColumn()
  version;

  /**
   * testVersion
   *  
   * @example '0.0.1'
   * @example '0.0.2'
   * @memberof Audit
   */
  testVersion;
}

=>

export class Audit {
    static _OPENAPI_METADATA_FACTORY() {
        return { createdAt: { description: "test on createdAt", required: true, type: () => Object }, updatedAt: { description: "test on updatedAt1\\ntest on updatedAt2", required: true, type: () => Object }, version: { description: "test\\nversion", required: true, type: () => Object, example: "0.0.1" }, testVersion: { description: "testVersion", required: true, type: () => Object, examples: ["0.0.1", "0.0.2"] } };
    }
}

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@qbcbyb qbcbyb force-pushed the descriptionFromCommentInPlugin branch 2 times, most recently from 8657d1b to 6355d5e Compare February 26, 2020 04:48
@BenWildeman
Copy link

BenWildeman commented Mar 10, 2020

@qbcbyb Could I make a suggestion and remove the // normal comments support. if feels wrong to use these as these aren't documentation comments. this ties in better with TS and JSDoc spec. it could potentially show a commented out line in the documentation which is obviously undesired

@kamilmysliwiec
Copy link
Member

Thanks you for contribution. However, I'm not entirely sure why we should even extract comments to enhance the specification. These are 2 completely separate areas. I would recommend publishing this a separate plugin (separate NPM package).

@BenWildeman
Copy link

BenWildeman commented Mar 10, 2020

@kamilmysliwiec it would have been good to have a discussion around this instead of straight up closing it. right now in my project, we have to duplicate the comment for both swagger and JS docs, whereas, this PR would have only required one. the whole point of the swagger plugin you created was to limit the code duplication
image

@kamilmysliwiec
Copy link
Member

As I've pointed out above, you can always publish a separate plugin - CLI allows applying multiple plugins at once.

@BenWildeman
Copy link

BenWildeman commented Mar 10, 2020

Your plugin misses the mark when you have to still use ApiProperty for specifying the description. even more so when ApiProperty originally inferred the types from TS anyway* without the plugin. but sure, more plugins it is

@qbcbyb
Copy link
Author

qbcbyb commented Mar 18, 2020

@qbcbyb Could I make a suggestion and remove the // normal comments support. if feels wrong to use these as these aren't documentation comments. this ties in better with TS and JSDoc spec. it could potentially show a commented out line in the documentation which is obviously undesired

@BenWildeman, Have removed the // normal comments support. And publish in npm: @qbcbyb/nestjs_swagger_plugin

@sjakos
Copy link

sjakos commented Jul 31, 2020

@kamilmysliwiec is this PR lined up to be merged in soon?

@qbcbyb qbcbyb force-pushed the descriptionFromCommentInPlugin branch from 6355d5e to 47d394c Compare August 5, 2020 05:43
@qbcbyb qbcbyb force-pushed the descriptionFromCommentInPlugin branch from e3d8b82 to a2a6721 Compare August 5, 2020 07:27
@kamilmysliwiec kamilmysliwiec merged commit 7b96d7f into nestjs:master Aug 20, 2020
@kamilmysliwiec
Copy link
Member

Thanks for your contribution!

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

Successfully merging this pull request may close these issues.

None yet

4 participants