Eslint: adopt @wordpress/use-recommended-components

[simison]

Likely best done after #48405 and #48404 land (which updates UI and Eslint libraries)

Proposed changes

Enables @wordpress/use-recommended-components as an error-level ESLint rule monorepo-wide (in tools/js-tools/eslintrc/base.mjs). This encourages migration away from deprecated/experimental @wordpress/components exports toward stable @wordpress/ui components:

@wordpress/ui — allowlist only

Only these imports are allowed. Anything else from @wordpress/ui is discouraged (ESLint error):

Allowed
Badge
Card	(namespace: Card.Root, etc.)
Collapsible
CollapsibleCard
EmptyState
Link
Stack
Text
VisuallyHidden

We use a few more components in Jetpack as our own judgment call (Button, Popover, Icon, Notice). Usages are now listed in the suppressions.json list in this PR, but I might wait with merging to have those stable (they're close).

@wordpress/components — denylist only

Imports from @wordpress/components are fine by default, except these discouraged names (each points to a @wordpress/ui alternative):

Discouraged import	Use instead
ExternalLink	Link from @wordpress/ui with openInNewTab
__experimentalHeading	Text
__experimentalHStack	Stack
__experimentalText	Text
__experimentalVStack	Stack
__experimentalZStack	Write your own CSS (planned deprecation)
Card	Card.Root
CardBody	Card.Content
CardDivider	(no replacement — pattern dropped)
CardFooter	(no replacement)
CardHeader	Card.Header / Card.Title
CardMedia	Card.FullBleed
VisuallyHidden	VisuallyHidden from @wordpress/ui

All other @wordpress/components imports (Button, Modal, Popover, Panel, …) are not flagged by this rule yet, but future updates will include those, too.

Related product discussion/links

Does this pull request change what data or activity we track or use?

No.

Testing instructions

Verify the rule is active:

1. Open any JS/TS file that imports ExternalLink from @wordpress/components — ESLint should report an error: "Use Link from @wordpress/ui with the openInNewTab prop instead."
2. Open any file listed in tools/eslint/suppressions.json (e.g. projects/js-packages/components/components/admin-page/index.tsx) — the existing violation should be silently suppressed and pnpm run lint-file <file> should exit 0.

pnpm run lint

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
3. You can use GitHub's Reviewers functionality to request a review.
4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

[jp-launch-control]

Code Coverage Summary

This PR did not change code coverage!

That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷

Full summary · PHP report · JS report

[anomiex]

Likely best done after #48405 and #48404 land (which updates UI and Eslint libraries)

Agreed. That would save some of the noise in pnpm-lock.yaml in this PR.

[tbradsha]

I haven't done a deep code review, but any reason we're not just cleaning these up directly? Looks like ~250 instances in the suppressions file, and I suspect many of them are the same or similar instances. Could one throw AI at it?

I'd at least recommend trying a cleanup of violations/merge into trunk before adding this.

Also, if I'm skimming correctly, it seems that once a file has suppressed errors of the given type, it won't prevent additional such errors from being added to a file.

My fear is that we'll end up with an additional layer of complexity that only serves to suppress issues we haven't properly fixed.

[simison]

any reason we're not just cleaning these up directly? Looks like ~250 instances in the suppressions file, and I suspect many of them are the same or similar instances. Could one throw AI at it?

That's not trivial as these aren't mere code-formatting lints; code change would be simple'ish, but each component swap usually needs some design decisions and, of course, lots of testing that things continue working, which is a lot of work.

There are four folks replacing old components right now with @wordpress/ui ones. I've observed is a bunch of new PRs adding code with old components, and I've been going around commenting to use the new ones instead. This lint will hopefully guide both humans and agents to use the right ones without oversight.

Note that Gutenberg is using an identical strategy to change their components.

[tbradsha]

I've observed is a bunch of new PRs adding code with old components, and I've been going around commenting to use the new ones instead. This lint will hopefully guide both humans and agents to use the right ones without oversight.

We definitely appreciate the cleanup/modernization efforts! I totally get this, having dealt with Phan cleanup (which uses baselines) and Stylelint (no baselines but rule-by-rule cleanup). We have the same goal, so let's see if we can align on implementation. :^)

Some thoughts:

• @wordpress/use-recommended-components is generally okay. It's good to have rules that prod people to use the proper things.
• The rule currently doesn't have any config options (e.g. it will always throw warnings about experimental components, and those will require suppressions).
• Baselines can easily become a crutch and hide real problems. Along with the added complexity, if a file has one violation, there's nothing preventing someone from adding an additional one, which defeats the goal (people can continue to use bad rules).
• If we instead use eslint-changed, it would prevent the introduction of new violations (checking only changed lines) while ignoring old ones during the cleanup process.
• It'd also be good if the ESLint plugin was modified with some config overrides:
  • disable warnings on specific components
  • allow one to extend the ALLOWLIST so we can use experimental components as desired.
  • allow one to extend the DENYLIST so we can incrementally tighten the set of components we need to replace during a transition period.
  • anything else?

[tbradsha]

As an aside, it seems <Link openInNewTab /> is largely a drop-in replacement for <ExternalLink />, and that makes up over half of the current violations. If it helps, I put up a draft PR that addresses those: #48529

If it's noise/interfering with your current work, feel free to close it. 😄 I also notice you put up a similar PR targeting just Forms here: #48407

[simison]

It'd also be good if the ESLint plugin was modified with some config overrides:

FYI @mirka @ciampo @aduth, some good feedback here from a monorepo maintainer about the component swapover ESLint helper. :-)

