getByRole("progressbar") is pretty slow

[ValentinH]

• React Testing Library version: 10.0.4 (I'm using /react not /dom but I'm posting here as getByRole is implemented here)
• node version: 🤷 (codesandbox)
• npm (or yarn) version: 🤷 (codesandbox)

I've noticed that some of my tests were pretty slow to run (more than 2 seconds). After investigating, I've noticed that the following pattern was to source of the slowness:

await waitForElementToBeRemoved(() => screen.getByRole('progressbar'))

If I add a data-testid to my loader and switch to:

await waitForElementToBeRemoved(() => screen.getByTestId('loader'))

it's much faster.

Relevant code or config:

I'm sometimes displayed a CircularProgress from material-ui while fetching some data and I use this pattern to wait for it to be removed.

await waitForElementToBeRemoved(() => screen.getByRole('progressbar'))

example of implementation

if(isLoading) {
  return <CircularProgress className={classes.loader} data-testid="loader" />
}

What happened:

Here are 2 tests outputs where I only change the pattern mentioned above.

getByRole

getByTestId:

Reproduction:

I've create the following Sandbox: https://codesandbox.io/s/musing-gauss-e1ynf?file=/src/Demo.js

The numbers are much smaller but when running the tests several times we can see that the getByRole is slower when used on the CircularProgress.

Problem description:

I think that

await waitForElementToBeRemoved(() => screen.getByRole('progressbar'))

is the best pattern to test that a loader is displayed on screen. However, the slowness prevents us from using in all our tests as it quickly gets slow.

[eps1lon]

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.

[ValentinH]

Thanks for the quick reply!

I've just tested the { hidden: false } and it doesn't have an impact in my example unfortunately. 😞
The thing is that in the sandbox example, the tree is not so big. What I don't get, it the difference when using CircularProgress and directly using its output div + svg.

I'm using version 25.2.7 of Jest.

[eps1lon]

I've just tested the { hidden: false } and it doesn't have an impact in my example unfortunately.

Have you tried increasing the interval? Would be helpful if you could provide a full repro. Otherwise it's unclear what makes this test so slow.

What I don't get, it the difference when using CircularProgress and directly using its output div + svg.

It's unclear where this difference is coming from. Could just be the CircularProgress doing more things and slowing the test down because of it.

[ValentinH]

Increasing the interval didn't help either.

I'm preparing a demo with my real use case :)

[ValentinH]

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.

[eps1lon]

Great. I think this is somewhat expected with the current implementation. ByTestId takes 130ms while ByRole takes 200ms. This is 50% slower.

Might be nice to experiment with different implementations that don't loop over all nodes but convert the role directly to a selector and use that directly.

[ValentinH]

I've dug in the (transpiled) code and try to see where it was spending the most time. Surprisingly, this is not in queryAllByRole, but at this line:

dom-testing-library/src/query-helpers.js

Line 62 in ca1b608

throw getConfig().getElementError(

Especially, the call to getMissingError(). Before this call every operations takes a few milliseconds. getMissingError takes almost 1 second on my example!

[ValentinH]

Digging even more, I'm landing on this line: https://github.com/testing-library/dom-testing-library/blob/master/src/queries/role.js#L116

However, I'm using 7.3.0 and seeing that you changed the implementation in 7.4.0. Thus, I need to check if it has changed the behaviour. 🙂

[ValentinH]

Still the same behavior on 7.5.1.

I'm now checking this line:

dom-testing-library/src/role-helpers.js

Line 134 in 429de04

return hidden === false ? isInaccessible(element) === false : true

isInaccessible gets called 30 times when getByRole rejects and each call last around 30 ms.

[ValentinH]

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

[kentcdodds]

Hi @ValentinH! Thanks for all this digging.

I believe that isInaccessible has historically been some bottleneck code. I'm not sure of the best way to speed it up, but if anyone has ideas that would be pretty sweet :)

[ValentinH]

Yes, I'm trying to see what part of it is the slowest part :)

[ValentinH]

Not a big surprise, but most of the time is spent on

window.getComputedStyle(element).display