Eslint-plugin-jsx-a11y: label-has-for should not fail on children content

Created on 6 Jun 2017  路  6Comments  路  Source: jsx-eslint/eslint-plugin-jsx-a11y

As mentioned in #250, the following pattern fails in label-has-for.

<label>
   {children}
</label>

This pattern should be allowed. We cannot know what {children} will render, so we can't raise a failure with certainty.

bug enhancement help wanted

Most helpful comment

@evcohen Great! I'll look to incorporate it later this afternoon/evening. Thanks for the direction.

All 6 comments

Hi Jesse,

To be clear, we want to test for:

<label>
   {children}
</label>

Not:

<label>
   {children>
</label>

Correct?

I've been looking at the label-has-for code and tests and have altered the following in src/label-has-for.js:

const validateNesting = node => !!node.parent.children.find(child => child.type === 'JSXElement');

To:

const validateNesting = node => !!node.parent.children.find(child => child.type === 'JSXElement' || child.type === 'JSXExpressionContainer');

This passes the following tests, in addition to all the other label-has-for tests:

{ code: '<label>{children}</label>', options: optionsRequiredSome },
{ code: '<label>{children}</label>', options: optionsRequiredNesting },

I fear checking just for a JSXExpressionContainer type is too permissive, though, since it is possible to pass in any interpolated value and it wouldn't be flagged. Thoughts?

I'd like this under an option; I'd prefer the linter to forbid this pattern, as the only place in my codebase I want either an input or a label to appear is in the same render method.

I fear checking just for a JSXExpressionContainer type is too permissive, though, since it is possible to pass in any interpolated value and it wouldn't be flagged. Thoughts?

I'd like this under an option; I'd prefer the linter to forbid this pattern, as the only place in my codebase I want either an input or a label to appear is in the same render method.

There's the thought on that :) I'd personally let it pass and test at a higher level of abstraction. @ljharb cracks a finer whip than me =D

Fixed the OP description. Solution looks good. Roll it up into a PR please!

@mjaltamirano there is a util for hasAccessibleChildren that we could reuse 馃憤

@evcohen Great! I'll look to incorporate it later this afternoon/evening. Thanks for the direction.

Was this page helpful?
0 / 5 - 0 ratings