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

ESLint Plugin: Fix no-unused-vars-before-return JSX identifier reference #21358

Merged
merged 2 commits into from
Apr 17, 2020

Conversation

aduth
Copy link
Member

@aduth aduth commented Apr 2, 2020

Fixes #16418

This pull request seeks to resolve an issue with the custom ESLint rule @wordpress/no-unused-vars-before-return where an assigned variable which is referenced in a JSX identifier as tag name would wrongly report as being an error.

Previously, the following example would have been wrongly considered as an error:

function MyComponent() {
	const Foo = getSomeComponent();
	return <Foo />;
}

Implementation Notes:

As described in the inline code documentation and in the comments of #16418, the underlying issue is that we rely on variable references provided by ESLint, which at the present time does not include references of JSX identifiers. The approach here is to manually identify and merge those identifiers when considering references.

Testing Instructions:

Ensure unit tests pass:

npm run test-unit packages/eslint-plugin/rules/__tests__/no-unused-vars-before-return.js

Ensure lint passes:

npm run lint-js

@aduth aduth added [Type] Bug An existing feature does not function as intended [Tool] ESLint plugin /packages/eslint-plugin labels Apr 2, 2020
@github-actions
Copy link

github-actions bot commented Apr 2, 2020

Size Change: 0 B

Total Size: 842 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.01 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.24 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-editor/index.js 105 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.08 kB 0 B
build/block-library/editor.css 7.09 kB 0 B
build/block-library/index.js 112 kB 0 B
build/block-library/style-rtl.css 7.17 kB 0 B
build/block-library/style.css 7.17 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 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 57.7 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 16.9 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.25 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 3.54 kB 0 B
build/edit-navigation/style-rtl.css 485 B 0 B
build/edit-navigation/style.css 485 B 0 B
build/edit-post/index.js 27.9 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.5 kB 0 B
build/edit-site/style-rtl.css 5.05 kB 0 B
build/edit-site/style.css 5.05 kB 0 B
build/edit-widgets/index.js 7.49 kB 0 B
build/edit-widgets/style-rtl.css 4.67 kB 0 B
build/edit-widgets/style.css 4.67 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/index.js 43.3 kB 0 B
build/editor/style-rtl.css 3.48 kB 0 B
build/editor/style.css 3.47 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.32 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 622 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.51 kB 0 B
build/keycodes/index.js 1.91 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.28 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.67 kB 0 B
build/primitives/index.js 1.49 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.67 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.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@aduth
Copy link
Member Author

aduth commented Apr 10, 2020

19b7f26 demonstrates how these changes fix a preexisting issue. The ESLint rule had previously been disabled (in #21354) due to this bug.

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.

Nice fix 👍

For bug fixes touching non-production code we need to iterate quicker. You shouldn’t wait so long for review 🙁

packages/eslint-plugin/CHANGELOG.md Outdated Show resolved Hide resolved
@aduth aduth force-pushed the fix/16418-no-unused-vars-jsx branch from 19b7f26 to 6cf819a Compare April 17, 2020 20:04
@aduth aduth merged commit c14ae55 into master Apr 17, 2020
@aduth aduth deleted the fix/16418-no-unused-vars-jsx branch April 17, 2020 20:59
@github-actions github-actions bot added this to the Gutenberg 8.0 milestone Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] ESLint plugin /packages/eslint-plugin [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no-unused-vars-before-return does not detect use as React element
2 participants