-
Notifications
You must be signed in to change notification settings - Fork 209
feat: Add --raw-jsdoc option to include full JSDoc in schema #2224
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: next
Are you sure you want to change the base?
Conversation
This adds support for a new CLI/config option `--raw-jsdoc` (`rawJsDoc`), which includes a `rawJsDoc` field in the generated JSON Schema. This field preserves the full JSDoc comment block *as is* (after removing asterisks), without any formatting, tag filtering, inheritance, etc. - Adds CLI and config option: `--raw-jsdoc` / `rawJsDoc` - Adds `rawJsDoc` keyword to Ajv validator - Enhances `ExtendedAnnotationsReader` to extract raw JSDoc when available - Includes thorough tests in `test/config/jsdoc-raw` - Updates documentation (README) Related to discussion in vega#1544 and complements `--markdown-description` (vega#1773)
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.
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Files not reviewed (1)
- test/config/jsdoc-raw/schema.json: Language not supported
I already did not like going out of the JSDoc spec with |
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.
Wrongfully approved.
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.
Instead of another command, we should combine it with --markdown-description and make the option an enum.
Awesome, enum value seems better :) |
That was actually my first idea too — to reuse the Replacing So, while I initially wasn't a big fan of introducing a new field either, adding a separate @domoritz
That would definitely simplify the config surface and avoid introducing another CLI flag, so if we all agree this is a good approach, I can rework the PR in that direction. |
It's an opt-in feature, so it shouldn't break anything by default. The user should know why they chose it.
It would be good if we could remove |
By "break the default behavior," I meant that overwriting the There are real cases where both are useful: for example, showing a concise If we allow only one at a time, then in such cases the user would have to generate the schema twice with different options and invent a strategy to merge them. That doesn't feel like a clean approach. At the same time, I can imagine cases where you do want the raw JSDoc to go into the
Agreed! The only question is: what option values should we support? |
So, what are we doing? 🙂 |
Hmm, after seeing this, I think actually what you have (no enum) makes more sense. It lets users enable and disable whatever they need. |
@@ -47,6 +47,7 @@ By default, the command-line generator will use the `tsconfig.json` file in the | |||
-e, --expose <expose> Type exposing (choices: "all", "none", "export", default: "export") | |||
-j, --jsDoc <extended> Read JsDoc annotations (choices: "none", "basic", "extended", default: "extended") | |||
--markdown-description Generate `markdownDescription` in addition to `description`. | |||
--raw-jsdoc Include the full raw JSDoc comment as `rawJsDoc` in the schema. |
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.
Should this be --raw-jsDoc
since we have --jsDoc
already?
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.
Honestly, I would replace --jsDoc
with --jsdoc
or --js-doc
, but that's up to you guys
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'd too if we started over but let's not break it now.
Or do we want to get rid of |
Some users might rely on markdown description, but that's probably because we don't yet provide raw JSdoc. As soon as we provide raw JSdoc, it's up to the user to parse the raw JSDoc into markdown however they want... |
So, is the plan to deprecate |
Up to @domoritz decide :) |
This adds support for a new CLI/config option
--raw-jsdoc
(rawJsDoc
), which includes arawJsDoc
field in the generated JSON Schema. This field preserves the full JSDoc comment block as is (after removing asterisks), without any formatting, tag filtering, inheritance, etc.--raw-jsdoc
/rawJsDoc
rawJsDoc
keyword to Ajv validatorExtendedAnnotationsReader
to extract raw JSDoc when availabletest/config/jsdoc-raw
Related to discussion in #1544 and complements
--markdown-description
(#1773)Example:
Typescript:
Output: