ESLint Plugin/Scripts: Update ESLint and related deps to 7.1.0 #22771

ocean90 · 2020-05-30T16:37:34Z

Description

Fixes #22385.
Fixes #20235.

This is a first pass for updating @wordpress/eslint-plugin and @wordpress/scripts to ESLint 7.x.

What I have done:

# Remove eslint from packages/scripts/package.json
npm i
npx lerna add eslint packages/scripts
# Remove eslint-plugin-react-hooks from packages/eslint-plugin/package.json
npm i
npx lerna add eslint-plugin-react-hooks packages/eslint-plugin
# Remove eslint-plugin-jsdoc from packages/eslint-plugin/package.json
npm i
npx lerna add eslint-plugin-jsdoc packages/eslint-plugin
# Remove eslint-plugin-react from packages/eslint-plugin/package.json
npm i
npx lerna add eslint-plugin-react packages/eslint-plugin

Changelogs:

eslint-plugin-react-hooks: https://github.com/facebook/react/blob/master/packages/eslint-plugin-react-hooks/CHANGELOG.md
eslint-plugin-jsdoc: https://github.com/gajus/eslint-plugin-jsdoc/releases (Commits)
eslint-plugin-react: https://github.com/yannickcr/eslint-plugin-react/blob/master/CHANGELOG.md

There two remaining deps which haven't got an update yet:

~~During the commit I got a package.lock failure:~~

Error Details

> node ./bin/validate-package-lock.js "/Users/Dominik/Development/WordPress/wordpress-core/src/wp-content/plugins/gutenberg/package-lock.json"

Invalid resolved dependency in package-lock.json.

{
	"node-pre-gyp": {
		"version": "0.12.0",
		"resolved": false,
		"integrity": "sha512-4KghwV8vH5k+g2ylT+sLTjy5wmUOb9vPhnM8NHvRf9dHmnW/CndrFXy2aRPaPST6dugXSdHXfeaHQm77PIz/1A==",
		"dev": true,
		"optional": true,
		"requires": {
			"detect-libc": "^1.0.2",
			"mkdirp": "^0.5.1",
			"needle": "^2.2.1",
			"nopt": "^4.0.1",
			"npm-packlist": "^1.1.6",
			"npmlog": "^4.0.2",
			"rc": "^1.2.7",
			"rimraf": "^2.6.1",
			"semver": "^5.3.0",
			"tar": "^4"
		},
		"dependencies": {
			"rimraf": {
				"version": "2.7.1",
				"resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.7.1.tgz",
				"integrity": "sha512-uWjbaKIK3T1OSVptzX7Nl6PvQ3qAGtKEtVRjRuazjfL3Bx5eI409VZSqgND+4UNnmzLVdPj9FqFJNPqBZFve4w==",
				"dev": true,
				"optional": true,
				"requires": {
					"glob": "^7.1.3"
				}
			}
		}
	}
}

~~I tried removing node_modules but that didn't solve it. What else can I try? I'm using npm 6.14.5.~~ Manually fixed in 33b287b

How has this been tested?

wp-scripts lint-js which produced a lot of Missing JSDoc and Missing @param errors (see on Travis). Probably due to changes in eslint-plugin-jsdoc v25.0.0 https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v25.0.0.

Types of changes

Breaking change

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.
I've updated all React Native files affected by any refactorings/renamings in this PR.

github-actions · 2020-05-30T16:43:56Z

Size Change: 0 B

