eslint-plugin-jsx-a11y: `label-has-for` false positive when using `allowChildren`

When using this rule:

"jsx-a11y/label-has-for": [2, {
  "required": {"every": ["nesting"]},
  "allowChildren": true
}]

This code passes the linter check:

<label>Firstname</label>
<input type="text" name="firstname" />

Currently it passes due to case 'Literal': return Boolean(child.value); in hasAccessibleChild I think.

Or am I missing something?

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 19 (10 by maintainers)

Most upvoted comments

Hey y’all, sorry for the inconvenience. I’ll check this first thing tomorrow morning. I suspect it is what @amannn pointed out, in hasAccessibleChild. When I initially wrote #259 I didn’t use it and checked specifically for a JSXExpressionContainer. That change might fix it, and I’ll add additional test cases to check for that, too.

michaeljaltamirano on Jul 20, 2017

provided that the allowChildren option could be disabled, whatever that means for the new rule’s configuration

@ljharb understood, I’ll make it happen and add the test cases.

jessebeach on Jan 3, 2018

@mjaltamirano agreed- that’s my bad for pointing you in this direction. we should be explicitly checking for a child element that’s not Literal. sorry about that 😞 and thanks for fixing!

beefancohen on Jul 20, 2017

I agree with point number 1 😃 the choice of all 4 options (perhaps with better names: [id, nesting, id-and-nesting, id-or-nesting]?) would suffice for me (provided that the allowChildren option could be disabled, whatever that means for the new rule’s configuration)

ljharb on Jan 2, 2018

the current some/every mechanics allow for future extensibility if a third thing is added.

@ljharb Aha, I see what you mean. I’d like to push back a little:

A “third-thing” would require an update to how HTML works – either a new attribute or method of associating a label to an input. An event like that would warrant a large breaking change to a rule like this. I may not be aware of some mechanism that you have in mind.
I’d like to eliminate the two independently varying options that affect each other. every means nothing unless both id and nesting are also specified. The interdependence is confusing.
[“id”, “nesting”, “both”, or “either”] seem like a complete set to achieve the behavior to satisfy the use case you describe.
I’d like to have one rule for input labels, that’s why I’m pushing so hard on this. Two rules with slightly varied behavior will be confusing for end users of the plugin. I think the feedback here is helping me construct a better rule, but it still needs some tweaking. If you agree with point number 1 above, then I think we can move forward. I don’t disagree with your intent, just the mechanics of the config to achieve it with simple user-facing options.

jessebeach on Jan 2, 2018

Yep, that’s correct.

amannn on Jul 19, 2017