Change href-or-on-click ESLint rule to require href with onClick

[TamerlanG]

Summary

We have cases where we would want both href and onClick, for example cmd-click behavior, collecting telemetry within the onClick, etc...

Changes:

• Changed the href-or-on-click ESLint rule to warn when onClick is present without href.
• Previously the rule warned when both href and onClick were supplied; now it encourages always including href alongside onClick so that Cmd+Click / "Open in new tab" works
• Updated tests to match the new behavior

[Copilot]

Pull request overview

Updates the href-or-on-click custom ESLint rule in packages/eslint-plugin to encourage link-like EUI components to include an href when an onClick handler is provided, improving expected “open in new tab” behavior.

Changes:

• Invert rule behavior to report when onClick is present without href (instead of reporting when both are present).
• Reclassify the rule as a suggestion and update the rule’s documentation/message text accordingly.
• Update and expand RuleTester cases to match the new expected valid/invalid patterns.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
packages/eslint-plugin/src/rules/href_or_on_click.ts	Changes the rule logic to flag onClick without href and updates metadata/message text.
packages/eslint-plugin/src/rules/href_or_on_click.test.ts	Updates valid/invalid test cases to reflect the new rule behavior.

Comments suppressed due to low confidence (1)

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

• Given the rule change, consider adding a test case that includes a JSX spread attribute (e.g. <EuiButton {...props} onClick={handleClick} />). If the rule is updated to bail out on spreads (to avoid false positives), this should be a valid case; if not, the test should assert the intended behavior explicitly.

    {
      code: dedent`
        module.export = () => (
          <EuiButtonEmpty href="/" onClick={handleClick} />
        )
      `,
      languageOptions,
    },
    {
      code: dedent`
        module.export = () => (
          <EuiLink href="/home" onClick={handleClick} />
        )
      `,
      languageOptions,
    },
    {
      code: dedent`
        module.export = () => (
          <SomeOtherComponent onClick={handleClick} />
        )
      `,
      languageOptions,
    },

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[weronikaolejniczak]

@TamerlanG for releasable packages like @elastic/eslint-plugin-eui we need a changelog. You can see the exemplary entries in our CHANGELOG_{YEAR}.md files (without the PR link, that is added automatically). So you'd need to add one in packages/eslint-plugin/changelogs/upcoming. The filename should be 9615.md.

[weronikaolejniczak]

We should also update README.md accordingly 😄

[TamerlanG]

Wait, I just noticed that this affects the buttons and badge too. Let's have this be just for links? I can create a separate rule for that?

wdyt @weronikaolejniczak and @tkajtoch

[weronikaolejniczak]

@TamerlanG ah, so for EuiLink (link components) we want both onClick AND href but for EuiButton and EuiBadge that's not necessarily the case? Then yes, I would separate it out into a different rule that's specific for links. We could try to share as much code as possible with this one.

[TamerlanG]

I still have to add the changelog and update the readme, so lemme do that :)

[TamerlanG]

added updates to readme and change log here: 20768c6

[weronikaolejniczak]

I added a couple of nits. The only blocking thing is the changelog entry 😄

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: ff0b3d2
• Documentation website
• Storybook

History

• 💚 Build #3371 succeeded b3c2f24
• 💚 Build #3344 succeeded 5dc6e08
• 💚 Build #3338 succeeded f013d19
• 💚 Build #3335 succeeded 2e175c2

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: ff0b3d2

History

• 💚 Build #6358 succeeded b3c2f24
• 💚 Build #6339 succeeded 5dc6e08

[weronikaolejniczak]

I'll approve but as discussed on Slack, we'll merge it only after the next release