Total Size: 1.12 MB

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	0 B
`build/annotations/index.js`	3.62 kB	0 B
`build/api-fetch/index.js`	3.4 kB	0 B
`build/autop/index.js`	2.83 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	6.75 kB	0 B
`build/block-directory/style-rtl.css`	892 B	0 B
`build/block-directory/style.css`	892 B	0 B
`build/block-editor/index.js`	106 kB	0 B
`build/block-editor/style-rtl.css`	11.4 kB	0 B
`build/block-editor/style.css`	11.4 kB	0 B
`build/block-library/editor-rtl.css`	7.87 kB	0 B
`build/block-library/editor.css`	7.88 kB	0 B
`build/block-library/index.js`	126 kB	0 B
`build/block-library/style-rtl.css`	7.69 kB	0 B
`build/block-library/style.css`	7.68 kB	0 B
`build/block-library/theme-rtl.css`	684 B	0 B
`build/block-library/theme.css`	686 B	0 B
`build/block-serialization-default-parser/index.js`	1.88 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/blocks/index.js`	48.2 kB	0 B
`build/components/index.js`	193 kB	0 B
`build/components/style-rtl.css`	19.5 kB	0 B
`build/components/style.css`	19.5 kB	0 B
`build/compose/index.js`	9.33 kB	0 B
`build/core-data/index.js`	11.4 kB	0 B
`build/data-controls/index.js`	1.29 kB	0 B
`build/data/index.js`	8.46 kB	0 B
`build/date/index.js`	5.47 kB	0 B
`build/deprecated/index.js`	771 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.11 kB	0 B
`build/edit-navigation/index.js`	8.25 kB	0 B
`build/edit-navigation/style-rtl.css`	878 B	0 B
`build/edit-navigation/style.css`	876 B	0 B
`build/edit-post/index.js`	302 kB	0 B
`build/edit-post/style-rtl.css`	5.43 kB	0 B
`build/edit-post/style.css`	5.43 kB	0 B
`build/edit-site/index.js`	15 kB	0 B
`build/edit-site/style-rtl.css`	2.96 kB	0 B
`build/edit-site/style.css`	2.96 kB	0 B
`build/edit-widgets/index.js`	8.83 kB	0 B
`build/edit-widgets/style-rtl.css`	2.4 kB	0 B
`build/edit-widgets/style.css`	2.4 kB	0 B
`build/editor/editor-styles-rtl.css`	425 B	0 B
`build/editor/editor-styles.css`	428 B	0 B
`build/editor/index.js`	44.7 kB	0 B
`build/editor/style-rtl.css`	4.26 kB	0 B
`build/editor/style.css`	4.27 kB	0 B
`build/element/index.js`	4.65 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.72 kB	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/hooks/index.js`	2.13 kB	0 B
`build/html-entities/index.js`	621 B	0 B
`build/i18n/index.js`	3.56 kB	0 B
`build/is-shallow-equal/index.js`	711 B	0 B
`build/keyboard-shortcuts/index.js`	2.52 kB	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/index.js`	3.12 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	226 B	0 B
`build/list-reusable-blocks/style.css`	226 B	0 B
`build/media-utils/index.js`	5.3 kB	0 B
`build/notices/index.js`	1.79 kB	0 B
`build/nux/index.js`	3.4 kB	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/plugins/index.js`	2.56 kB	0 B
`build/primitives/index.js`	1.5 kB	0 B
`build/priority-queue/index.js`	789 B	0 B
`build/redux-routine/index.js`	2.85 kB	0 B
`build/rich-text/index.js`	14.8 kB	0 B
`build/server-side-render/index.js`	2.68 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.28 kB	0 B
`build/url/index.js`	4.06 kB	0 B
`build/viewport/index.js`	1.85 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.17 kB	0 B

_{compressed-size-action}

ocean90 · 2020-05-30T17:26:05Z

The JSDoc lint errors seem to be correct and should probably get fixed in a separate PR before the ESLint update gets merged?

Some examples:

[1] /home/travis/build/WordPress/gutenberg/packages/annotations/src/store/actions.js
[1]    6:1  error  Missing JSDoc @param "annotation.selector" declaration  jsdoc/require-param
[1]    6:1  error  Missing JSDoc @param "annotation.source" declaration    jsdoc/require-param
[1]    6:1  error  Missing JSDoc @param "annotation.id" declaration        jsdoc/require-param
[1]   16:0  error  Missing @param "annotation.selector"                    jsdoc/check-param-names
[1]   16:0  error  Missing @param "annotation.source"                      jsdoc/check-param-names
[1]   16:0  error  Missing @param "annotation.id"                          jsdoc/check-param-names

gutenberg/packages/annotations/src/store/actions.js

Lines 22 to 24 in 78587c9

    
            * @param {string} annotation.[selector="range"] The way to apply this annotation. 
        
            * @param {string} annotation.[source="default"] The source that added the annotation. 
        
            * @param {string} annotation.[id]               The ID the annotation should have. Generates a UUID by default.

Syntax for optional props changed?

[1] /home/travis/build/WordPress/gutenberg/packages/annotations/src/store/actions.js
[1]    6:1  error  Missing JSDoc @param "annotation.selector" declaration  jsdoc/require-param
[1]    6:1  error  Missing JSDoc @param "annotation.source" declaration    jsdoc/require-param
[1]    6:1  error  Missing JSDoc @param "annotation.id" declaration        jsdoc/require-param
[1]   16:0  error  Missing @param "annotation.selector"                    jsdoc/check-param-names
[1]   16:0  error  Missing @param "annotation.source"                      jsdoc/check-param-names
[1]   16:0  error  Missing @param "annotation.id"                          jsdoc/check-param-names

gutenberg/packages/block-directory/src/store/actions.js

