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.

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(eslint-plugin): misc bugfixes related to recent jsdoc changes #16975

Merged
merged 3 commits into from
Aug 12, 2019

Conversation

dsifford
Copy link
Contributor

@dsifford dsifford commented Aug 8, 2019

Description

This PR does the following:

  • addresses an issue brought up by @gziolo related to how es5 configurations were being skipped over with jsdoc being at the top-level. Add eslint-plugin-jsdoc for better JSDoc linting #16869 (comment)
  • adds in a few helpful TypeScript-defined helper types that are nice to have available for JSDoc in some cases.
  • adds in the missing void type since it appears that's defined in globals and we're filtering it out.
  • fixes a few misc scattered JSDoc issues that are indeed errors and easily fixable.

How has this been tested?

Lints still run and pass. However, there are quite a bit more warnings that are currently showing up. I don't think these warnings are a big deal for now. They'll get handled when I get around to fixing up that package. The warnings are all valid.

Types of changes

Code quality / Bugfix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I did a quick check of npm run lint-js result on Travis: https://travis-ci.com/WordPress/gutenberg/jobs/223779531

It looks a bit better warning wise. It seems like we can fix everything else by adding type definitions or replacing types which don't exist with some alternatives.

@@ -1,4 +1,7 @@
module.exports = {
extends: [
Copy link
Member

Choose a reason for hiding this comment

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

Cool, thanks for making closer to what we had before 💯

@gziolo gziolo added [Package] ESLint plugin /packages/eslint-plugin [Type] Enhancement A suggestion for improvement. labels Aug 9, 2019
*
* @see http://www.typescriptlang.org/docs/handbook/utility-types.html
*/
const typescriptUtilityTypes = [
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, this comes from the official TypeScript npm package but I guess it wouldn't be as easy to extract. I did a quick google search and I didn't find anything that would fit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, unfortunately they aren't able to be extracted in this way because they're defined as types and/or interfaces. It would be much cleaner though if that was possible, I agree.

@gziolo gziolo added the [Type] Developer Documentation Documentation for developers label Aug 9, 2019
@gziolo gziolo merged commit 87e2c15 into master Aug 12, 2019
@gziolo gziolo deleted the fix/eslint-jsdoc-misc-fixes branch August 12, 2019 10:53
gziolo pushed a commit that referenced this pull request Aug 29, 2019
…6975)

* fix(eslint-plugin): misc assorted fixes to jsdoc config

* fix: misc jsdoc fixes

* remove duplicate Readonly
gziolo pushed a commit that referenced this pull request Aug 29, 2019
…6975)

* fix(eslint-plugin): misc assorted fixes to jsdoc config

* fix: misc jsdoc fixes

* remove duplicate Readonly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] ESLint plugin /packages/eslint-plugin [Type] Developer Documentation Documentation for developers [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants