Skip to content

Commit

Permalink
Merge pull request #448 from github/kh-bump-aria-query
Browse files Browse the repository at this point in the history
Bump aria-query and update to fix tests
  • Loading branch information
khiga8 committed Jul 13, 2023
2 parents 65fa6f2 + 40c0b2b commit 8d691db
Show file tree
Hide file tree
Showing 8 changed files with 378 additions and 115 deletions.
48 changes: 4 additions & 44 deletions lib/rules/role-supports-aria-props.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,7 @@
// @ts-check
const {aria, elementRoles, roles} = require('aria-query')
const {getProp, getPropValue, propName} = require('jsx-ast-utils')
const {getElementType} = require('../utils/get-element-type')
const ObjectMap = require('../utils/object-map')

// Clean-up `elementRoles` from `aria-query`
const elementRolesMap = new ObjectMap()
for (const [key, value] of elementRoles.entries()) {
// - Remove unused `constraints` key
delete key.constraints
key.attributes = key.attributes?.filter(attribute => !('constraints' in attribute))
// - Remove empty `attributes` key
if (!key.attributes || key.attributes?.length === 0) {
delete key.attributes
}
elementRolesMap.set(key, value)
}
// - Remove insufficiently-disambiguated `menuitem` entry
elementRolesMap.delete({name: 'menuitem'})
// - Disambiguate `menuitem` and `menu` roles by `type`
elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'command'}]}, ['menuitem'])
elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'radio'}]}, ['menuitemradio'])
elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
elementRolesMap.set({name: 'menu', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
const {aria, roles} = require('aria-query')
const {getPropValue, propName} = require('jsx-ast-utils')
const {getRole} = require('../utils/get-role')

module.exports = {
meta: {
Expand All @@ -37,27 +16,8 @@ module.exports = {
create(context) {
return {
JSXOpeningElement(node) {
// Assemble a key for looking-up the element’s role in the `elementRolesMap`
// - Get the element’s name
const key = {name: getElementType(context, node)}
// - Get the element’s disambiguating attributes
for (const prop of ['aria-expanded', 'type', 'size', 'role', 'href', 'multiple', 'scope']) {
// - Only provide `aria-expanded` when it’s required for disambiguation
if (prop === 'aria-expanded' && key.name !== 'summary') continue
const value = getPropValue(getProp(node.attributes, prop))
if (value) {
if (!('attributes' in key)) {
key.attributes = []
}
if (prop === 'href') {
key.attributes.push({name: prop})
} else {
key.attributes.push({name: prop, value})
}
}
}
// Get the element’s explicit or implicit role
const role = getPropValue(getProp(node.attributes, 'role')) ?? elementRolesMap.get(key)?.[0]
const role = getRole(context, node)

// Return early if role could not be determined
if (!role) return
Expand Down
108 changes: 108 additions & 0 deletions lib/utils/get-role.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
const {getProp, getPropValue} = require('jsx-ast-utils')
const {elementRoles} = require('aria-query')
const {getElementType} = require('./get-element-type')
const ObjectMap = require('./object-map')

const elementRolesMap = cleanElementRolesMap()

/*
Returns an element roles map which uses `aria-query`'s elementRoles as the foundation.
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.
In a few scenarios, we stray from the roles returned by `aria-query` and hard code the mapping.
*/
function cleanElementRolesMap() {
const rolesMap = new ObjectMap()

for (const [key, value] of elementRoles.entries()) {
// - Remove empty `attributes` key
if (!key.attributes || key.attributes?.length === 0) {
delete key.attributes
}
rolesMap.set(key, value)
}
// Remove insufficiently-disambiguated `menuitem` entry
rolesMap.delete({name: 'menuitem'})
// Disambiguate `menuitem` and `menu` roles by `type`
rolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'command'}]}, ['menuitem'])
rolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'radio'}]}, ['menuitemradio'])
rolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
rolesMap.set({name: 'menu', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])

/* 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.
However, we benefit more from assuming the role, than assuming it's generic or undefined so we opt to hard code the mapping */
rolesMap.set({name: 'aside'}, ['complementary']) // `aside` still maps to `complementary` in https://www.w3.org/TR/html-aria/#docconformance.
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.

return rolesMap
}

/*
Determine role of an element, based on its name and attributes.
We construct a key and look up the element's role in `elementRolesMap`.
If there is no match, we return undefined.
*/
function getRole(context, node) {
// Early return if role is explicitly set
const explicitRole = getPropValue(getProp(node.attributes, 'role'))
if (explicitRole) {
return explicitRole
}

// Assemble a key for looking-up the element’s role in the `elementRolesMap`
// - Get the element’s name
const key = {name: getElementType(context, node)}

for (const prop of [
'aria-label',
'aria-labelledby',
'alt',
'type',
'size',
'role',
'href',
'multiple',
'scope',
'name',
]) {
if ((prop === 'aria-labelledby' || prop === 'aria-label') && !['section', 'form'].includes(key.name)) continue
if (prop === 'name' && key.name !== 'form') continue
if (prop === 'href' && key.name !== 'a' && key.name !== 'area') continue
if (prop === 'alt' && key.name !== 'img') continue

const propOnNode = getProp(node.attributes, prop)

if (!('attributes' in key)) {
key.attributes = []
}
// Disambiguate "undefined" props
if (propOnNode === undefined && prop === 'alt' && key.name === 'img') {
key.attributes.push({name: prop, constraints: ['undefined']})
continue
}

const value = getPropValue(propOnNode)
if (value || (value === '' && prop === 'alt')) {
if (
prop === 'href' ||
prop === 'aria-labelledby' ||
prop === 'aria-label' ||
prop === 'name' ||
(prop === 'alt' && value !== '')
) {
key.attributes.push({name: prop, constraints: ['set']})
} else {
key.attributes.push({name: prop, value})
}
}
}

// - Remove empty `attributes` key
if (!key.attributes || key.attributes?.length === 0) {
delete key.attributes
}

// Get the element’s implicit role
return elementRolesMap.get(key)?.[0]
}

