Rule enhancement: allow regex in forbid-component-props allowedFor / disallowedFor

[bschlenk]

Is there an existing issue for this?

• I have searched the existing issues and my issue is unique
• My issue appears in the command-line and not only in the text editor

Description Overview

My team uses react/forbid-component-props to disallow className and style props, but we maintain a list of legacy components that took these props before we introduced this rule. We now also have a bunch of icon components in the codebase that all have names like Icon24Something. We like to pass style or className props to these to override css properties in some cases, so it'd be nice to be able to allow any component that matches /^Icon\d+/.

Does this sound like a change you'd be happy to accept? I'd be okay sending a PR. Otherwise I'm also happy to fork the rule internally for our use case, but figured I'd check here to contribute back to the community.

Expected Behavior

I'd expect to be able to pass a regex to allowedFor.

eslint-plugin-react version

v7.32.2

eslint version

v8.36.0

node version

v18.17.1

[bschlenk]

What I'd add is (in addition to allowedFor and disallowedFor) allowedForRegex and disallowedForRegex options that take an optional string. Then you can check in both the arrays and test the regex against the string. This would be a backwards compatible change.

[ljharb]

Allowing regex strings in eslint config is an extremely bad idea (it's a CVE farm), but allowing globs would be tenable.

[bschlenk]

What do you mean by CVE farm?

Looks like this package already depends on minimatch, I can rework my approach.

[bschlenk]

I think I get it. It isn't a real issue (the user of this package owns their config so why would they ddos themself with a bad regex) but you'd rather not deal with automated security reports against your package. If that's the case, seems unfortunate but understandable.

[bschlenk]

Alright, unless you think globs would be a useful addition here, I'm going to close this one out. Globs aren't really meant for generic string matching IMO, more for paths. I'm fine with modifying this rule in our repo or figuring out a different api for our icons that doesn't involve className or style.

[ljharb]

If globs can help you solve your use case, I think it's useful, but I agree they're more for paths than strings.

[figmatom]

Thoughts on supporting StartsWith and EndsWith instead? Gives you the 95% case of regexes.

[ljharb]

You can do that with globs - *end or start*

[figmatom]

Yeah, they will solve most things, but globs are just such a weird API, since folks associate globs pretty strongly with file systems, and this applies to the names. I would make the assumption taht it was a glob on the import path, not the filename, and do a triple take on the docs 😅 But that's definitely only one antecdote. That's why I was suggesting a startswith/endswith or just an includes.

[dominicfraser]

@bschlenk We'd also love to be able to allow any component prop based on a prefix, for very similar reasons.

Keeping an interested eye on the discussion here.

[ljharb]

We already use that pattern in a few places, as does eslint itself, so I think it'll be fine.

If you can adapt #3687 to use a glob-based approach, I'd be happy to review it.

[akulsr0]

This should be solved with #3774

[ljharb]

Fixed in #3774; duplicate of #3686.

[Efimenko]

@akulsr0, maybe I didn't get it, but how does propNamePattern solve the initially described case with allowedFor/dissalowedFor?