Skip to content

Option to disable a11y plugin? #2032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
justinhelmer opened this issue May 3, 2019 · 22 comments
Closed

Option to disable a11y plugin? #2032

justinhelmer opened this issue May 3, 2019 · 22 comments

Comments

@justinhelmer
Copy link

justinhelmer commented May 3, 2019

We love the airbnb style guide, including the react stuff. But we don't want any of the a11y rules.

I saw this, which makes me think it would be relatively easy to only include a11y through some configuration:

module.exports = {
  extends: [
    'eslint-config-airbnb-base', // want this
    'eslint-config-airbnb-base/rules/strict', // want this
    './rules/react', // want this
    './rules/react-a11y', // don't want this
  ].map(require.resolve),
  rules: {}
};

The other option would be to include each item independently, but it would require the react rules to be published separately:

module.exports = {
  extends: [
    'airbnb-base',
    'airbnb-base/rules/strict',
    'airbnb-react' // doesn't actually exist
  ]
}

When we try to just access airbnb-react via the eslint-config-airbnb package, it still requires eslint-plugin-jsx-a11y due to the way modules are resolved:

module.exports = {
  extends: [
    'airbnb-base',
    'airbnb-base/rules/strict',
    'airbnb/rules/react'
  ]
}

Throws an error

ESLint couldn't find the plugin "eslint-plugin-jsx-a11y". This can happen for a couple different reasons...

Our workaround is to install eslint-plugin-jsx-a11y even though it's not used, and include each item independently like above. This works, but forces us to include the extra dependency that we don't want or need.

Any suggestions?

@ljharb
Copy link
Collaborator

ljharb commented May 3, 2019

Why don't you want any of the a11y rules?

@justinhelmer
Copy link
Author

@ljharb Accessibility is not a requirement for our internal customer base and we are constrained by time to deliver our product. For most projects a11y is a good thing but sometimes it adds overhead if the rules are not necessary for your audience.

@ljharb
Copy link
Collaborator

ljharb commented May 4, 2019

As a workaround, you could manually disable all the rules from the plugin - i think it’d be more reasonable to include an “off” preset in the a11y plugin itself than for airbnb’s guide to make it easy to avoid a11y.

Separately, you may want to check local laws; employees typically have the same protections as customers, which means that a11y is typically always an unconditional requirement for everyone.

@justinhelmer
Copy link
Author

Manually disabling the rules is a bit difficult to track and maintain - and still forces us to require the dependency, even though we don't want it at all. If you believe the best option is to manually track all the rules and disable them, then I suppose this issue can be marked as resolved.

Also, thanks for calling out the legal implications; I'll be sure to pass this along and do the necessary research and make sure we aren't skipping out on what would otherwise be necessary by law.

Either way, perhaps you see value in keeping this issue open to resolution. For example building something that is not consumer-facing nor for a set of employees (i.e. projects that have nothing to do with making money or supporting employers).

As always thanks for the quick help @ljharb you are a beast.

@ljharb
Copy link
Collaborator

ljharb commented May 4, 2019

Sorry, to clarify: as a maintainer of the a11y plugin, I’m suggesting a change there so that you don’t have to manually manage all the rule names - you’d extend a11y/disable or something.

@justinhelmer
Copy link
Author

justinhelmer commented May 4, 2019

Ah, yeah that would be much nicer. Even nicer if we would be able to skip including eslint-plugin-jsx-a11y in the list of dependencies all-together (in conjunction with the ability to disable), but I'll leave that to better people than myself to decide if it's worth the effort to do so.

@ljharb
Copy link
Collaborator

ljharb commented May 4, 2019

I def see how that’d be easier for you, but that’d require us to maintain a third package that included react and base; and then the slippery slope of one that’s just base and a11y, etc :-/

Want to file a PR to the a11y plugin that adds that disable preset?

@justinhelmer
Copy link
Author

justinhelmer commented May 4, 2019

@ljharb - My first attempt. Let me know if I missed anything or you need me to add anything. Comments in-line with diff. Tested locally; works like a charm.

@beefancohen
Copy link

just saw the PR in eslint-plugin-jsx-a11y, but I'm commenting here to keep the conversation in a single thread. i'm not sure this is something I'd want to put into the plugin, as it makes very little sense to provide an easy escape for users to avoid accessibility. the whole point of the plugin is to make it easier to build accessible apps. i have some sympathy for your time constraints, but accessibility isn't just something that's tacked on for use with assistive technologies; it's something that benefits all users. have you ever tabbed through a page before or turned on captions on a video? if you truly want to disable all of the rules (although this seems developer-hostile), I'd prefer you manually disabling the rules in your eslint config. Yes, you'll have an added devDependency that's not being used, but that's a much lower cost than providing an escape hatch for developers to ignore accessibility patterns in their code.

i'm happy to be convinced otherwise if you think I'm missing something! thanks for contributing and starting a discussion @justinhelmer 😄

@justinhelmer
Copy link
Author

@evcohen - It's a fair point. Why make it easy for devs to bypass what is typically always a good thing to have, and even often mandatory by law? Hard to argue with that logic.

