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.

Sign up for GitHub

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

fix: generate doc for getters #1780

Merged
merged 7 commits into from
Feb 15, 2024
Merged

fix: generate doc for getters #1780

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a76bfd4
fix: generate doc for getters
akkshitgupta Feb 4, 2024
024a33f
fix: snapshot testing error
akkshitgupta Feb 4, 2024
4327d2a
Merge branch 'master' into fix/ts-playground-doc
jonaslagoni Feb 13, 2024
2814b49
Merge branch 'master' into fix/ts-playground-doc
jonaslagoni Feb 13, 2024
d6fe12e
Merge branch 'master' into fix/ts-playground-doc
Samridhi-98 Feb 15, 2024
473c6c4
Merge branch 'master' into fix/ts-playground-doc
Samridhi-98 Feb 15, 2024
e9c692b
Merge branch 'master' into fix/ts-playground-doc
jonaslagoni Feb 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/generators/typescript/presets/DescriptionPreset.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const TS_DESCRIPTION_PRESET: TypeScriptPreset = {
self({ renderer, model, content }) {
return renderDescription({ renderer, content, item: model });
},
property({ renderer, property, content }) {
Copy link
Member

Choose a reason for hiding this comment

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

We need to make it slightly more advanced, and use the options argument to conditionally write docs for properties or getters depending on whether the option for using getters is sat 🙂

Copy link
Collaborator Author

@akkshitgupta akkshitgupta Feb 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I could not understand from the given issue description, unfortunately.

Can you confirm whether the option we are referring to is passed from the playground UI just like includeDescription or there is any other way for that.

export interface ModelinaTypeScriptOptions extends ModelinaGeneralOptions {
  tsMarshalling: boolean;
  tsModelType: 'class' | 'interface' | undefined;
  tsEnumType: 'union' | 'enum' | undefined;
  tsMapType: 'indexedObject' | 'map' | 'record' | undefined;
  tsModuleSystem: 'ESM' | 'CJS' | undefined;
  tsIncludeDescriptions: boolean;
  tsIncludeJsonBinPack: boolean;
  tsIncludeExampleFunction: boolean;
}

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late reply @akkshitgupta 🙇

So actually I see that it's quite hard to check whether we are using getters or properties, cause it's determined based on the presets that the user includes.

As a quick workaround, lets add the description both for getter and property (does not matter if there are duplicate information for now ✌️

Copy link
Collaborator Author

@akkshitgupta akkshitgupta Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for the late reply @akkshitgupta 🙇

I understand that you are occupied with your work as well. Thank you for being helpful as it encourages me to accept more challenges and continue my journey with AsyncAPI.

does not matter if there are duplicate information for now.

I would like to solve it out rather than just allowing the duplicates if it not much necessary to include description for getters and hold up the PR for now. LMK WDYT

Copy link
Member

Choose a reason for hiding this comment

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

I am getting so confused with all these PRs targeting different languages where some use getters and setters automatically and others have options to turn them on and off..

For TypeScript it's always on and can't be turned off... So your implementation is excellent 😄

getter({ renderer, property, content }) {
return renderDescription({ renderer, content, item: property.property });
}
},
Expand Down
14 changes: 7 additions & 7 deletions test/generators/typescript/preset/__snapshots__/DescriptionPreset.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,7 @@ exports[`Description generation should render example function for model 1`] = `
*/
class Test {
private _stringProp: string;
/**
* Description
* @example Example
*/
private _numberProp?: number;
/**
* @example Example 1, Example 2
*/
private _objectProp?: NestedTest;
private _additionalProperties?: Map<string, any>;

Expand All @@ -32,9 +25,16 @@ class Test {
get stringProp(): string { return this._stringProp; }
set stringProp(stringProp: string) { this._stringProp = stringProp; }

/**
* Description
* @example Example
*/
get numberProp(): number | undefined { return this._numberProp; }
set numberProp(numberProp: number | undefined) { this._numberProp = numberProp; }

/**
* @example Example 1, Example 2
*/
get objectProp(): NestedTest | undefined { return this._objectProp; }
set objectProp(objectProp: NestedTest | undefined) { this._objectProp = objectProp; }

Expand Down
Loading