Create a specification for getNodeText used by queries

[NicholasBoll]

Describe the feature you'd like:

I've seen churn on what "text" means for a given element. The pendulum has swung from innerText to textContent based off unexpected use-cases. I think we should create a specification for what getNodeText returns for the queries that rely on it.

Effected queries:

• *ByTitle
• *ByText
• *ByDisplayValue

I think *ByLabelText used to use it, but it is no longer.

Some examples that aren't working as expected.

Nested inline elements

This example is probably more common for Cypress, Puppeteer, etc. For example, Cypress implicitly waits for actionability. If a button is disabled and the test calls cy.findByText('Hello').click(), Cypress will not wait for the disabled to be removed from the button element. There is no workaround other than not using findByText at all. Adding a selector should define what element we want to return, but getNodeText won't be able to find anything.

<button><span>Hello</span></button>
<script>
  getByText('Hello') // returns span
  getByText('Hello', { selector: 'button' }) // error - not found
  // expected: `button` element
</script>

<th>
  Country
  <!-- Country Filter for the table header -->
  <select>
    <option>Colombia</option>
  </select>
</th>
<script>
  getByText('Country') // returns th
  // expected: `th` element
</script>

Inconsistencies with *ByLabelText

<label for="1234">My Label<abbr>*</abbr></label>
<input id="1234">
<script>
  getByText('My Label'); // returns <label>
  getByLabelText('My Label'); // Error: Unable to find a label with the text of: My Label
  getByText('My Label*'); // Error: Unable to find an element with the text: My Label*. This could be because the text is broken up by multiple elements. In ...
  getByLabelText('My Label*'); // returns <input>
</script>

There are probably other edge cases we can build a specification around.

This would be a breaking change for existing implementations that rely on the current functionality. For example:

<button><span>Hello</span></button>
<script>
  getByText('Hello') // error, 2 elements found. Before, only span was returned
  getAllByText('Hello') // [button, span]. Before, only [span] was returned
  getByText('Hello', { selector: 'span' }) // span
  getByText('Hello', { selector: 'button' }) // button
</script>

Suggested implementation:

Write specifications for what should be returned given a few different DOM structures.

Describe alternatives you've considered:

I've looked through some of the history around the implementation of the getNodeText function. It seems edge cases have driven the implementation, but there isn't much conditional coding to handle multiple edge cases at once.

Teachability, Documentation, Adoption, Migration Strategy:

Hopefully this feature doesn't require much documentation as it will mostly do what one would expect. Edge cases could be enumerated in the documentation, however. Something like:

getByText tries to grab elements by most likely text of an element. For example, a button element with nested span elements will return the whole text of the button, spans included. getByText also tries to filter out unintended text. For example, a table header that contains a select element will remove the text of the select element itself as that is most likely the intent of selecting the table's header text.

[eps1lon]

I think we want to encourage getByRole('button', { name: 'Hello' }) for these specific examples. I think this should be preferred for most use cases but I'd be interested in scenarios where you want *ByText on an element without a role.

[NicholasBoll]

Interesting, getByRole('button', {name: 'Hello'}) didn't work for me. It tells me multiple elements were found with the role of 'button' (if there are multiple buttons on the page). This happened on v4 and v5 of this library. Is it supposed to work?

[eps1lon]

This happened on v4 and v5 of this library. Is it supposed to work?

Possibly if there are indeed multiple elements with that role and name. But this was only added in 6.15 so you might be on a version that doesn't support the name filter.

The text that is used for name even has a spec (full docs).

[NicholasBoll]

Oops, I was looking at @testing-library/jest-dom. This library is a transitive dependency of @testing-library/react. The version I was using was 6.12. It looks like 6.15 was released 5 days ago.

The name spec is hard to read, but it looks like it could cover the button-specific use case.

[eps1lon]

The name spec is hard to read, but it looks like it could cover the button-specific use case.

You don't actually need to read it but rather know what an accessible name is. Should get you most of the way. For button it is essentially the textContent.

[NicholasBoll]

I also like to separate selectors from assertions for specifications. It helps understand failures:

<nav>
  <!-- elements here -->
  <button aria-label="Page 1">1</button>
  <!-- more elements here -->
</nav>

expect(getByText('1')).toHaveAttribute('aria-label', 'Page 1')
// example failures
// * No element:
//    Error: Unable to find an element with the text: 1. This could be because the text is broken up by multiple elements. In this ca...
// * Missing label:
//     Expected the element to have attribute: aria-label="Page 1" Received: null
// * Wrong label:
//     Expected the element to have attribute: aria-label="Page 1" Received: aria-label="Foobar"

// vs

getByLabelText('Page 1')
// example failures - all of them could have a HUGE dump of HTML
// * No element:
//     Error: Unable to find a label with the text of: Page 1 <body> <div> <nav aria-label="Pagination" class="css-r...
// * Missing label:
//     Error: Unable to find a label with the text of: Page 1 <body> <div> <nav aria-label="Pagination" class="css-r...
// * Wrong label:
//     Error: Unable to find a label with the text of: Page 1 <body> <div> <nav aria-label="Pagination" class="css-r...

If I only have 1 element, maybe it doesn't matter. If I have a few elements, it gets harder to understand which element is missing what. For understanding the spec? I think it matters. If I change my above example:

// oops, spec says aria-label, aria-labelledby, title, aria-describe, and aria-describedby take precedence.
expect(getByRole('button', { name: '1' })).toHaveAttribute('aria-label', 'Page 1') // error

