-
-
Notifications
You must be signed in to change notification settings - Fork 8
Add option to include the jsdoc of the rule options #18
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
base: main
Are you sure you want to change the base?
Conversation
Can you add a test where this option is on? Thanks |
sure, I'll add it in a few moments |
done! |
*/ | ||
type _PluginRuleWithDescriptionInPropertiesSchemaBId = string | ||
interface _PluginRuleWithDescriptionInPropertiesSchemaSchemaId { | ||
a?: _PluginRuleWithDescriptionInPropertiesSchemaAId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the jsdocs should be here to make it effective on the userland, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I think that too, but the snapshot in the tests places it there, and all tests will pass, it's strange but works xD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, snapshot only tests changes, but not correctness. If snapshot shows that, it means the implementation should be updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good point, I'll try out how to fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently, the json-schema-to-typescript-lite
package
in the generateInterface and generateStandaloneType functions
the jsdocs is conditioned by ast.standaloneName
if has standaloneName
the jsdoc is placed in the top level type definition.
It can be fixed by adding an option to prefer the jsdoc of the options inside the interface instead of the top level type, (I can send a pr for this), or it can be left as is, and used in this way, because the original package also has this behavior (bcherny/json-schema-to-typescript), generateInterface and generateStandaloneType functions
Description
Add option to include the jsdoc of the rule options
Linked Issues
#17