Skip to content

Commit 8d691db

Browse files
khiga8khiga8
authored
Merge pull request #448 from github/kh-bump-aria-query
Bump aria-query and update to fix tests
2 parents 65fa6f2 + 40c0b2b commit 8d691db

8 files changed

+378
-115
lines changed

‎lib/rules/role-supports-aria-props.js

+4-44
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,7 @@
11
// @ts-check
2-
const {aria, elementRoles, roles} = require('aria-query')
3-
const {getProp, getPropValue, propName} = require('jsx-ast-utils')
4-
const {getElementType} = require('../utils/get-element-type')
5-
const ObjectMap = require('../utils/object-map')
6-
7-
// Clean-up `elementRoles` from `aria-query`
8-
const elementRolesMap = new ObjectMap()
9-
for (const [key, value] of elementRoles.entries()) {
10-
// - Remove unused `constraints` key
11-
delete key.constraints
12-
key.attributes = key.attributes?.filter(attribute => !('constraints' in attribute))
13-
// - Remove empty `attributes` key
14-
if (!key.attributes || key.attributes?.length === 0) {
15-
delete key.attributes
16-
}
17-
elementRolesMap.set(key, value)
18-
}
19-
// - Remove insufficiently-disambiguated `menuitem` entry
20-
elementRolesMap.delete({name: 'menuitem'})
21-
// - Disambiguate `menuitem` and `menu` roles by `type`
22-
elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'command'}]}, ['menuitem'])
23-
elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'radio'}]}, ['menuitemradio'])
24-
elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
25-
elementRolesMap.set({name: 'menu', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
2+
const {aria, roles} = require('aria-query')
3+
const {getPropValue, propName} = require('jsx-ast-utils')
4+
const {getRole} = require('../utils/get-role')
265

276
module.exports = {
287
meta: {
@@ -37,27 +16,8 @@ module.exports = {
3716
create(context) {
3817
return {
3918
JSXOpeningElement(node) {
40-
// Assemble a key for looking-up the element’s role in the `elementRolesMap`
41-
// - Get the element’s name
42-
const key = {name: getElementType(context, node)}
43-
// - Get the element’s disambiguating attributes
44-
for (const prop of ['aria-expanded', 'type', 'size', 'role', 'href', 'multiple', 'scope']) {
45-
// - Only provide `aria-expanded` when it’s required for disambiguation
46-
if (prop === 'aria-expanded' && key.name !== 'summary') continue
47-
const value = getPropValue(getProp(node.attributes, prop))
48-
if (value) {
49-
if (!('attributes' in key)) {
50-
key.attributes = []
51-
}
52-
if (prop === 'href') {
53-
key.attributes.push({name: prop})
54-
} else {
55-
key.attributes.push({name: prop, value})
56-
}
57-
}
58-
}
5919
// Get the element’s explicit or implicit role
60-
const role = getPropValue(getProp(node.attributes, 'role')) ?? elementRolesMap.get(key)?.[0]
20+
const role = getRole(context, node)
6121

6222
// Return early if role could not be determined
6323
if (!role) return

‎lib/utils/get-role.js

+108
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
const {getProp, getPropValue} = require('jsx-ast-utils')
2+
const {elementRoles} = require('aria-query')
3+
const {getElementType} = require('./get-element-type')
4+
const ObjectMap = require('./object-map')
5+
6+
const elementRolesMap = cleanElementRolesMap()
7+
8+
/*
9+
Returns an element roles map which uses `aria-query`'s elementRoles as the foundation.
10+
We additionally clean the data so we're able to fetch a role using a key we construct based on the node we're looking at.
11+
In a few scenarios, we stray from the roles returned by `aria-query` and hard code the mapping.
12+
*/
13+
function cleanElementRolesMap() {
14+
const rolesMap = new ObjectMap()
15+
16+
for (const [key, value] of elementRoles.entries()) {
17+
// - Remove empty `attributes` key
18+
if (!key.attributes || key.attributes?.length === 0) {
19+
delete key.attributes
20+
}
21+
rolesMap.set(key, value)
22+
}
23+
// Remove insufficiently-disambiguated `menuitem` entry
24+
rolesMap.delete({name: 'menuitem'})
25+
// Disambiguate `menuitem` and `menu` roles by `type`
26+
rolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'command'}]}, ['menuitem'])
27+
rolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'radio'}]}, ['menuitemradio'])
28+
rolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
29+
rolesMap.set({name: 'menu', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
30+
31+
/* These have constraints defined in aria-query's `elementRoles` which depend on knowledge of ancestor roles which we cant accurately determine in a linter context.
32+
However, we benefit more from assuming the role, than assuming it's generic or undefined so we opt to hard code the mapping */
33+
rolesMap.set({name: 'aside'}, ['complementary']) // `aside` still maps to `complementary` in https://www.w3.org/TR/html-aria/#docconformance.
34+
rolesMap.set({name: 'li'}, ['listitem']) // `li` can be generic if it's not within a list but we would never want to render `li` outside of a list.
35+
36+
return rolesMap
37+
}
38+
39+
/*
40+
Determine role of an element, based on its name and attributes.
41+
We construct a key and look up the element's role in `elementRolesMap`.
42+
If there is no match, we return undefined.
43+
*/
44+
function getRole(context, node) {
45+
// Early return if role is explicitly set
46+
const explicitRole = getPropValue(getProp(node.attributes, 'role'))
47+
if (explicitRole) {
48+
return explicitRole
49+
}
50+
51+
// Assemble a key for looking-up the element’s role in the `elementRolesMap`
52+
// - Get the element’s name
53+
const key = {name: getElementType(context, node)}
54+
55+
for (const prop of [
56+
'aria-label',
57+
'aria-labelledby',
58+
'alt',
59+
'type',
60+
'size',
61+
'role',
62+
'href',
63+
'multiple',
64+
'scope',
65+
'name',
66+
]) {
67+
if ((prop === 'aria-labelledby' || prop === 'aria-label') && !['section', 'form'].includes(key.name)) continue
68+
if (prop === 'name' && key.name !== 'form') continue
69+
if (prop === 'href' && key.name !== 'a' && key.name !== 'area') continue
70+
if (prop === 'alt' && key.name !== 'img') continue
71+
72+
const propOnNode = getProp(node.attributes, prop)
73+
74+
if (!('attributes' in key)) {
75+
key.attributes = []
76+
}
77+
// Disambiguate "undefined" props
78+
if (propOnNode === undefined && prop === 'alt' && key.name === 'img') {
79+
key.attributes.push({name: prop, constraints: ['undefined']})
80+
continue
81+
}
82+
83+
const value = getPropValue(propOnNode)
84+
if (value || (value === '' && prop === 'alt')) {
85+
if (
86+
prop === 'href' ||
87+
prop === 'aria-labelledby' ||
88+
prop === 'aria-label' ||
89+
prop === 'name' ||
90+
(prop === 'alt' && value !== '')
91+
) {
92+
key.attributes.push({name: prop, constraints: ['set']})
93+
} else {
94+
key.attributes.push({name: prop, value})
95+
}
96+
}
97+
}
98+
99+
// - Remove empty `attributes` key
100+
if (!key.attributes || key.attributes?.length === 0) {
101+
delete key.attributes
102+
}
103+
104+
// Get the element’s implicit role
105+
return elementRolesMap.get(key)?.[0]
106+
}
107+
108+
module.exports = {getRole}

‎package-lock.json

+22-9
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"@github/browserslist-config": "^1.0.0",
3333
"@typescript-eslint/eslint-plugin": "^5.1.0",
3434
"@typescript-eslint/parser": "^5.1.0",
35-
"aria-query": "^5.1.3",
35+
"aria-query": "^5.3.0",
3636
"eslint-config-prettier": ">=8.0.0",
3737
"eslint-plugin-escompat": "^3.3.3",
3838
"eslint-plugin-eslint-comments": "^3.2.0",

‎tests/role-supports-aria-props.js

+14-36
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,6 @@ ruleTester.run('role-supports-aria-props', rule, {
5757
{code: '<a href="#" aria-owns />'},
5858
{code: '<a href="#" aria-relevant />'},
5959

60-
// this will have global
61-
{code: '<a aria-checked />'},
62-
6360
// AREA TESTS - implicit role is `link`
6461
{code: '<area href="#" aria-expanded />'},
6562
{code: '<area href="#" aria-atomic />'},
@@ -78,30 +75,6 @@ ruleTester.run('role-supports-aria-props', rule, {
7875
{code: '<area href="#" aria-owns />'},
7976
{code: '<area href="#" aria-relevant />'},
8077

81-
// this will have global
82-
{code: '<area aria-checked />'},
83-
84-
// LINK TESTS - implicit role is `link`
85-
{code: '<link href="#" aria-expanded />'},
86-
{code: '<link href="#" aria-atomic />'},
87-
{code: '<link href="#" aria-busy />'},
88-
{code: '<link href="#" aria-controls />'},
89-
{code: '<link href="#" aria-describedby />'},
90-
{code: '<link href="#" aria-disabled />'},
91-
{code: '<link href="#" aria-dropeffect />'},
92-
{code: '<link href="#" aria-flowto />'},
93-
{code: '<link href="#" aria-grabbed />'},
94-
{code: '<link href="#" aria-hidden />'},
95-
{code: '<link href="#" aria-haspopup />'},
96-
{code: '<link href="#" aria-label />'},
97-
{code: '<link href="#" aria-labelledby />'},
98-
{code: '<link href="#" aria-live />'},
99-
{code: '<link href="#" aria-owns />'},
100-
{code: '<link href="#" aria-relevant />'},
101-
102-
// this will have global
103-
{code: '<link aria-checked />'},
104-
10578
// this will have role of `img`
10679
{code: '<img alt="foobar" aria-busy />'},
10780

@@ -344,20 +317,25 @@ ruleTester.run('role-supports-aria-props', rule, {
344317
{code: '<datalist aria-expanded />'},
345318
{code: '<div role="heading" aria-level />'},
346319
{code: '<div role="heading" aria-level="1" />'},
320+
{code: '<link href="#" aria-expanded />'}, // link maps to nothing
347321
],
348322

349323
invalid: [
350324
// implicit basic checks
351325
{
352-
code: '<a href="#" aria-checked />',
353-
errors: [getErrorMessage('aria-checked', 'link')],
326+
code: '<area aria-checked />',
327+
errors: [getErrorMessage('aria-checked', 'generic')],
354328
},
355329
{
356-
code: '<area href="#" aria-checked />',
330+
code: '<a aria-checked />',
331+
errors: [getErrorMessage('aria-checked', 'generic')],
332+
},
333+
{
334+
code: '<a href="#" aria-checked />',
357335
errors: [getErrorMessage('aria-checked', 'link')],
358336
},
359337
{
360-
code: '<link href="#" aria-checked />',
338+
code: '<area href="#" aria-checked />',
361339
errors: [getErrorMessage('aria-checked', 'link')],
362340
},
363341
{
@@ -394,7 +372,7 @@ ruleTester.run('role-supports-aria-props', rule, {
394372
},
395373
{
396374
code: '<body aria-expanded />',
397-
errors: [getErrorMessage('aria-expanded', 'document')],
375+
errors: [getErrorMessage('aria-expanded', 'generic')],
398376
},
399377
{
400378
code: '<li aria-expanded />',
@@ -414,6 +392,10 @@ ruleTester.run('role-supports-aria-props', rule, {
414392
},
415393
{
416394
code: '<section aria-expanded />',
395+
errors: [getErrorMessage('aria-expanded', 'generic')],
396+
},
397+
{
398+
code: '<section aria-label="something" aria-expanded />',
417399
errors: [getErrorMessage('aria-expanded', 'region')],
418400
},
419401
{
@@ -480,10 +462,6 @@ ruleTester.run('role-supports-aria-props', rule, {
480462
code: '<menu type="toolbar" aria-expanded />',
481463
errors: [getErrorMessage('aria-expanded', 'toolbar')],
482464
},
483-
{
484-
code: '<link href="#" aria-invalid />',
485-
errors: [getErrorMessage('aria-invalid', 'link')],
486-
},
487465
{
488466
code: '<area href="#" aria-invalid />',
489467
errors: [getErrorMessage('aria-invalid', 'link')],

‎tests/utils/get-element-type.js

+2-25
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,11 @@
11
const {getElementType} = require('../../lib/utils/get-element-type')
2+
const {mockJSXAttribute, mockJSXOpeningElement} = require('./mocks')
3+
24
const mocha = require('mocha')
35
const describe = mocha.describe
46
const it = mocha.it
57
const expect = require('chai').expect
68

7-
function mockJSXAttribute(prop, propValue) {
8-
return {
9-
type: 'JSXAttribute',
10-
name: {
11-
type: 'JSXIdentifier',
12-
name: prop,
13-
},
14-
value: {
15-
type: 'Literal',
16-
value: propValue,
17-
},
18-
}
19-
}
20-
21-
function mockJSXOpeningElement(tagName, attributes = []) {
22-
return {
23-
type: 'JSXOpeningElement',
24-
name: {
25-
type: 'JSXIdentifier',
26-
name: tagName,
27-
},
28-
attributes,
29-
}
30-
}
31-
329
function mockSetting(componentSetting = {}) {
3310
return {
3411
settings: {

0 commit comments

Comments
 (0)