-
Notifications
You must be signed in to change notification settings - Fork 337
chore(repo): Add eslint-plugin-jsdoc #5697
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?
chore(repo): Add eslint-plugin-jsdoc #5697
Conversation
🦋 Changeset detectedLatest commit: a3f014a The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Self-review
@@ -67,7 +67,9 @@ exports[`Typedoc output > should have a deliberate file structure 1`] = ` | |||
"types/user-organization-invitation-resource.mdx", | |||
"types/user-resource.mdx", | |||
"types/without.mdx", | |||
"shared/api-url-from-publishable-key.mdx", |
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.
Whenever we add JSDoc comments to previously undocumented functions we'll need to update this snapshot as Typedoc will only generate MDX files for documented, exported functions.
You can update the snapshot by running pnpm test:typedoc -u
}, | ||
rules: { | ||
...pluginJsDoc.configs['flat/recommended-typescript'].rules, | ||
'jsdoc/check-examples': 'off', |
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 don't want it to raise false positives in the examples, we want more control over it
rules: { | ||
...pluginJsDoc.configs['flat/recommended-typescript'].rules, | ||
'jsdoc/check-examples': 'off', | ||
'jsdoc/informative-docs': 'warn', |
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.
This ensures that this doesn't happen:
/** The user id. */
let userId;
You have to write a longer description, not restate the name
...pluginJsDoc.configs['flat/recommended-typescript'].rules, | ||
'jsdoc/check-examples': 'off', | ||
'jsdoc/informative-docs': 'warn', | ||
'jsdoc/check-tag-names': ['warn', { definedTags: ['inline', 'unionReturnHeadings'], typed: false }], |
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.
This will warn when an unknown/invalid tag name is used
'jsdoc/check-tag-names': ['warn', { definedTags: ['inline', 'unionReturnHeadings'], typed: false }], | ||
'jsdoc/require-hyphen-before-param-description': 'warn', | ||
'jsdoc/require-description': 'warn', | ||
'jsdoc/require-description-complete-sentence': 'warn', |
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.
Sentences end with fullstop and start with capital letter
'jsdoc/require-description-complete-sentence': 'warn', | ||
'jsdoc/require-param': ['warn', { ignoreWhenAllParamsMissing: true }], | ||
'jsdoc/require-param-description': 'warn', | ||
'jsdoc/require-returns': 'off', |
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.
Together with require-param
I turned these off (unless you define at least one @param
) since we use TypeScript types in most places. It doesn't hurt to add them but I don't think it's necessary in most places
'jsdoc/require-param': ['warn', { ignoreWhenAllParamsMissing: true }], | ||
'jsdoc/require-param-description': 'warn', | ||
'jsdoc/require-returns': 'off', | ||
'jsdoc/tag-lines': [ |
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.
This ensures empty lines between tags/description, unless for multiple @param
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 fixed this file as an exercise and check how the rules will apply
Description
This PR adds https://github.com/gajus/eslint-plugin-jsdoc to our ESLint configuration and initially enables it for
packages/shared
. It will report warnings, not errors. I want to see how y'all feel using it while working on PRs and see if the configuration choices I made make sense.I plan on enabling it for more of our monorepo packages in the future. We then can also think about turning the warnings into errors.
Check the inline review comments for more details.
Fixes ECO-242
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change