Eslint-plugin-jsx-a11y: no-static-element-interactions should not fail on complex values for role

[jessebeach]

The following should pass because we can't determine in lint what the value is, so we can't make a definitive judgement call.

{ code: '<div onClick={() => {}} role={isBool ? "presentation" : "button"} />;' }

• [ ] Introduce a dedicated rule -- no-dynamic-roles -- that errors on non-string literal roles.
• [x] Loosen the stringency of no-static-element-interactions to bail out on dynamic role values.

[ljharb]

I'd argue that an option should be used to control that; in particular, I'd prefer that to always fail - I don't want dynamic role values to appear in my codebase.

[jessebeach]

I'd argue that an option should be used to control that; in particular, I'd prefer that to always fail - I don't want dynamic role values to appear in my codebase.

Noted. I can see the arguments on both sides. I'll take your point into consideration while coding.

[evcohen]

I think that discouraging dynamic roles is fine (and better, I agree), but it is separate from using the role prop value for enforcing a rule. We should be focused on providing the best tool for developing accessible apps more so than enforcing prop value styles.

Using only literal values for role makes sense to me, as I think that would be the most accurate and actionable warning we can provide

[jessebeach]

We should be focused on providing the best tool for developing accessible apps more so than enforcing prop value styles.

@evcohen I'm confused where you fall on this issue. Could you elaborate a little more please?

The case that caused me to open this issue is one with a ternary expression as a value for role that does provide the correct behavior, but raises a lint warning. It's a case that we can't determine via lint; it's a false positive. My intent with this issue is to provide a steam valve for the behavior for teams that don't want such a strict level of evaluation. I do agree that providing a strict interpretation (and making that the default) is preferable.

[ljharb]

In this specific case - a ternary where both paths are a string literal - the rule could certainly allow it statically.

In a general case, I'm totally fine with an option that configures ignore vs fail behavior on non-string-literals (where a template with no substitutions is considered a string literal)

[evcohen]

Sorry, let me clarify. I agree with @ljharb, but I don't think it's this plugin's responsibility to enforce prop value style. I think we should be focusing on how we can provide the most accurate and actionable feedback for accessibility, and in this case would be to only evaluate string literal role prop values. So I agree with the original proposal for this ticket.

You can enforce dynamic prop values as a bad pattern with a different lint rule.

[ljharb]

Is it not actionable to dictate a prop style that can be assured to be accessible?

[ljharb]

More specifically, I don't want all or even most dynamic props warned on - only the ones where their dynamism precludes ensuring its accessible. I'm not sure how a generic rule would know that, since that knowledge belongs to individual a11y rules.

[evcohen]

In that case, I'd argue that it'd be better to have a rule dedicated to that (no-dynamic-roles) and allow every other rule to only error on what it's designed to enforce as best as it can. Plenty of rules here check role prop and it doesn't scale to include that option for all of them, IMO

[ljharb]

That's fair; could we add that before loosening this (and the other) rules? That way I'm not blocked on upgrading.

[jessebeach]

In that case, I'd argue that it'd be better to have a rule dedicated to that (no-dynamic-roles) and allow every other rule to only error on what it's designed to enforce as best as it can. Plenty of rules here check role prop and it doesn't scale to include that option for all of them, IMO

That's fair; could we add that before loosening this (and the other) rules? That way I'm not blocked on upgrading.

I like the compromise. +2

[julianaleon]

Is there any update on this issue?

[jessebeach]

@julianaleon none yet. Would you like to take a shot at the no-dynamic-roles rule that @evcohen suggests in a comment above? I'm happy to advise and review the PR.

[jessebeach]

This is a dupe of #245. We'll handle the issue there.