Skip to content

Commit 746def6

Browse files
authored
fix(ByRole): Constrain API (#1211)
BREAKING CHANGE: Only allow `string` as a `role`. Drop support for `exact`, `trim`, `collapseWhitespace`, and `normalizer` options.
1 parent 2d360a6 commit 746def6

File tree

7 files changed

+36
-74
lines changed

7 files changed

+36
-74
lines changed

src/__tests__/element-queries.js

+16-23
Original file line numberDiff line numberDiff line change
@@ -788,35 +788,28 @@ test('queryAllByRole returns semantic html elements', () => {
788788
</form>
789789
`)
790790

791-
expect(queryAllByRole(/table/i)).toHaveLength(1)
792-
expect(queryAllByRole(/tabl/i, {exact: false})).toHaveLength(1)
793-
expect(queryAllByRole(/columnheader/i)).toHaveLength(1)
794-
expect(queryAllByRole(/rowheader/i)).toHaveLength(1)
795-
expect(queryAllByRole(/grid/i)).toHaveLength(1)
796-
expect(queryAllByRole(/form/i)).toHaveLength(0)
797-
expect(queryAllByRole(/button/i)).toHaveLength(1)
798-
expect(queryAllByRole(/heading/i)).toHaveLength(6)
791+
expect(queryAllByRole('table')).toHaveLength(1)
792+
expect(queryAllByRole('columnheader')).toHaveLength(1)
793+
expect(queryAllByRole('rowheader')).toHaveLength(1)
794+
expect(queryAllByRole('grid')).toHaveLength(1)
795+
expect(queryAllByRole('form')).toHaveLength(0)
796+
expect(queryAllByRole('button')).toHaveLength(1)
797+
expect(queryAllByRole('heading')).toHaveLength(6)
799798
expect(queryAllByRole('list')).toHaveLength(2)
800-
expect(queryAllByRole(/listitem/i)).toHaveLength(3)
801-
expect(queryAllByRole(/textbox/i)).toHaveLength(2)
802-
expect(queryAllByRole(/checkbox/i)).toHaveLength(1)
803-
expect(queryAllByRole(/radio/i)).toHaveLength(1)
799+
expect(queryAllByRole('listitem')).toHaveLength(3)
800+
expect(queryAllByRole('textbox')).toHaveLength(2)
801+
expect(queryAllByRole('checkbox')).toHaveLength(1)
802+
expect(queryAllByRole('radio')).toHaveLength(1)
804803
expect(queryAllByRole('row')).toHaveLength(3)
805-
expect(queryAllByRole(/rowgroup/i)).toHaveLength(2)
806-
expect(queryAllByRole(/(table)|(textbox)/i)).toHaveLength(3)
807-
expect(queryAllByRole(/img/i)).toHaveLength(1)
804+
expect(queryAllByRole('rowgroup')).toHaveLength(2)
805+
expect(queryAllByRole('img')).toHaveLength(1)
808806
expect(queryAllByRole('meter')).toHaveLength(1)
809807
expect(queryAllByRole('progressbar')).toHaveLength(0)
810808
expect(queryAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)
811809
expect(queryAllByRole('combobox')).toHaveLength(1)
812810
expect(queryAllByRole('listbox')).toHaveLength(1)
813811
})
814812

815-
test('queryByRole matches case with non-string matcher', () => {
816-
const {queryByRole} = render(`<span role="1" />`)
817-
expect(queryByRole(1)).toBeTruthy()
818-
})
819-
820813
test('getAll* matchers return an array', () => {
821814
const {
822815
getAllByAltText,
@@ -827,7 +820,7 @@ test('getAll* matchers return an array', () => {
827820
getAllByText,
828821
getAllByRole,
829822
} = render(`
830-
<div role="container">
823+
<div role="section">
831824
<img
832825
data-testid="poster"
833826
alt="finding nemo poster"
@@ -864,7 +857,7 @@ test('getAll* matchers return an array', () => {
864857
expect(getAllByDisplayValue('Japanese cars')).toHaveLength(1)
865858
expect(getAllByDisplayValue(/cars$/)).toHaveLength(2)
866859
expect(getAllByText(/^where/i)).toHaveLength(1)
867-
expect(getAllByRole(/container/i)).toHaveLength(1)
860+
expect(getAllByRole('section')).toHaveLength(1)
868861
expect(getAllByRole('meter')).toHaveLength(1)
869862
expect(getAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)
870863
})
@@ -879,7 +872,7 @@ test('getAll* matchers throw for 0 matches', () => {
879872
getAllByText,
880873
getAllByRole,
881874
} = render(`
882-
<div role="container">
875+
<div role="section">
883876
<label>No Matches Please</label>
884877
</div>,
885878
`)

src/__tests__/get-by-errors.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ cases(
3636
html: `<div title="his"></div><div title="history"></div>`,
3737
},
3838
getByRole: {
39-
query: /his/,
40-
html: `<div role="his"></div><div role="history"></div>`,
39+
query: 'button',
40+
html: `<button>one</button><div role="button">two</button>`,
4141
},
4242
getByTestId: {
4343
query: /his/,
@@ -87,8 +87,8 @@ cases(
8787
html: `<div title="his"></div><div title="history"></div>`,
8888
},
8989
queryByRole: {
90-
query: /his/,
91-
html: `<div role="his"></div><div role="history"></div>`,
90+
query: 'button',
91+
html: `<button>one</button><div role="button">two</button>`,
9292
},
9393
queryByTestId: {
9494
query: /his/,

src/__tests__/text-matchers.js

-4
Original file line numberDiff line numberDiff line change
@@ -291,10 +291,6 @@ cases(
291291
dom: `<input value="User ${LRM}name" />`,
292292
queryFn: 'queryAllByDisplayValue',
293293
},
294-
queryAllByRole: {
295-
dom: `<input role="User ${LRM}name" />`,
296-
queryFn: 'queryAllByRole',
297-
},
298294
},
299295
)
300296

src/queries/role.ts

+11-38
Original file line numberDiff line numberDiff line change
@@ -29,28 +29,17 @@ import {
2929
Matcher,
3030
MatcherFunction,
3131
MatcherOptions,
32-
NormalizerFn,
3332
} from '../../types'
3433

35-
import {
36-
buildQueries,
37-
fuzzyMatches,
38-
getConfig,
39-
makeNormalizer,
40-
matches,
41-
} from './all-utils'
34+
import {buildQueries, getConfig, matches} from './all-utils'
4235

4336
const queryAllByRole: AllByRole = (
4437
container,
4538
role,
4639
{
47-
exact = true,
48-
collapseWhitespace,
4940
hidden = getConfig().defaultHidden,
5041
name,
5142
description,
52-
trim,
53-
normalizer,
5443
queryFallbacks = false,
5544
selected,
5645
checked,
@@ -61,8 +50,6 @@ const queryAllByRole: AllByRole = (
6150
} = {},
6251
) => {
6352
checkContainerType(container)
64-
const matcher = exact ? matches : fuzzyMatches
65-
const matchNormalizer = makeNormalizer({collapseWhitespace, trim, normalizer})
6653

6754
if (selected !== undefined) {
6855
// guard against unknown roles
@@ -136,7 +123,7 @@ const queryAllByRole: AllByRole = (
136123
return Array.from(
137124
container.querySelectorAll<HTMLElement>(
138125
// Only query elements that can be matched by the following filters
139-
makeRoleSelector(role, exact, normalizer ? matchNormalizer : undefined),
126+
makeRoleSelector(role),
140127
),
141128
)
142129
.filter(node => {
@@ -148,22 +135,18 @@ const queryAllByRole: AllByRole = (
148135
return roleValue
149136
.split(' ')
150137
.filter(Boolean)
151-
.some(text => matcher(text, node, role as Matcher, matchNormalizer))
152-
}
153-
// if a custom normalizer is passed then let normalizer handle the role value
154-
if (normalizer) {
155-
return matcher(roleValue, node, role as Matcher, matchNormalizer)
138+
.some(roleAttributeToken => roleAttributeToken === role)
156139
}
157-
// other wise only send the first word to match
158-
const [firstWord] = roleValue.split(' ')
159-
return matcher(firstWord, node, role as Matcher, matchNormalizer)
140+
// other wise only send the first token to match
141+
const [firstRoleAttributeToken] = roleValue.split(' ')
142+
return firstRoleAttributeToken === role
160143
}
161144

162145
const implicitRoles = getImplicitAriaRoles(node) as string[]
163146

164-
return implicitRoles.some(implicitRole =>
165-
matcher(implicitRole, node, role as Matcher, matchNormalizer),
166-
)
147+
return implicitRoles.some(implicitRole => {
148+
return implicitRole === role
149+
})
167150
})
168151
.filter(element => {
169152
if (selected !== undefined) {
@@ -228,18 +211,8 @@ const queryAllByRole: AllByRole = (
228211
})
229212
}
230213

231-
function makeRoleSelector(
232-
role: ByRoleMatcher,
233-
exact: boolean,
234-
customNormalizer?: NormalizerFn,
235-
) {
236-
if (typeof role !== 'string') {
237-
// For non-string role parameters we can not determine the implicitRoleSelectors.
238-
return '*'
239-
}
240-
241-
const explicitRoleSelector =
242-
exact && !customNormalizer ? `*[role~="${role}"]` : '*[role]'
214+
function makeRoleSelector(role: ByRoleMatcher) {
215+
const explicitRoleSelector = `*[role~="${role}"]`
243216

244217
const roleRelations =
245218
roleElements.get(role as ARIARoleDefinitionKey) ?? new Set()

types/__tests__/type-tests.ts

-2
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,7 @@ export async function testByRole() {
183183
)
184184

185185
console.assert(queryByRole(element, 'foo') === null)
186-
console.assert(queryByRole(element, /foo/) === null)
187186
console.assert(screen.queryByRole('foo') === null)
188-
console.assert(screen.queryByRole(/foo/) === null)
189187
}
190188

191189
export function testA11yHelper() {

types/matches.d.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ export type MatcherFunction = (
77
export type Matcher = MatcherFunction | RegExp | number | string
88

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

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

types/queries.d.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@ export type FindByText<T extends HTMLElement = HTMLElement> = (
6666
waitForElementOptions?: waitForOptions,
6767
) => Promise<T>
6868

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

0 commit comments

Comments
 (0)