Enable ESLint JSDoc checks #2913
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,6 +11,47 @@ module.exports = { | |||||||
| 'src/govuk/vendor/polyfills/**/*' | ||||||||
| ], | ||||||||
| overrides: [ | ||||||||
| { | ||||||||
| extends: 'plugin:jsdoc/recommended', | ||||||||
| files: ['**/*.{cjs,js,mjs}'], | ||||||||
|
There was a problem hiding this comment. question: Will that not catch all JavaScript files? Or is that necessary because we need to create an override section. suggestion: We may want to exclude test files (in case we need file-specific helper functions sometimes)
Suggested change
There was a problem hiding this comment. Yeah I'm happy with JSDoc checking all JavaScript files 😮 |
||||||||
| plugins: [ | ||||||||
| 'jsdoc' | ||||||||
| ], | ||||||||
| rules: { | ||||||||
| // JSDoc blocks are optional | ||||||||
| 'jsdoc/require-jsdoc': 'off', | ||||||||
| 'jsdoc/require-param-description': 'off', | ||||||||
| 'jsdoc/require-param': 'off', | ||||||||
|
There was a problem hiding this comment. Presumably by disabling this rule we're making it possible to accidentally omit params from otherwise valid JSDoc blocks? Is there any way to lint that if a JSDoc block exists then all params should be covered by that JSDoc? There was a problem hiding this comment. @36degrees Yeah if we solely have ESLint is clever enough to make /**
* Umbrella scripts tasks (for watch)
* Runs JavaScript code quality checks and compilation
*/
gulp.task('scripts', gulp.series(
npmScriptTask('lint:js', ['--silent']),
'js:compile'
))☝️ We quite frequently use JSDoc style comments for scripts and tests We have a few other options on the table:
|
||||||||
|
|
||||||||
| // Require hyphens before param description | ||||||||
| // Aligns with TSDoc style: https://tsdoc.org/pages/tags/param/ | ||||||||
| 'jsdoc/require-hyphen-before-param-description': 'warn', | ||||||||
|
There was a problem hiding this comment. We sometimes hyphenate, sometimes don't, so I've added this rule It's recommended by JSDoc (and mandatory for TSDoc)
|
||||||||
|
|
||||||||
| // Check for valid formatting | ||||||||
| 'jsdoc/check-line-alignment': 'warn', | ||||||||
| 'jsdoc/check-syntax': 'error', | ||||||||
|
|
||||||||
| // Add unknown @jest-environment tag name | ||||||||
| 'jsdoc/check-tag-names': [ | ||||||||
| 'warn', { | ||||||||
| definedTags: ['jest-environment'] | ||||||||
| } | ||||||||
| ], | ||||||||
|
|
||||||||
| // Add missing .querySelectorAll() type | ||||||||
| 'jsdoc/no-undefined-types': [ | ||||||||
| 'error', { | ||||||||
| definedTypes: ['NodeListOf'] | ||||||||
|
There was a problem hiding this comment. question: Does that mean that a syntax like There was a problem hiding this comment. @romaricpascal Yeah exactly Without this change then One of the JSDoc plugin dependencies must not be aware of it as a known DOM type (possibly because the TypeScript flavour is In Visual Studio Code if you mouse over
|
||||||||
| } | ||||||||
| ] | ||||||||
| }, | ||||||||
| settings: { | ||||||||
| jsdoc: { | ||||||||
| // Allows us to use type declarations that exist in our dependencies | ||||||||
| mode: 'typescript' | ||||||||
|
There was a problem hiding this comment. Why Rather than spend time documenting every complex object (or instance) we can use the declarations already published by the package author: /**
* @param {import('puppeteer').Page} page
* @param {import('puppeteer').ElementHandle} element
* @param {import('puppeteer').WaitForOptions} options
*/TypeScript declarations are widely available unlike local-only
36degrees marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| } | ||||||||
| } | ||||||||
| }, | ||||||||
| { | ||||||||
| files: ['**/*.test.{cjs,js,mjs}'], | ||||||||
| env: { | ||||||||
|
|
||||||||
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.
question: Why do we need to re-extend the 'plugin:jsdoc/recommended'? It's already declared in the root
extendsoption.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 spotted! I thought I'd moved it, as it felt better in an override versus the root
Here I've made sure I'm only loading the JSDoc plugin for
'**/*.{cjs,js,mjs}'Gives us more flexibility when adding new plugins for other files. For example, if we added
*.tsfiles in future their own override section would add the TSDoc plugineslint-plugin-tsdocinstead