// same problem as above, only worse. How does `Page 1` get calculated?
// Does this mean the button should visibly show `Page 1` or show `Page 1` for assistive technology?
getByRole('button', { name: 'Page 1' })

If getByRole supports computeAccessibleName from dom-accessible-api, why not have jest-dom support it with a matcher?

expect(getByText('1')).toHaveName('Page 1') // This will work event if we change `aria-label` to `aria-labelledby`

[NicholasBoll]

Note, after we've established a generic assertion works according to a specification, now we can move on to the specific selectors and assert something else interesting.

it('should have an accessible name for page button 1', () => {
  // awesome! I don't have to worry about _how_ that element's accessible name was calculated!
  // aria-label? aria-labelledby?
  expect(getByText('1')).toHaveName('Page 1')
})

it('should change the page', () => {
  fireEvent('click', getByRole('button', { name: 'Page 1' })
  // hmm... name changed. Maybe we still want to getBy something easily verifiable
  expect(getByText('1')).toHaveName('Selected, Page 1')
})

[eps1lon]

I also like to separate selectors from assertions for specifications.

Sure. But this looks like you want to assert on the accessible name. I think getting it by test-id (or role) and then matching against the accessible name is closest to what you actually want.

render(<button data-testid="first-page" aria-label="Page 1">1</button>);
const button = screen.getByTestId("first-page");
expect(computeAccessibleName(button)).toBe('Page 1');

or maybe better

render(<button aria-label="Page 1">1</button>);
const button = screen.getByRole("button", "Page 1");
expect(button).toHaveText('1');

computeAccessibleName can be imported from dom-accessibility-api which is used internally.

Though this example looks a bit l ike speech-overkill. I don't think you want to repeat "page" on every link.

[NicholasBoll]

Your first example is closer to what I want. I want a to start with the easiest, least abstracted implementation and then assert the more complicated, higher abstracted implementation.

Your first example has a much clearer starting point. You're grabbing an element by an attribute you manually added. It is simple enough to match those in the test if something goes wrong. The assertion could be improved with a matcher.

A matcher is an assertion with additional context. The failure here would look like:

  Error: Expected 'Foobar' to equal 'Page 1'

This doesn't give me much to go off of. It's like getting a bug report stating "It doesn't work". I'd rather tests and implementation be written like specifications that read like acceptance tests. When it fails, it tells you the what the specification is (the English part of a test) and the failure tells you exactly how it fails.

expect(button).toHave[Accessible]Name('Page 1')

Failures can now look like:

Error: Expected <button ...> to have an accessible name of 'Page 1', but was calculated to be 'Foobar'. See https://www.w3.org/TR/accname-1.1/ for more details on how accessible names are calculated
// bonus points if you can tell me how the name was calculated! From text? aria-label? title? etc.

The second example is definitely not what I want. The complicated abstraction is up-front. If screen.getByRole fails to find an element with an accessible name of 'Page 1', what does that mean? The best context I get from those types of "failed to find something" is a dump of all the HTML. What if there is a lot of HTML? Accessible names can be calculated in a complicated fashion indirectly through IDREFs (references through an id attribute. e.g. <label for="1234"> + <input id="1234">)

In the Pagination example of buttons with numbers on them, it is easier to understand if a button with "1" in it isn't found. That is easy enough to verify by the HTML or by rendering to a screen. Once I have the element, matchers have enough context for useful failure messages.

[eps1lon]

expect(button).toHave[Accessible]Name('Page 1')

That is what I do personally but this repository is not concerned with matchers. As you said: You want to decouple matchers from getters.

[NicholasBoll]

Agreed. You wanted to know scenarios where *ByText queries would be used without a role. I think *ByRole('button', { text: 'Hello' }) would be helpful in identifying a role and text. This should still be easy enough to manually verify if a query isn't found.

I did some further testing:

<label for="1234">My Label<abbr>*</abbr></label>
<input id="1234">
<script>
  getByLabelText('My Label*') // returns <input> That `*` is required
  getByText('My Label') // returns <label>. Cannot include the `*`
</script>

Both those queries are by the text, but the "text" part is different.

[eps1lon]

I don't follow what the issue is now. The button examples you gave in the opening post can switch to *ByRole + name which has an official spec. *ByDisplayValue is a different issue.

[NicholasBoll]

A case where I have a button text that is different than the accessible name:

<button aria-label="Page 1"><span>1</span></button>
<script>
  getByText('1') // doesn't work
  getByRole('button', { name: '1' }) // doesn't work. `name` is actually 'Page 1'
  getByRole('button', { text: '1' }) // API doesn't exist

  // works, but fails unhelpfully if we somehow remove or change the accessible name.
  getByRole('button', { name: 'Page 1'})
</script>

So I can't cover this case with existing APIs. I must resort to using low-level DOM API to accomplish this. It seems like getByText('1') should work. getByLabelText seems to handle multiple child elements just fine when evaluating text.

[eps1lon]

A case where I have a button text that is different than the accessible name

And why can't you query by the accessible name? Why is it so important to query the full text content?

It seems like you're tunnel visioning on

getByText()
matchAccessibleName()

when you can already do

getByAccessibleName()
matchText()

We cannot use the same method for getting the text in ByDisplayValue, ByText and ByLabelText. That would defeat the purpose of having different methods in the first place.

[eps1lon]

// works, but fails unhelpfully if we somehow remove or change the accessible name.

What do you mean "unhelpfully"? The error message should include the accessible names of each element in the a11y tree.