module.exports = {getRole}
31 changes: 22 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"@github/browserslist-config": "^1.0.0",
"@typescript-eslint/eslint-plugin": "^5.1.0",
"@typescript-eslint/parser": "^5.1.0",
"aria-query": "^5.1.3",
"aria-query": "^5.3.0",
"eslint-config-prettier": ">=8.0.0",
"eslint-plugin-escompat": "^3.3.3",
"eslint-plugin-eslint-comments": "^3.2.0",
Expand Down
50 changes: 14 additions & 36 deletions tests/role-supports-aria-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ ruleTester.run('role-supports-aria-props', rule, {
{code: '<a href="#" aria-owns />'},
{code: '<a href="#" aria-relevant />'},

// this will have global
{code: '<a aria-checked />'},

// AREA TESTS - implicit role is `link`
{code: '<area href="#" aria-expanded />'},
{code: '<area href="#" aria-atomic />'},
Expand All @@ -78,30 +75,6 @@ ruleTester.run('role-supports-aria-props', rule, {
{code: '<area href="#" aria-owns />'},
{code: '<area href="#" aria-relevant />'},

// this will have global
{code: '<area aria-checked />'},

// LINK TESTS - implicit role is `link`
{code: '<link href="#" aria-expanded />'},
{code: '<link href="#" aria-atomic />'},
{code: '<link href="#" aria-busy />'},
{code: '<link href="#" aria-controls />'},
{code: '<link href="#" aria-describedby />'},
{code: '<link href="#" aria-disabled />'},
{code: '<link href="#" aria-dropeffect />'},
{code: '<link href="#" aria-flowto />'},
{code: '<link href="#" aria-grabbed />'},
{code: '<link href="#" aria-hidden />'},
{code: '<link href="#" aria-haspopup />'},
{code: '<link href="#" aria-label />'},
{code: '<link href="#" aria-labelledby />'},
{code: '<link href="#" aria-live />'},
{code: '<link href="#" aria-owns />'},
{code: '<link href="#" aria-relevant />'},

// this will have global
{code: '<link aria-checked />'},

// this will have role of `img`
{code: '<img alt="foobar" aria-busy />'},

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

invalid: [
// implicit basic checks
{
code: '<a href="#" aria-checked />',
errors: [getErrorMessage('aria-checked', 'link')],
code: '<area aria-checked />',
errors: [getErrorMessage('aria-checked', 'generic')],
},
{
code: '<area href="#" aria-checked />',
code: '<a aria-checked />',
errors: [getErrorMessage('aria-checked', 'generic')],
},
{
code: '<a href="#" aria-checked />',
errors: [getErrorMessage('aria-checked', 'link')],
},
{
code: '<link href="#" aria-checked />',
code: '<area href="#" aria-checked />',
errors: [getErrorMessage('aria-checked', 'link')],
},
{
Expand Down Expand Up @@ -394,7 +372,7 @@ ruleTester.run('role-supports-aria-props', rule, {
},
{
code: '<body aria-expanded />',
errors: [getErrorMessage('aria-expanded', 'document')],
errors: [getErrorMessage('aria-expanded', 'generic')],
},
{
code: '<li aria-expanded />',
Expand All @@ -414,6 +392,10 @@ ruleTester.run('role-supports-aria-props', rule, {
},
{
code: '<section aria-expanded />',
errors: [getErrorMessage('aria-expanded', 'generic')],
},
{
code: '<section aria-label="something" aria-expanded />',
errors: [getErrorMessage('aria-expanded', 'region')],
},
{
Expand Down Expand Up @@ -480,10 +462,6 @@ ruleTester.run('role-supports-aria-props', rule, {
code: '<menu type="toolbar" aria-expanded />',
errors: [getErrorMessage('aria-expanded', 'toolbar')],
},
{
code: '<link href="#" aria-invalid />',
errors: [getErrorMessage('aria-invalid', 'link')],
},
{
code: '<area href="#" aria-invalid />',
errors: [getErrorMessage('aria-invalid', 'link')],
Expand Down
27 changes: 2 additions & 25 deletions tests/utils/get-element-type.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,11 @@
const {getElementType} = require('../../lib/utils/get-element-type')
const {mockJSXAttribute, mockJSXOpeningElement} = require('./mocks')

const mocha = require('mocha')
const describe = mocha.describe
const it = mocha.it
const expect = require('chai').expect

function mockJSXAttribute(prop, propValue) {
return {
type: 'JSXAttribute',
name: {
type: 'JSXIdentifier',
name: prop,
},
value: {
type: 'Literal',
value: propValue,
},
}
}

function mockJSXOpeningElement(tagName, attributes = []) {
return {
type: 'JSXOpeningElement',
name: {
type: 'JSXIdentifier',
name: tagName,
},
attributes,
}
}

function mockSetting(componentSetting = {}) {
return {
settings: {
Expand Down
Loading

0 comments on commit 8d691db

Please sign in to comment.