Lines 53 to 60 in 78587c9

    
           /** 
        
            * Action triggered to install a block plugin. 
        
            * 
        
            * @param {Object} item The block item returned by search. 
        
            * 
        
            * @return {boolean} Whether the block was successfully installed & loaded. 
        
            */ 
        
           export function* installBlockType( { id, name, assets } ) {

The documentation is missing.

[1] /home/travis/build/WordPress/gutenberg/packages/block-editor/src/autocompleters/block.js
[1]   84:1  error  Missing JSDoc @param "root0" declaration                                  jsdoc/require-param
[1]   84:1  error  Missing JSDoc @param "root0.getBlockInsertionParentClientId" declaration  jsdoc/require-param
[1]   84:1  error  Missing JSDoc @param "root0.getInserterItems" declaration                 jsdoc/require-param
[1]   84:1  error  Missing JSDoc @param "root0.getSelectedBlockName" declaration             jsdoc/require-param

gutenberg/packages/block-editor/src/autocompleters/block.js

Lines 84 to 94 in 78587c9

    
           /** 
        
            * Creates a blocks repeater for replacing the current block with a selected block type. 
        
            * 
        
            * @return {WPCompleter} A blocks completer. 
        
            */ 
        
           export function createBlockCompleter( { 
        
           	// Allow store-based selectors to be overridden for unit test. 
        
           	getBlockInsertionParentClientId = defaultGetBlockInsertionParentClientId, 
        
           	getInserterItems = defaultGetInserterItems, 
        
           	getSelectedBlockName = defaultGetSelectedBlockName, 
        
           } = {} ) {

The documentation is missing.

gziolo · 2020-06-01T08:57:34Z

The JSDoc lint errors seem to be correct and should probably get fixed in a separate PR before the ESLint update gets merged?

It looks like there are 2 rules that are more strict now based on failures from:
https://travis-ci.com/github/WordPress/gutenberg/jobs/341716759

Those rules are

jsdoc/require-param
jsdoc/check-param-name

I agree that it makes sense to address them separately. On the technical side of things, it might be simpler to downgrade them to warnings and bring back to errors in two follow up PRs for each rule separately.

I see also that some unit tests are failing:
https://travis-ci.com/github/WordPress/gutenberg/jobs/341716764

This is very surprising but it looks like those are valid issues since the impact ESLint related logic:

packages/eslint-plugin/rules/__tests__/gutenberg-phase.js
packages/eslint-plugin/rules/__tests__/no-unused-vars-before-return.js

There two remaining deps which haven't got an update yet:

New release supporting ESLint 7 import-js/eslint-plugin-import#1772

Support ESLint 7.x jsx-eslint/eslint-plugin-jsx-a11y#683

Maybe, they don't need to be updated. I think it's quite common that those plugins release at their own pace.

packages/eslint-plugin/CHANGELOG.md

ocean90 · 2020-06-01T11:03:06Z

@gziolo Thanks for the feedback!

Nice catch with the failing unit tests, looks like I focused to match on the lint errors. Fixed in 338639a.

I agree that it makes sense to address them separately. On the technical side of things, it might be simpler to downgrade them to warnings and bring back to errors in two follow up PRs for each rule separately.

I'm not sure, might be easier to just fix them in one in one sweep. The error report might look severe but the jsdoc/require-param/jsdoc/check-param-name are just duplicated for the same params. I'll give this a try.

Maybe, they don't need to be updated.

Yes, I think we can release without them for now, at least there seem to be no issues. It's just about the npm warning:

npm WARN eslint-plugin-jsx-a11y@6.2.3 requires a peer of eslint@^3 || ^4 || ^5 || ^6 but none is installed. You must install peer dependencies yourself.

ocean90 · 2020-06-01T12:33:04Z

@gziolo Okay, after fixing some issues I think we should go with your downgrade to warning idea here (5757cc9) which should make it easier to fix the errors in follow-up PRs. Switching branches isn't so efficient and only relying on the current Travis output isn't good either because there may be follow-up errors like shown in #22794.

Marking this PR as ready for review and will continue with a tracking issue for the JSDoc update if you all agree with the proposed workflow.

gziolo · 2020-06-02T08:25:23Z

There is one more option for how we can approach it. Now that this branch passes, we can open another branch targeting this branch that fixes individual rules (or both together if they are tightly coupled). Otherwise, we would have to update @wordpress/eslint-plugin package rather than .eslintrc.js which hides the issue for Gutenberg only but not for 3rd party projects.

ocean90 · 2020-06-02T09:28:08Z

Otherwise, we would have to update @wordpress/eslint-plugin package rather than .eslintrc.js which hides the issue for Gutenberg only but not for 3rd party projects.

It hides them only temporarily for the project until we have fixed the issues. 3rd party projects should follow the same procedure or fix them directly without the warning step since those are valid errors. So I don't think we should change the severity in the published package.

See eslint/eslint@78c8cda.

…rrors to warnings

ocean90 · 2020-06-04T08:20:22Z

Related chat on Slack: https://wordpress.slack.com/archives/C5UNMSU4R/p1591107916367900

The decision was to merge this PR as is and then I'll follow-up with a tracking ticket for the new JSDoc warnings.

gziolo

It works as expected, there are new warnings introduced:

✖ 215 problems (0 errors, 215 warnings)
  0 errors and 129 warnings potentially fixable with the `--fix` option.

Let's move forward according to the plan discussed earlier this week during the Core JS meeting 🚢

gziolo · 2020-06-04T16:16:02Z

@ocean90, one interesting thing in the hint from ESLint. I executed npm run lint-js -- --fix and it adds all those missing params but they miss types and description. Still, it seems like a good way to speed up process :)

ocean90 added [Package] Scripts /packages/scripts [Package] ESLint plugin /packages/eslint-plugin labels May 30, 2020

ocean90 mentioned this pull request May 31, 2020

Scripts: Upgrade ESLint to the new major version (7.x) #22385

Closed

gziolo requested review from aduth, sirreal and talldan and removed request for sirreal and aduth June 1, 2020 10:54

gziolo assigned aduth and unassigned aduth Jun 1, 2020

gziolo requested a review from aduth June 1, 2020 10:57

gziolo added the [Type] Breaking Change For PRs that introduce a change that will break existing functionality label Jun 1, 2020

gziolo reviewed Jun 1, 2020

View reviewed changes

packages/eslint-plugin/CHANGELOG.md Show resolved Hide resolved

ocean90 mentioned this pull request Jun 1, 2020

Fix JSDoc for __experimentalAddAnnotation() #22794

Merged

6 tasks

ocean90 marked this pull request as ready for review June 1, 2020 12:33

ocean90 requested review from ajitbohra, nerrad and ntwb as code owners June 1, 2020 12:33

ocean90 self-assigned this Jun 1, 2020

aduth mentioned this pull request Jun 1, 2020

Dependency Extraction Webpack Plugin: Add types #22498

Merged

6 tasks

ocean90 mentioned this pull request Jun 2, 2020

Support ESLint 7.x wearerequired/coding-standards#89

Closed

ocean90 force-pushed the update/eslint-7 branch from 5757cc9 to c6d2ba3 Compare June 2, 2020 07:09

ESLint Plugin/Scripts: Update ESLint and related deps to 7.1.0

bcc90b3

ocean90 added 4 commits June 4, 2020 10:17

Restore resolved fields in lockfile

5bbeb2b

Replace removed start/end properties with range[0]/range[1].

af5d8cf

See eslint/eslint@78c8cda.

Move eslint-plugin-react update to enhancements section

ff246fe

Temporarily convert jsdoc/check-param-names and jsdoc/require-param e…

2c9a69f

…rrors to warnings

ocean90 force-pushed the update/eslint-7 branch from c6d2ba3 to 2c9a69f Compare June 4, 2020 08:18

gziolo approved these changes Jun 4, 2020

View reviewed changes

gziolo merged commit 2320c04 into master Jun 4, 2020

gziolo deleted the update/eslint-7 branch June 4, 2020 16:10

github-actions bot added this to the Gutenberg 8.3 milestone Jun 4, 2020

This was referenced Jun 4, 2020

JSDoc: Add missing params to fix new ESLint errors #22907

Closed

Scripts: Remove temporary workaround in ESLint config #22915

Merged

ocean90 mentioned this pull request Jun 16, 2020

ESLint plugin: Allow ESLint 7.x as peer dependency #23190

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESLint Plugin/Scripts: Update ESLint and related deps to 7.1.0 #22771

ESLint Plugin/Scripts: Update ESLint and related deps to 7.1.0 #22771

ocean90 commented May 30, 2020 •

edited

github-actions bot commented May 30, 2020 •

edited

ocean90 commented May 30, 2020

gziolo commented Jun 1, 2020

ocean90 commented Jun 1, 2020 •

edited

ocean90 commented Jun 1, 2020

gziolo commented Jun 2, 2020

ocean90 commented Jun 2, 2020

ocean90 commented Jun 4, 2020

gziolo left a comment

gziolo commented Jun 4, 2020

ESLint Plugin/Scripts: Update ESLint and related deps to 7.1.0 #22771

ESLint Plugin/Scripts: Update ESLint and related deps to 7.1.0 #22771

Conversation

ocean90 commented May 30, 2020 • edited

Description

How has this been tested?

Types of changes

Checklist:

github-actions bot commented May 30, 2020 • edited

ocean90 commented May 30, 2020

gziolo commented Jun 1, 2020

ocean90 commented Jun 1, 2020 • edited

ocean90 commented Jun 1, 2020

gziolo commented Jun 2, 2020

ocean90 commented Jun 2, 2020

ocean90 commented Jun 4, 2020

gziolo left a comment

Choose a reason for hiding this comment

gziolo commented Jun 4, 2020

ocean90 commented May 30, 2020 •

edited

github-actions bot commented May 30, 2020 •

edited

ocean90 commented Jun 1, 2020 •

edited