Skip to content

Commit 3665907

Browse files
Change href-or-on-click ESLint rule to require href with onClick (elastic#9615)
Co-authored-by: Weronika Olejniczak <32842468+weronikaolejniczak@users.noreply.github.com>
1 parent 49cb983 commit 3665907

7 files changed

Lines changed: 162 additions & 14 deletions

File tree

packages/eslint-plugin/README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,14 @@ This package contains an eslint plugin that enforces some default rules for usin
1313

1414
`<EuiButton />` should either be a button or a link, for a11y purposes. When given an `href` the button behaves as a link, otherwise an `onClick` handler is expected and it will behave as a button.
1515

16+
### `@elastic/eui/href-or-on-click`
17+
18+
`<EuiButton />` should either be a button or a link, for a11y purposes. When given an `href` the button behaves as a link, otherwise an `onClick` handler is expected and it will behave as a button.
19+
1620
In some cases it makes sense to disable this rule locally, such as when <kbd>cmd</kbd> + click should open the link in a new tab, but a standard click should use the `history.pushState()` API to change the URL without triggering a full page load.
1721

22+
**Exception**: `EuiLink` has to be provided with both `onClick` and `href` so that it renders as an anchor and support Ctrl/Cmd+Click to open in a new tab, and other standard link interactions. See `@elastic/eui/require-href-for-link`.
23+
1824
### `@elastic/eui/no-restricted-eui-imports`
1925

2026
At times, we deprecate features that may need more highlighting and/or that are not possible to annotate with JSDoc `@deprecated`, e.g. JSON token imports: `@elastic/eui/dist/eui_theme_*.json` (for context: https://github.com/elastic/kibana/issues/199715#json-tokens).
@@ -175,6 +181,10 @@ Ensure the `EuiBadge` includes appropriate accessibility attributes.
175181
- `iconOnClickAriaLabel` is only valid when `iconOnClick` is present. The rule autofixes by removing `iconOnClickAriaLabel`.
176182
- `onClickAriaLabel` is only valid when `onClick` is present. The rule autofixes by removing `onClickAriaLabel`.
177183

184+
### `@elastic/eui/require-href-for-link`
185+
186+
Ensure `EuiLink` components that have an `onClick` handler also include an `href` prop. Without `href`, the component does not render as a true link, which means users cannot Ctrl/Cmd+Click to open in a new tab or use other standard link interactions. The rule bails out when spread attributes are present, since `href` may be provided via the spread.
187+
178188
### `@elastic/eui/icon-accessibility-rules`
179189

180190
Ensure the `EuiIcon` includes appropriate accessibility attributes.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Added new `require-href-for-link` rule

packages/eslint-plugin/src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { AccessibleInteractiveElements } from './rules/a11y/accessible_interacti
1010
import { CallOutAnnounceOnMount } from './rules/a11y/callout_announce_on_mount';
1111
import { ConsistentIsInvalidProps } from './rules/a11y/consistent_is_invalid_props';
1212
import { HrefOnClick } from './rules/href_or_on_click';
13+
import { RequireHrefForLink } from './rules/require_href_for_link';
1314
import { NoCssColor } from './rules/no_css_color';
1415
import { NoRestrictedEuiImports } from './rules/no_restricted_eui_imports';
1516
import { NoStaticZIndex } from './rules/no_static_z_index';
@@ -44,6 +45,7 @@ const config = {
4445
PreferTooltipTriggerFocusTestUtility,
4546
'badge-accessibility-rules': EuiBadgeAccessibilityRules,
4647
'icon-accessibility-rules': EuiIconAccessibilityRules,
48+
'require-href-for-link': RequireHrefForLink,
4749
},
4850
configs: {
4951
recommended: {
@@ -66,6 +68,7 @@ const config = {
6668
'@elastic/eui/prefer-tooltip-trigger-focus-test-utility': 'warn',
6769
'@elastic/eui/badge-accessibility-rules': 'warn',
6870
'@elastic/eui/icon-accessibility-rules': 'warn',
71+
'@elastic/eui/require-href-for-link': 'warn',
6972
},
7073
},
7174
},

packages/eslint-plugin/src/rules/href_or_on_click.test.ts

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,5 @@ ruleTester.run('href-or-on-click', HrefOnClick, {
9696
},
9797
],
9898
},
99-
{
100-
code: dedent`
101-
module.export = () => (
102-
<EuiLink href="/" onClick={fooBar} />
103-
)
104-
`,
105-
languageOptions,
106-
errors: [
107-
{
108-
messageId: 'hrefOrOnClick',
109-
},
110-
],
111-
},
11299
],
113100
});

