feat: add a11y-no-static-element-interactions compiler rule

[timmcca-be]

Ref: #820

This adds the no-static-element-interactions rule from eslint-plugin-jsx-a11y as a compiler rule. It is tripped if an element with no semantic meaning on the accessibility layer (e.g. div, a without href, header when it is not a direct descendant of body) does not have a role and has an interactive handler (e.g. on:click, on:mousedown).

This implementation follows the options from the recommended config by the ESLint plugin. That means:

• Dynamic role values are allowed
• We only check for these handlers:
  • on:click
  • on:mousedown
  • on:mouseup
  • on:keypress
  • on:keydown
  • on:keyup

In the ESLint plugin's default state, it disallows dynamic role values and checks for every focus, keyboard, and mouse handler. If we want to do this, we can remove the dynamic role check and copy the relevant handlers from index.d.ts.

Note that this PR also removes generic from non_interactive_roles in src/compiler/compile/utils/a11y.ts. It is also excluded from the associated array in eslint-plugin-jsx-a11y, and removing it made the new rule behave correctly, so I believe its inclusion was a bug. This did not affect any of the tests for the existing a11y rules, but I suspect it may still be a breaking change in some rare cases.

New breaking change here

• Who does this affect: Users who are not compliant with no-static-element-interactions from eslint-plugin-jsx-a11y, configured with the options from the plugin's recommended config
• How to migrate: Add an appropriate ARIA role to the static element with an interactive handler
• Why make this breaking change: Improves accessibility
• Severity (number of people affected x effort): Likely affects a moderate number of people, but low effort to fix

Before submitting the PR, please make sure you do the following

• ☑️ It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• ☑️ Prefix your PR title with [feat], [fix], [chore], or [docs].
• ☑️ This message body should clearly illustrate what problems it solves.
• ☑️ Ideally, include a test that fails without this PR but passes with it.

Tests

• ☑️ Run the tests with npm test and lint the project with npm run lint

[vercel]

@timmcca-be is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
svelte-dev-2	❌ Failed (Inspect)			Feb 22, 2023 at 0:44AM (UTC)

[dummdidumm]

Thank you for implementing this!
In contrast to some other rules this one might have quite an impact in the wild. I'm not sure if we should still have this part of v3 or in v4. @Conduitry thoughts?

[ngtr6788]

Hello there! I just took a quick look at your PR and it looks like both your PR and my PR deals with checking if an element is non-interactive and/or static.

Your PR has a new function is_non_interactive_element and implemented it similar to how eslint-jsx would do, while I united is_interactive_element and is_non_interactive_element into one function and is_interactive_role and is_non_interactive_role into one function. I can see that using your method is good because future contributors can just use eslint-jsx as their starting point, while my method can simplify the interactive/non-interactive logic, though you can disagree with me. Maintainers, which method do you think is better?

[edited because clarity]

[dummdidumm]

I feel like a mix of both would be best: Your internal enum paired with methods that wrap it into simple boolean methods (like is_interactive_element(..) { return element_interactivity(..) === .. } so that the code using them stays more readable.

[timmcca-be]

Looks like #8167 is going to release ahead of this, so once that is merged I'll update this PR

[benmccann]

@timmcca-be #8167 has been merged. Would you be able to rebase this PR? Thanks!

[timmcca-be]

@benmccann done!

[dummdidumm]

Thank you!

[wtachau]

Hi there, is there a way to disable this rule in our own projects? Accessibility is a noble goal but doesn't make sense in our project, though we still love using Svelte. Thanks!

[dummdidumm]

If you're using VS Code or some other IDE that uses the Svelte language server you can use this setting and if you're using svelte-check you can use the --compiler-warnings option

[wtachau]

Thank you for the response!

Here's my example code, and my attempt to follow your advice. Is there something I'm doing wrong?

[dummdidumm]

Ah, that error is coming from eslint - I think it's not silenceable there yet. Could you open an issue over at https://github.com/sveltejs/eslint-plugin-svelte asking for a feature request to silence specific Svelte compiler warnings?

[wtachau]

Here is a relevant issue, is this what you're looking for? sveltejs/eslint-plugin-svelte#311