dom-testing-library: getByRole("progressbar") is pretty slow

getByRole can get quite expensive in large trees since it currently checks every element while ByTestId is a simple querySelector. It also includes a lot more a11y related checks. For example it won’t return inaccessible elements.

Since you actually remove the element from the DOM you can ignore the additional a11y tree checks and go with getByRole('progressbar', { hidden: false }).

On slow machines these additional checks can starve your CPU and you should increase the polling interval with waitForElementToBeRemoved(() => {}, { interval: 200 }) (try out which interval works best).

And last but not least: What version of jest or, more specifically, jsdom are you using? In later jsdom versions (or actual browsers) getByRole should be a lot faster.

For us: I think we should make the perf impact of hidden in ByRole clearer in the docs. Also mention too low polling rate on slow machines.

I came here after realizing there’s a huge performance difference between getByRole and other query methods. Using getByText immediately makes my tests run 8x faster. I understand the reasons for recommending getByRole but that much additional time spent waiting for tests adds up fast for a large project. I think the docs should be up front about this so people can make their informed decisions about what method to use.

I personally think a PR to the docs is warranted. Something like: “NOTE: some have experienced poor performance with the role query, learn more: link”

For those who use styled-components a lot, such wrapper could improve getByRole performance by leaving only a few css rules used by @testing-library and thus significantly reducing <style> size

import { ReactNode } from 'react'
import { StyleSheetManager, StylisPlugin } from 'styled-components'

const ALLOWED_RULES = ['display', 'visibility', 'pointer-events']

const simplifiedStylesPlugin: StylisPlugin = (context, content) => {
  if (context === 1) {
    if (!ALLOWED_RULES.some(rule => content.toString().startsWith(`${rule}:`))) {
      return ''
    }
  }

  return undefined
}

export const withSimplifiedStyles = (component: ReactNode) => (
  <StyleSheetManager stylisPlugins={[simplifiedStylesPlugin]} disableVendorPrefixes>
    {component}
  </StyleSheetManager>
)

https://github.com/eps1lon/dom-accessibility-api/pull/323 This PR improves getByRole performance by up to 20% by removing one extra getComputedStyle call

Hey guys I don’t know if anybody else opened a new issue but today I had this exact issue, one of my test suite started to have systematic timeouts after updating to new version of testing library and updating my queries to getByRole I run a bit of profiling and those are my findings

Let’s say I have an Input component similar to this

<Input
  {...el.input}
  placeholder="My El"
  data-testid="MyEl"
/>

In my tests I’m running this:

console.log('pre getByTestId', performance.now())
screen.getByTestId('MyEl')
console.log('after getByTestId', performance.now())
console.log('pre getByRole', performance.now())
screen.getByRole('textbox', { name: /My El/ })
console.log('after getByRole', performance.now())
console.log('pre getByText', performance.now())
screen.getByText(/My El/)
console.log('after getByText', performance.now())

Those are my results before without configure({ defaultHidden: true })

pre getByTestId 10829.295586
after getByTestId 10835.550812
pre getByRole 10836.638695
after getByRole 12147.583377
pre getByText 12148.72231
after getByText 12153.527319

Those are the results after configure({ defaultHidden: true })

pre getByTestId 6450.390944
after getByTestId 6454.832352
pre getByRole 6457.684077
after getByRole 7683.902226
pre getByText 7693.771436
after getByText 7698.411979

In general for me getByRole is slower it’s 1 / 2 sec vs 5ms with the other queries

@arahansen I didn’t take the task at the end, because I’m not sure about how useful is going to be. Even if benchmarks here have some point, after a few more talks about it, turns out that small sized teams or alone individuals are probably going to assume that the doc is talking about a huge performance impact (even if you used the right wording there, or even if you provide the link), and they’re going to do it maybe without any real performance assessment on the test stages. That means, final result could be people opting-out of getByRole (and all that implies) for not a really outstanding practical reason.

I was not using waitFor and getByRole was still unusably slow in a complex form component.

Please open a new issue (since this issue was concerned about ByRole in waitFor) and fill out the template. Having a repro like the one @tommarien provided helps a lot.

I’ve updated the demo with a more complex form and here’s the result:

Sandbox: https://codesandbox.io/s/musing-gauss-e1ynf?file=/src/DemoForm.js

I’ve removed the CircularProgress so that we don’t focus on it.

I’ve just update testing-library/dom in the reproduction repo and see times go from 280ms on average to 38ms 😉, so thanks a lot 👍 . If next release of testing-libs dom dependency to > 7.6.0 than everyone can benefit from this 😃. @eps1lon Thanks and i’ll remove the repro repo today

🎉 This issue has been resolved in version 7.6.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

@eps1lon I think the demo I shared above is already showing a big difference on a quite complex form: https://codesandbox.io/s/musing-gauss-e1ynf?file=/src/DemoForm.js

I’ve added some content around the form to make the tests even slower though.

This is my real use case except I have a bunch of providers on top.

Ok, so I think the simplest solution is to default hidden to true and make the default configurable. I know the arguments against this that @eps1lon will have (we’ve been through this before) but I’m convinced that opting into this kind of a performance drain in exchange for the confidence is better than having to opt out of it. I worry that people will switch from ByRole to a suboptimal query due to performance issues and I don’t think that’s wise.

Because this is a breaking change, we should do this in two phases.

1. Make it configurable and release that as a minor version
2. Change the default and release that as a major version (maybe batched with other major changes in we can think of any).

Anyone want to work on phase 1?

I think we really need to be careful here. I want to give it a little bit of extra thought…

If I understand correctly were determining the accessibility of everything before matching. Is that right? If so… What if we find the match first and then determine if it’s accessible?

I ran into this today while trying to debug why a mocked network request was so slow. I’ll try to put together a minimal reproduction but the Cliffs Notes is:

My app code was making a network request and on success would navigate to another route. I was waiting for the new route to load with screen.findByRole('heading', {name: 'xxx'}). Using console.time around the fetch call was showing ~1900ms elapsed. A similar request that would 400 was taking around ~22ms to display the error message. Swapping to screen.findByText('xxx') brought the elapsed time for the former down to ~25ms. I took a look at the profiler and it seems that *ByRole is starving my CPU. I tried both {hidden: true} and {interval: 500} with no improvement. This is in a toy app with a pretty tiny DOM.

The only real difference between the tests seems to be the route change. I’ll try to put together a minimal reproduction this weekend to narrow it down.

Funnily, if I do getByRole('progressbar', { hidden: true }) it solves my issue as it does not call isInaccessible. 😄