Skip to content

Commit 789d368

Browse files
michaelfaithmichaelfaith
committed
feat(label-has-associated-control): add option for enforcing label's htmlFor matches control's id
This change adds an option to the `label-has-associated-control` rule, enforcing that the label's htmlFor attribute matches the associated control's id attribute. Previously, the only validation done on htmlFor was that it was on the label component and had text. There was no attempt to cross-check that value against any attribute on the associated control. Not, when the option is enabled, cases where they don't match will report. I also took the opportunity to update the error messages so that each assert type gets an error message with verbiage specific to the assertion. (not sure if this should be called out as a separate feature in the changelog?). Note: the current implementation only checks the first instance it finds of child component that matches each control component type. It assumes that there won't be any acceptable cases where a label would have multiple inputs nested beneath it. Let me know if that assumption doesn't hold. Closes:
1 parent bfb0e9e commit 789d368

11 files changed

+686
-63
lines changed

‎__mocks__/JSXFragmentMock.js

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* @flow
3+
*/
4+
5+
export type JSXFragmentMockType = {
6+
type: 'JSXFragment',
7+
children: Array<Node>,
8+
};
9+
10+
export default function JSXFragmentMock(
11+
children?: Array<Node> = [],
12+
): JSXFragmentMockType {
13+
return {
14+
type: 'JSXFragment',
15+
children,
16+
};
17+
}

‎__tests__/src/rules/label-has-associated-control-test.js