packages/eslint-plugin/src/rules/href_or_on_click.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import { TSESTree, ESLintUtils } from '@typescript-eslint/utils';
1010

11-
const componentNames = ['EuiButton', 'EuiButtonEmpty', 'EuiLink', 'EuiBadge'];
11+
const componentNames = ['EuiButton', 'EuiButtonEmpty', 'EuiBadge'];
1212

1313
export const HrefOnClick = ESLintUtils.RuleCreator.withoutDocs({
1414
create(context) {
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import dedent from 'dedent';
10+
import { RuleTester } from '@typescript-eslint/rule-tester';
11+
12+
import { RequireHrefForLink } from './require_href_for_link';
13+
14+
const languageOptions = {
15+
parserOptions: {
16+
ecmaFeatures: {
17+
jsx: true,
18+
},
19+
},
20+
};
21+
22+
const ruleTester = new RuleTester();
23+
24+
ruleTester.run('require-href-for-link', RequireHrefForLink, {
25+
valid: [
26+
{
27+
code: dedent`
28+
module.export = () => (
29+
<EuiLink />
30+
)
31+
`,
32+
languageOptions,
33+
},
34+
{
35+
code: dedent`
36+
module.export = () => (
37+
<EuiLink href="/" />
38+
)
39+
`,
40+
languageOptions,
41+
},
42+
{
43+
code: dedent`
44+
module.export = () => (
45+
<EuiLink href="/" onClick={handleClick} />
46+
)
47+
`,
48+
languageOptions,
49+
},
50+
{
51+
code: dedent`
52+
module.export = () => (
53+
<EuiLink {...linkProps} onClick={handleClick} />
54+
)
55+
`,
56+
languageOptions,
57+
},
58+
{
59+
code: dedent`
60+
module.export = () => (
61+
<EuiButton onClick={handleClick} />
62+
)
63+
`,
64+
languageOptions,
65+
},
66+
],
67+
68+
invalid: [
69+
{
70+
code: dedent`
71+
module.export = () => (
72+
<EuiLink onClick={handleClick} />
73+
)
74+
`,
75+
languageOptions,
76+
errors: [
77+
{
78+
messageId: 'requireHrefForLink',
79+
},
80+
],
81+
},
82+
],
83+
});
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import { TSESTree, ESLintUtils } from '@typescript-eslint/utils';
10+
11+
import { hasSpread } from '../utils/has_spread';
12+
13+
export const RequireHrefForLink = ESLintUtils.RuleCreator.withoutDocs({
14+
create(context) {
15+
return {
16+
JSXOpeningElement(node: TSESTree.JSXOpeningElement): void {
17+
if (
18+
node.name.type !== 'JSXIdentifier' ||
19+
node.name.name !== 'EuiLink'
20+
) {
21+
return;
22+
}
23+
24+
// Bail out if props are spread — we can't statically determine
25+
// whether `href` is provided via the spread
26+
if (hasSpread(node.attributes)) {
27+
return;
28+
}
29+
30+
// Check if the node has `href` and `onClick` attributes
31+
const hasHref = node.attributes.some(
32+
(attr) => attr.type === 'JSXAttribute' && attr.name.name === 'href'
33+
);
34+
const hasOnClick = node.attributes.some(
35+
(attr) => attr.type === 'JSXAttribute' && attr.name.name === 'onClick'
36+
);
37+
38+
// Report an issue if `onClick` is present without `href`
39+
if (hasOnClick && !hasHref) {
40+
context.report({
41+
node,
42+
messageId: 'requireHrefForLink',
43+
data: {
44+
name: node.name.name,
45+
},
46+
});
47+
}
48+
},
49+
};
50+
},
51+
meta: {
52+
type: 'suggestion',
53+
docs: {
54+
description:
55+
'Recommend including `href` alongside `onClick` on EuiLink so that Ctrl/Cmd+Click (open link in new tab) works.',
56+
},
57+
schema: [],
58+
messages: {
59+
requireHrefForLink:
60+
'<{{name}}> has `onClick` but no `href`. Consider adding `href` so that the component renders as a link and supports Ctrl/Cmd+Click / "Open link in new tab".',
61+
},
62+
},
63+
defaultOptions: [],
64+
});

0 commit comments

Comments
 (0)