[ciampo]

Feel free to open an issue in Gutenberg to discuss / agree on improvements to @wordpress/use-recommended-components, we're happy to collaborate and gather feedback from usage outside of Gutenberg 🙏

[tbradsha]

Feel free to open an issue in Gutenberg to discuss / agree on improvements

Added here: WordPress/gutenberg#78062

[simison]

@anomiex I've updated the PR to use the existing custom tools/eslint-excludelist.json.

FYI, current failures in the lint job are known and described in the PR description. I'm still working on those aspects.

Can you clarify a couple of developer experience nuances just so that I understand correctly how the custom Jetpack Eslint workflow works, vs the regular Eslint functionality:

• If folks modify files on the exclusion list and changes are unrelated to components, what happens? Will they need to fix the whole file if the old violations are now on new line numbers?
  • (That would be unfortunate and not exactly the goal of the component lint; it's fine to keep using the old components, we just want to avoid using them in new work going forward.)
• If I understand right, new lint violations on these exclusion files will continue getting flagged?
• Once we update upstream Eslint configs (will likely be each version bump now with work actively going to both @wordpress/components and @wordpress/ui); the files on the excluded files might become clean and existing files "dirty"; the current setup won't notice the changes on excluded list but it does notice changed lint requirements on other files?

[anomiex]

• If folks modify files on the exclusion list and changes are unrelated to components, what happens? Will they need to fix the whole file if the old violations are now on new line numbers?

eslint-changed analyzes the diff between the old and new versions to account for changed line numbers.

• If I understand right, new lint violations on these exclusion files will continue getting flagged?

If the new lint violations are due to changes in the file itself, yes.

If the new lint violations are due to changes in other files, no. For one thing, we only run eslint-changed on excluded files that are being changed in the PR. For another, eslint-changed lints the old and new versions of each file individually, but does not attempt to account for anything else in the repo changing (e.g. different versions of eslint plugins, changes to eslint config, or changes to other files).

• Once we update upstream Eslint configs (will likely be each version bump now with work actively going to both @wordpress/components and @wordpress/ui); the files on the excluded files might become clean and existing files "dirty"; the current setup won't notice the changes on excluded list but it does notice changed lint requirements on other files?

If an excluded file becomes clean, the "Check linter exclude lists" job will flag that tools/eslint-excludelist.json needs updating.

If a non-excluded file becomes "dirty", it'll fail the "ESLint (non-excluded files only)" check in CI. Whoever is working on the update PR will have to choose between fixing it or adding it to the exclude list.

If an excluded file becomes cleaner (without becoming completely clean) or dirtier, CI will not report it.