+33
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const errorMessages = {
2727
nesting: 'A form label must have an associated control as a descendant.',
2828
either: 'A form label must either have a valid htmlFor attribute or a control as a descendant.',
2929
both: 'A form label must have a valid htmlFor attribute and a control as a descendant.',
30+
htmlForShouldMatchId: 'A form label must have a htmlFor attribute that matches the id of the associated control.',
3031
};
3132
const expectedErrors = {};
3233
Object.keys(errorMessages).forEach((key) => {
@@ -58,6 +59,7 @@ const htmlForValid = [
5859
{ code: '<label htmlFor="js_id" aria-label="A label" />' },
5960
{ code: '<label htmlFor="js_id" aria-labelledby="A label" />' },
6061
{ code: '<div><label htmlFor="js_id">A label</label><input id="js_id" /></div>' },
62+
{ code: '<div><label htmlFor={inputId}>A label</label><input id={inputId} /></div>' },
6163
{ code: '<label for="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], settings: attributesSettings },
6264
{ code: '<label for="js_id" aria-label="A label" />', settings: attributesSettings },
6365
{ code: '<label for="js_id" aria-labelledby="A label" />', settings: attributesSettings },
@@ -128,6 +130,12 @@ const alwaysValid = [
128130
{ code: '<input type="hidden" />' },
129131
];
130132

133+
const shouldHtmlForMatchIdValid = [
134+
{ code: '<label htmlFor="js_id" aria-label="A label"><span><span><input id="js_id" /></span></span></label>', options: [{ depth: 4, shouldHtmlForMatchId: true }] },
135+
{ code: '<div><label htmlFor="js_id">A label</label><input id="js_id" /></div>', options: [{ shouldHtmlForMatchId: true }] },
136+
{ code: '<div><label htmlFor={inputId} aria-label="A label" /><input id={inputId} /></div>', options: [{ shouldHtmlForMatchId: true }] },
137+
];
138+
131139
const htmlForInvalid = (assertType) => {
132140
const expectedError = expectedErrors[assertType];
133141
return [
@@ -164,6 +172,13 @@ const nestingInvalid = (assertType) => {
164172
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedError] },
165173
];
166174
};
175+
const shouldHtmlForMatchIdInvalid = [
176+
{ code: '<label htmlFor="js_id" aria-label="A label"><span><span><input /></span></span></label>', options: [{ depth: 4, shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
177+
{ code: '<label htmlFor="js_id" aria-label="A label"><span><span><input name="js_id" /></span></span></label>', options: [{ depth: 4, shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
178+
{ code: '<div><label htmlFor="js_id">A label</label><input /></div>', options: [{ shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
179+
{ code: '<div><label htmlFor="js_id">A label</label><input name="js_id" /></div>', options: [{ shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
180+
{ code: '<div><label htmlFor={inputId} aria-label="A label" /><input name={inputId} /></div>', options: [{ shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
181+
];
167182

168183
const neverValid = (assertType) => {
169184
const expectedError = expectedErrors[assertType];
@@ -266,3 +281,21 @@ ruleTester.run(ruleName, rule, {
266281
assert: 'both',
267282
})).map(parserOptionsMapper),
268283
});
284+
285+
// shouldHtmlForMatchId
286+
ruleTester.run(ruleName, rule, {
287+
valid: parsers.all([].concat(
288+
...shouldHtmlForMatchIdValid,
289+
))
290+
.map(ruleOptionsMapperFactory({
291+
assert: 'htmlFor',
292+
}))
293+
.map(parserOptionsMapper),
294+
invalid: parsers.all([].concat(
295+
...shouldHtmlForMatchIdInvalid,
296+
))
297+
.map(ruleOptionsMapperFactory({
298+
assert: 'htmlFor',
299+
}))
300+
.map(parserOptionsMapper),
301+
});

‎__tests__/src/util/getChildComponent-test.js

+224
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
import test from 'tape';
2+
3+
import getChildComponent from '../../../src/util/getChildComponent';
4+
import JSXAttributeMock from '../../../__mocks__/JSXAttributeMock';
5+
import JSXElementMock from '../../../__mocks__/JSXElementMock';
6+
import JSXExpressionContainerMock from '../../../__mocks__/JSXExpressionContainerMock';
7+
8+
test('mayContainChildComponent', (t) => {
9+
t.equal(
10+
getChildComponent(
11+
JSXElementMock('div', [], [
12+
JSXElementMock('div', [], [
13+
JSXElementMock('span', [], []),
14+
JSXElementMock('span', [], [
15+
JSXElementMock('span', [], []),
16+
JSXElementMock('span', [], [
17+
JSXElementMock('span', [], []),
18+
]),
19+
]),
20+
]),
21+
JSXElementMock('span', [], []),
22+
JSXElementMock('img', [
23+
JSXAttributeMock('src', 'some/path'),
24+
]),
25+
]),
26+
'FancyComponent',
27+
5,
28+
),
29+
undefined,
30+
'no FancyComponent returns undefined',
31+
);
32+
33+
t.test('contains an indicated component', (st) => {
34+
const inputMock = JSXElementMock('input');
35+
st.equal(
36+
getChildComponent(
37+
JSXElementMock('div', [], [
38+
inputMock,
39+
]),
40+
'input',
41+
),
42+
inputMock,
43+
'returns input',
44+
);
45+
46+
const FancyComponentMock = JSXElementMock('FancyComponent');
47+
st.equal(
48+
getChildComponent(
49+
JSXElementMock('div', [], [
50+
FancyComponentMock,
51+
]),
52+
'FancyComponent',
53+
),
54+
FancyComponentMock,
55+
'returns FancyComponent',
56+
);
57+
58+
st.equal(
59+
getChildComponent(
60+
JSXElementMock('div', [], [
61+
JSXElementMock('div', [], [
62+
JSXElementMock('FancyComponent'),
63+
]),
64+
]),
65+
'FancyComponent',
66+
),
67+
undefined,
68+
'FancyComponent is outside of default depth, should return undefined',
69+
);
70+
71+
st.equal(
72+
getChildComponent(
73+
JSXElementMock('div', [], [
74+
JSXElementMock('div', [], [
75+
FancyComponentMock,
76+
]),
77+
]),
78+
'FancyComponent',
79+
2,
80+
),
81+
FancyComponentMock,
82+
'FancyComponent is inside of custom depth, should return FancyComponent',
83+
);
84+
85+
st.equal(
86+
getChildComponent(
87+
JSXElementMock('div', [], [
88+
JSXElementMock('div', [], [
89+
JSXElementMock('span', [], []),
90+
JSXElementMock('span', [], [
91+
JSXElementMock('span', [], []),
92+
JSXElementMock('span', [], [
93+
JSXElementMock('span', [], [
94+
JSXElementMock('span', [], [
95+
FancyComponentMock,
96+
]),
97+
]),
98+
]),
99+
]),
100+
]),
101+
JSXElementMock('span', [], []),
102+
JSXElementMock('img', [
103+
JSXAttributeMock('src', 'some/path'),
104+
]),
105+
]),
106+
'FancyComponent',
107+
6,
108+
),
109+
FancyComponentMock,
110+
'deep nesting, returns FancyComponent',
111+
);
112+
113+
st.end();
114+
});
115+
116+
const MysteryBox = JSXExpressionContainerMock('mysteryBox');
117+
t.equal(
118+
getChildComponent(
119+
JSXElementMock('div', [], [
120+
MysteryBox,
121+
]),
122+
'FancyComponent',
123+
),
124+
MysteryBox,
125+
'Indeterminate situations + expression container children - returns component',
126+
);
127+
128+
t.test('Glob name matching - component name contains question mark ? - match any single character', (st) => {
129+
const FancyComponentMock = JSXElementMock('FancyComponent');
130+
st.equal(
131+
getChildComponent(
132+
JSXElementMock('div', [], [
133+
FancyComponentMock,
134+
]),
135+
'Fanc?Co??onent',
136+
),
137+
FancyComponentMock,
138+
'returns component',
139+
);
140+
141+
st.equal(
142+
getChildComponent(
143+
JSXElementMock('div', [], [
144+
JSXElementMock('FancyComponent'),
145+
]),
146+
'FancyComponent?',
147+
),
148+
undefined,
149+
'returns undefined',
150+
);
151+
152+
st.test('component name contains asterisk * - match zero or more characters', (s2t) => {
153+
s2t.equal(
154+
getChildComponent(
155+
JSXElementMock('div', [], [
156+
FancyComponentMock,
157+
]),
158+
'Fancy*',
159+
),
160+
FancyComponentMock,
161+
'returns component',
162+
);
163+
164+
s2t.equal(
165+
getChildComponent(
166+
JSXElementMock('div', [], [
167+
FancyComponentMock,
168+
]),
169+
'*Component',
170+
),
171+
FancyComponentMock,
172+
'returns component',
173+
);
174+
175+
s2t.equal(
176+
getChildComponent(
177+
JSXElementMock('div', [], [
178+
FancyComponentMock,
179+
]),
180+
'Fancy*C*t',
181+
),
182+
FancyComponentMock,
183+
'returns component',
184+
);
185+
186+
s2t.end();
187+
});
188+
189+
st.end();
190+
});
191+
192+
t.test('using a custom elementType function', (st) => {
193+
const CustomInputMock = JSXElementMock('CustomInput');
194+
st.equal(
195+
getChildComponent(
196+
JSXElementMock('div', [], [
197+
CustomInputMock,
198+
]),
199+
'input',
200+
2,
201+
() => 'input',
202+
),
203+
CustomInputMock,
204+
'returns the component when the custom elementType returns the proper name',
205+
);
206+
207+
st.equal(
208+
getChildComponent(
209+
JSXElementMock('div', [], [
210+
JSXElementMock('CustomInput'),
211+
]),
212+
'input',
213+
2,
214+
() => 'button',
215+
),
216+
undefined,
217+
'returns undefined when the custom elementType returns a wrong name',
218+
);
219+
220+
st.end();
221+
});
222+
223+
t.end();
224+
});

0 commit comments

Comments
 (0)