Providing a way to skip hidden check to boost RTL's ByRole performance

[ptamarit]

Hello,

I created the following repository showcasing performance issues with React Testing Library's ByRole queries: https://github.com/ptamarit/find-by-role-perf
It's using Material-UI's date picker v3, and the test is accessing date buttons by role and accessible name.

I'm already using hidden: true to bypass the getComputedStyle call in DOM Testing Library.
It improved performance, but it's still pretty bad.

The biggest time spent in the test is on computeAccessibleName.
It's particularly visible in the case of this calendar as there are 30+ buttons, meaning that accessible names needs to be computed for all of these buttons to find matches.

Half of the time spent on computeAccessibleName is spent on the isHidden method.

Which brings me to the following questions:

• Would it make sense to add an option to dom-accessibility-api allowing to skip this hidden check?
• If yes, then dom-testing-library could check if ByRole is called with hidden: true and pass it on to dom-accessibility-api.

Happy to hear your thoughts.

Cheers,
Pablo

[eps1lon]

Thanks for the feedback.

This is already possible by injecting a custom implementation of getComputedStyles: computeAccessibleName(element, { getComputedStyle: () => ({ display: 'block', visibility: 'visible' }) }). We should add docs for this though.

With regards to testing-library, I wouldn't want to skip this check. You should instead not use the accessible name to query the element and use other queries like ByText.

[ptamarit]

I totally missed the possibility to inject a custom getComputedStyle, thanks a lot.

Thanks for the suggestion, I ended up using the following query instead of screen.findByRole('button', { name: text }):

async function findParentButtonByText(text) {
  let element = await screen.findByText(text);
  while (element.tagName !== 'BUTTON') {
    if (element.parentElement) {
      element = element.parentElement
    } else {
      throw Error('Reached the top parent and did not find any button')
    }
  }
  return element
}

[ptamarit]

Hey @eps1lon,

I wanted to thank you for your suggestion of injecting a custom getComputedStyle. 🙏
I revisited this issue and tried your suggestion, and the tests in the aforementioned repo went from 22s to 2s.

Before:

PASS src/App.test.js (22.15 s)
  ✓ one day (1985 ms)
  ✓ two days (2987 ms)
  ✓ three days (3746 ms)
  ✓ ten days (11555 ms)

After:

PASS src/App.test.js
  ✓ one day (517 ms)
  ✓ two days (337 ms)
  ✓ three days (345 ms)
  ✓ ten days (602 ms)

I followed what you're doing in MUI, which is to disable these expensive operations locally but still run them on the CI environment:

// jest-dom adds custom jest matchers for asserting on DOM nodes.
// allows you to do things like:
// expect(element).toHaveTextContent(/react/i)
// learn more: https://github.com/testing-library/jest-dom
import '@testing-library/jest-dom';
import { configure } from '@testing-library/react';

// Applying changes to speed up tests locally.
// These changes are not applied on the Continuous Integration environment.
if (!process.env.CI) {
  // Skipping the checks on elements visibility for ByRole queries, since this is quite slow.
  configure({ defaultHidden: true });

  // Setting defaultHidden is not enough to improve performance when using ByRole with big DOM structures,
  // since a lot of time is also spent checking visibilities for determining accessible names.
  // Therefore, we override getComputedStyle to make elements always visible.
  // See: https://github.com/eps1lon/dom-accessibility-api/discussions/605
  Object.defineProperty(window, 'getComputedStyle', {
    value: () => ({
      getPropertyValue: (prop) => {
        if (prop === 'display') {
          return 'block';
        } else if (prop === 'visibility') {
          return 'visible';
        } else {
          return '';
        }
      },
    }),
  });
}

Thanks again!
Pablo