Add @wordpress/use-recommended-components config options

[tbradsha]

What problem does this address?

As-is, @wordpress/use-recommended-components is all-or-nothing. It doesn't allow fine-grained control over what components are acceptable/not acceptable, which makes it hard to early-adopt components or gradually transition from old ones.

What is your proposed solution?

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

• disable warnings on specific components
• allow one to extend the ALLOWLIST so they can use experimental components as desired.
• allow one to extend the DENYLIST so they can incrementally tighten the set of components to replace over a transition period.

Originally posited here: Automattic/jetpack#48487 (comment)

Off-topic, but I'll also note that in Jetpack we use eslint-changed, which allows us to apply lints more selectively on legacy files (apply them to all files unless they're on an legacy excludelist, in which case it only applies the lint to changed lines). That might be worth looking into.

[aduth]

For the specific use-cases, I wonder why configurability would be preferred over other mechanisms like inline disabling or bulk suppressions? It feels like those examples are legitimate deviations from what the rule is intending to enforce, and at worst could mislead someone into thinking they're using a core-recommended component when they're not.

[ciampo]

also cc @mirka who implemented the original eslint rule

[mirka]

When I first designed the lint rule, I considered having the component list config live in the eslintrc, so it can be configured. I decided against it because the point of the lint rule was the component list itself, not the mechanism to recommend arbitrary components from an allow/deny list.

So at a certain point, if your repo has custom needs for its recommended component list, it will probably make more sense to write your own lint rule. This way you can add components from packages specific to your repo as well, rather than just making tweaks to the official component list.

Otherwise, like both of you suggest, ratcheting is probably the most straightforward way to deal with incremental transitions. This has been crucial to Gutenberg's adoption of the lint rule as well.

[tbradsha]

First, an additional config option to consider: allowing per-component or per-list warnings vs. errors. 😄

We could certainly write our own version of the plugin, too; I wanted to propose it in case it seemed useful, but can respect if that's not what's desired.

As for the other suggestions:

• ESLint bulk suppressions: This disables a given rule for an entire file. As such, there's nothing to prevent someone from adding additional violations of that rule (or swapping one violation for another). Unless one cleans up an entire file to remove it from suppressions, this isn't great for incremental cleanup.
• Inline disabling: Sometimes this is necessary, but it's not ideal to add hundreds of these throughout the codebase.

And another use case to consider, given the lists will certainly change as the package grows:

Imagine a large codebase that uses this rule and is lint-free, with CI checks to prevent anything bad from making it in. Yay! If a new recommended component or a new disallowed component is introduced, the codebase has four options they need to immediately decide between:

• bulk suppress a bunch of files (and risk new violations being introduced)
• inline disable a bunch of lines (adding lots of noise to the codebase)
• correct all the problems (which isn't practical for a large codebase)
• keep the old version of the rule (but then they can't use new components without linting errors)

This isn't great DevEx. Instead, it'd be good to have a solution that allows them to enforce the current "clean-to-this-point" status with previous components while working to clean things up before enabling complete compliance with the rule.

[aduth]

bulk suppress a bunch of files (and risk new violations being introduced)

Can you expand on what this risk is? There might be a misunderstanding, because the bulk suppression feature doesn't just disable the rule for an entire file, it asserts a specific number of expected failures for a file, so new violations (as well as fixes) in that file will be flagged as a failure. Syncing the suppressions is the expected workflow for what you describe as "clean-to-this-point" while avoiding the risk of further violations.

Conversely, my fear would be that if we make this configurable, it gives developers in a project a false sense of confidence that they're using "approved" components, when in fact they're not. And there's no mechanism to ensure accountability to rectifying that over time.

[tbradsha]

There might be a misunderstanding

Indeed, upon testing I can confirm I misunderstood a bit how it works. There's an edge case that wouldn't trigger anything, of course: if someone removes and adds a violation prior to running the lint. That's fairly narrow, though.

I still think it'll be a headache for anyone using the rules when the lists change, but that's on the consumer to decide whether to use the rule. Feel free to close this if it's unlikely to change!

[simison]

I'll close for now but feel free to re-open convo with new perspectives & points!