Easy enough to dust off the PR if the community can provide overwhelming evidence that it's a good idea. Until that time, I am content having this issue closed out after @ljharb gets a chance to triage it (since he has invested his time as well).

Appreciate the consideration and looking to make the right decision here.

Cheers.

@ljharb
Copy link
Collaborator

ljharb commented May 4, 2019

I think that at this point the issue here should indeed be closed; as for the PR, if @evcohen isn't on board, then it's probably best to decline.

@justinhelmer thanks for the attempt! one thing you could do, if you have an JS-based eslint config, is programmatically require('eslint-plugin-jsx-a11y').rules and disable each of them, without worrying about hardcoding things.

@ljharb ljharb closed this as completed May 4, 2019
@justinhelmer
Copy link
Author

Thanks for the quick turnaround; have a great weekend @ljharb and @evcohen

@Bessonov
Copy link

For everyone who wondering how to use mentioned workaround:

const a11yOff = Object.keys(require('eslint-plugin-jsx-a11y').rules)
	.reduce((acc, rule) => { acc[`jsx-a11y/${rule}`] = 'off'; return acc }, {})

module.exports = {
	rules: {
		...a11yOff,
		// your rules
	},
}

@kaminskypavel
Copy link

@Bessonov thanks for posting this.
here are the rules being auto disabled (taken from the snippet above)

[
  'accessible-emoji',
  'alt-text',
  'anchor-has-content',
  'anchor-is-valid',
  'aria-activedescendant-has-tabindex',
  'aria-props',
  'aria-proptypes',
  'aria-role',
  'aria-unsupported-elements',
  'autocomplete-valid',
  'click-events-have-key-events',
  'control-has-associated-label',
  'heading-has-content',
  'html-has-lang',
  'iframe-has-title',
  'img-redundant-alt',
  'interactive-supports-focus',
  'label-has-associated-control',
  'label-has-for',
  'lang',
  'media-has-caption',
  'mouse-events-have-key-events',
  'no-access-key',
  'no-autofocus',
  'no-distracting-elements',
  'no-interactive-element-to-noninteractive-role',
  'no-noninteractive-element-interactions',
  'no-noninteractive-element-to-interactive-role',
  'no-noninteractive-tabindex',
  'no-onchange',
  'no-redundant-roles',
  'no-static-element-interactions',
  'role-has-required-aria-props',
  'role-supports-aria-props',
  'scope',
  'tabindex-no-positive'
]

@Arcanorum

This comment has been minimized.

@ljharb

This comment has been minimized.

@gkatsanos
Copy link

gkatsanos commented Dec 17, 2021

I guess using OSS implies there may be updates, deprecations, API changes or pretty much anything that the consumer / user of the OSS has to comply with. It is also of course legitimate to raise concerns whenever such decisions may create problems, issues and provide feedback to the maintainers.
In our case, we use a lot of click handlers that do nothing more than fire tracking events. I just updated the Vuejs eslint airbnb config library and (unsurprisingly) got hundreds of errors on missing keyboard handlers. This would of course involve fixing these errors or ignoring them, which in any case would involve time & money investment.
I guess we can choose to revert the library update or fork it and create our own ruleset. But I felt I had to report this here in case this decision might be reconsidered.
In any case most setups move towards code formatters / Prettier so I guess overloading the rulesets isn't particularly productive in some scenarios.
Thumbs up for the good work so far!

@ljharb
Copy link
Collaborator

ljharb commented Dec 17, 2021

@gkatsanos you can also just override the one rule, on top of the Airbnb config, if you find it undesirable, without investing any time or money.

@gkatsanos
Copy link

@gkatsanos you can also just override the one rule, on top of the Airbnb config, if you find it undesirable, without investing any time or money.

Unfortunately it's not the only rule that was added, the accessibility package comes with a strong set of opinionated rules that require plenty of fixes.

In my opinion it's a nice package and we would love to included it but it should have been optional, or at least there should have been a transition period.

@ljharb
Copy link
Collaborator

ljharb commented Dec 17, 2021

@gkatsanos this config has included the a11y plugin for half a decade. Every rule that can be has been applied that entire time. We recently had a major release which enabled more rules - that's just how semver works, no transition period is needed. You're welcome to stay on eslint 7 and the previous release if that's your preference.

As for vuejs, the airbnb config assumes and requires you're using react - if you're using anything else, you're voiding the warranty, as it were.

@gkatsanos
Copy link

gkatsanos commented Dec 18, 2021

I will take all these recommendations into account! thanks @yyx990803

@stevehanson
Copy link

A use-case for disabling the JSX-A11y rules that I consider valid is when using this lint config on React Native projects. The JSX-a11y rules are designed for the DOM and are not relevant for React Native. I have successfully used the approach shared above to disable all JSX-a11y rules on a couple React Native projects.

I think accessibility is important, and I feel like if Airbnb provided a way to easily opt out of the JSX-a11y plugin, then many projects would do that to the detriment of the web. For that reason, I'm fine with doing the workaround when needed for React Native projects. Or maybe there is a third option, like a separate config specifically for React Native.

eslint-plugin-react-native-a11y is a good alternative to JSX-a11y for React Native projects.

If anyone knows of a better approach for using the Airbnb config with React Native, I'd love to hear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants