Skip to content

Commit

Permalink
fix(ByRole): Constrain API (testing-library#1211)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Only allow `string` as a `role`.
Drop support for `exact`, `trim`, `collapseWhitespace`, and `normalizer` options.
  • Loading branch information
eps1lon authored Feb 9, 2023
1 parent 2d360a6 commit 746def6
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 74 deletions.
39 changes: 16 additions & 23 deletions src/__tests__/element-queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -788,35 +788,28 @@ test('queryAllByRole returns semantic html elements', () => {
</form>
`)

expect(queryAllByRole(/table/i)).toHaveLength(1)
expect(queryAllByRole(/tabl/i, {exact: false})).toHaveLength(1)
expect(queryAllByRole(/columnheader/i)).toHaveLength(1)
expect(queryAllByRole(/rowheader/i)).toHaveLength(1)
expect(queryAllByRole(/grid/i)).toHaveLength(1)
expect(queryAllByRole(/form/i)).toHaveLength(0)
expect(queryAllByRole(/button/i)).toHaveLength(1)
expect(queryAllByRole(/heading/i)).toHaveLength(6)
expect(queryAllByRole('table')).toHaveLength(1)
expect(queryAllByRole('columnheader')).toHaveLength(1)
expect(queryAllByRole('rowheader')).toHaveLength(1)
expect(queryAllByRole('grid')).toHaveLength(1)
expect(queryAllByRole('form')).toHaveLength(0)
expect(queryAllByRole('button')).toHaveLength(1)
expect(queryAllByRole('heading')).toHaveLength(6)
expect(queryAllByRole('list')).toHaveLength(2)
expect(queryAllByRole(/listitem/i)).toHaveLength(3)
expect(queryAllByRole(/textbox/i)).toHaveLength(2)
expect(queryAllByRole(/checkbox/i)).toHaveLength(1)
expect(queryAllByRole(/radio/i)).toHaveLength(1)
expect(queryAllByRole('listitem')).toHaveLength(3)
expect(queryAllByRole('textbox')).toHaveLength(2)
expect(queryAllByRole('checkbox')).toHaveLength(1)
expect(queryAllByRole('radio')).toHaveLength(1)
expect(queryAllByRole('row')).toHaveLength(3)
expect(queryAllByRole(/rowgroup/i)).toHaveLength(2)
expect(queryAllByRole(/(table)|(textbox)/i)).toHaveLength(3)
expect(queryAllByRole(/img/i)).toHaveLength(1)
expect(queryAllByRole('rowgroup')).toHaveLength(2)
expect(queryAllByRole('img')).toHaveLength(1)
expect(queryAllByRole('meter')).toHaveLength(1)
expect(queryAllByRole('progressbar')).toHaveLength(0)
expect(queryAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)
expect(queryAllByRole('combobox')).toHaveLength(1)
expect(queryAllByRole('listbox')).toHaveLength(1)
})

test('queryByRole matches case with non-string matcher', () => {
const {queryByRole} = render(`<span role="1" />`)
expect(queryByRole(1)).toBeTruthy()
})

test('getAll* matchers return an array', () => {
const {
getAllByAltText,
Expand All @@ -827,7 +820,7 @@ test('getAll* matchers return an array', () => {
getAllByText,
getAllByRole,
} = render(`
<div role="container">
<div role="section">
<img
data-testid="poster"
alt="finding nemo poster"
Expand Down Expand Up @@ -864,7 +857,7 @@ test('getAll* matchers return an array', () => {
expect(getAllByDisplayValue('Japanese cars')).toHaveLength(1)
expect(getAllByDisplayValue(/cars$/)).toHaveLength(2)
expect(getAllByText(/^where/i)).toHaveLength(1)
expect(getAllByRole(/container/i)).toHaveLength(1)
expect(getAllByRole('section')).toHaveLength(1)
expect(getAllByRole('meter')).toHaveLength(1)
expect(getAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)
})
Expand All @@ -879,7 +872,7 @@ test('getAll* matchers throw for 0 matches', () => {
getAllByText,
getAllByRole,
} = render(`
<div role="container">
<div role="section">
<label>No Matches Please</label>
</div>,
`)
Expand Down
8 changes: 4 additions & 4 deletions src/__tests__/get-by-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ cases(
html: `<div title="his"></div><div title="history"></div>`,
},
getByRole: {
query: /his/,
html: `<div role="his"></div><div role="history"></div>`,
query: 'button',
html: `<button>one</button><div role="button">two</button>`,
},
getByTestId: {
query: /his/,
Expand Down Expand Up @@ -87,8 +87,8 @@ cases(
html: `<div title="his"></div><div title="history"></div>`,
},
queryByRole: {
query: /his/,
html: `<div role="his"></div><div role="history"></div>`,
query: 'button',
html: `<button>one</button><div role="button">two</button>`,
},
queryByTestId: {
query: /his/,
Expand Down
4 changes: 0 additions & 4 deletions src/__tests__/text-matchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,6 @@ cases(
dom: `<input value="User ${LRM}name" />`,
queryFn: 'queryAllByDisplayValue',
},
queryAllByRole: {
dom: `<input role="User ${LRM}name" />`,
queryFn: 'queryAllByRole',
},
},
)

Expand Down
49 changes: 11 additions & 38 deletions src/queries/role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,17 @@ import {
Matcher,
MatcherFunction,
MatcherOptions,
NormalizerFn,
} from '../../types'

import {
buildQueries,
fuzzyMatches,
getConfig,
makeNormalizer,
matches,
} from './all-utils'
import {buildQueries, getConfig, matches} from './all-utils'

const queryAllByRole: AllByRole = (
container,
role,
{
exact = true,
collapseWhitespace,
hidden = getConfig().defaultHidden,
name,
description,
trim,
normalizer,
queryFallbacks = false,
selected,
checked,
Expand All @@ -61,8 +50,6 @@ const queryAllByRole: AllByRole = (
} = {},
) => {
checkContainerType(container)
const matcher = exact ? matches : fuzzyMatches
const matchNormalizer = makeNormalizer({collapseWhitespace, trim, normalizer})

if (selected !== undefined) {
// guard against unknown roles
Expand Down Expand Up @@ -136,7 +123,7 @@ const queryAllByRole: AllByRole = (
return Array.from(
container.querySelectorAll<HTMLElement>(
// Only query elements that can be matched by the following filters
makeRoleSelector(role, exact, normalizer ? matchNormalizer : undefined),
makeRoleSelector(role),
),
)
.filter(node => {
Expand All @@ -148,22 +135,18 @@ const queryAllByRole: AllByRole = (
return roleValue
.split(' ')
.filter(Boolean)
.some(text => matcher(text, node, role as Matcher, matchNormalizer))
}
// if a custom normalizer is passed then let normalizer handle the role value
if (normalizer) {
return matcher(roleValue, node, role as Matcher, matchNormalizer)
.some(roleAttributeToken => roleAttributeToken === role)
}
// other wise only send the first word to match
const [firstWord] = roleValue.split(' ')
return matcher(firstWord, node, role as Matcher, matchNormalizer)
// other wise only send the first token to match
const [firstRoleAttributeToken] = roleValue.split(' ')
return firstRoleAttributeToken === role
}

const implicitRoles = getImplicitAriaRoles(node) as string[]

return implicitRoles.some(implicitRole =>
matcher(implicitRole, node, role as Matcher, matchNormalizer),
)
return implicitRoles.some(implicitRole => {
return implicitRole === role
})
})
.filter(element => {
if (selected !== undefined) {
Expand Down Expand Up @@ -228,18 +211,8 @@ const queryAllByRole: AllByRole = (
})
}

function makeRoleSelector(
role: ByRoleMatcher,
exact: boolean,
customNormalizer?: NormalizerFn,
) {
if (typeof role !== 'string') {
// For non-string role parameters we can not determine the implicitRoleSelectors.
return '*'
}

const explicitRoleSelector =
exact && !customNormalizer ? `*[role~="${role}"]` : '*[role]'
function makeRoleSelector(role: ByRoleMatcher) {
const explicitRoleSelector = `*[role~="${role}"]`

const roleRelations =
roleElements.get(role as ARIARoleDefinitionKey) ?? new Set()
Expand Down
2 changes: 0 additions & 2 deletions types/__tests__/type-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@ export async function testByRole() {
)

console.assert(queryByRole(element, 'foo') === null)
console.assert(queryByRole(element, /foo/) === null)
console.assert(screen.queryByRole('foo') === null)
console.assert(screen.queryByRole(/foo/) === null)
}

export function testA11yHelper() {
Expand Down
4 changes: 2 additions & 2 deletions types/matches.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ export type MatcherFunction = (
export type Matcher = MatcherFunction | RegExp | number | string

// Get autocomplete for ARIARole union types, while still supporting another string
// Ref: https://github.com/microsoft/TypeScript/issues/29729#issuecomment-505826972
export type ByRoleMatcher = ARIARole | MatcherFunction | {}
// Ref: https://github.com/microsoft/TypeScript/issues/29729#issuecomment-567871939
export type ByRoleMatcher = ARIARole | (string & {})

export type NormalizerFn = (text: string) => string

Expand Down
4 changes: 3 additions & 1 deletion types/queries.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ export type FindByText<T extends HTMLElement = HTMLElement> = (
waitForElementOptions?: waitForOptions,
) => Promise<T>

export interface ByRoleOptions extends MatcherOptions {
export interface ByRoleOptions {
/** suppress suggestions for a specific query */
suggest?: boolean
/**
* If true includes elements in the query set that are usually excluded from
* the accessibility tree. `role="none"` or `role="presentation"` are included
Expand Down

0 comments on commit 746def6

Please sign in to comment.