Skip to content

waitFor + getByRole causing severe delays #820

Open

Description

@herecydev
  • @testing-library/dom version: 7.26.6
  • node version: 14.3.0

Relevant code or config:

it("Causes a big delay", async () => {
      // This fires a fetch request that is mocked using MSW (so it should be near instant)
      userEvent.click(screen.getByRole("button", { name: "Search"}));

      await waitFor(() => {
        //super fast call
        //screen.getByText("some_link_text");

        // causes massive delays
        screen.getByRole("link", { name: "some_link" });
      });
    });

What you did:

Running this test causes the fetch request promise to resolve a lot longer than usual (around 1.5 - 2s). I initially investigated this by wrapping console.time around the request.

image

I also had an additional test which looks very similar to the above, but instead of using getByRole it was using getByText to search for some text. That test took a fraction of the time:

image

What happened:

After changing getByRole to getByText or inserting a getByText query before the getByRole it resolved much faster. My working theory is that getByRole is so expensive that it was monopolising the waitFor loop and takes a long time to relinquish control to the pending promise that needs to resolve.

Activity

  1. nickserv commented on Nov 13, 2020

    @nickserv
    Member

    The issue is that getByRole is fairly slow, but it's also syncronous, so waitFor keeps rerunning it and has to wait for each call to finish before other code can run. It could potentially use some performance improvements, but perhaps we could make it asyncronous (or add an alternative for back compatibility) so this can be run asyncronously.

  2. herecydev commented on Nov 13, 2020

    @herecydev
    Author

    Thanks for the quick reply Nick, does that not suggest that any waitFor that is sufficiently slow would trigger this? If so, is waitFor problematic in it's design?

    What do you think the best short term resolution for this?

  3. nickserv commented on Nov 13, 2020

    @nickserv
    Member

    I'm guessing waitFor may be so eager to quickly rerun longer functions that it might actually be slowed down. You could try increasing the interval option, it defaults to only 50 milliseconds.

  4. herecydev commented on Nov 13, 2020

    @herecydev
    Author

    For reference the getByRole was taking 400-500ms and would cause the waitFor to loop 3-4 times. The best fix (for time) is to insert a more trivial query before that (like a getByText) as that flushes that promise much faster.

    Would you like me to document some of this? Feels like a sufficiently frustrating morning that I don't want others to go through!

  5. nickserv commented on Nov 13, 2020

    @nickserv
    Member

    A higher interval might cause less loops. Even if it doesn't make this specific test run shorter, a longer interval could allow other tests to run in parallel more effectively. Can you try this out?

    Alternatively, we could dynamically adjust the interval by default based on how long it takes each time. I'm not sure if that's worth the effort though.

  6. herecydev commented on Nov 13, 2020

    @herecydev
    Author

    Setting an interval of 750 still caused 2 iterations to fail (~450ms), then the fetch promise resolves. There's one more failure (output still not rendered to the screen), finally it passes.

    Setting an interval underneath the getByRole time causes it to fail as expected. I.e. setting it to 200ms will cause 3 iterations and then the waitFor will just give up (total time is 1s by default if I remember).

    I guess it works, but you keep having to up this arbitrary interval to "match" however long the waitFor will take as the complexity grows. That doesn't feel great to me.

  7. nickserv commented on Nov 13, 2020

    @nickserv
    Member

    Yea, I was thinking maybe waitFor could intelligently learn how long previous calls took and use that to adjust the interval. I'm not sure how well it would work in practice though. On the other hand, you may not need *byRole (see #698 (comment)), though PRs to improve performance would be welcome.

  8. herecydev commented on Nov 13, 2020

    @herecydev
    Author

    The problem is when the knowledge has been propogated differently;

    https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#using-the-wrong-query
    https://testing-library.com/docs/guide-which-query/#priority

    There's absolutely nothing in the docs about the downsides of ByRole, and previously I've been happy to accept the small performance hit for the better testing methodology. But this issue is straight out problematic now.

    I would love to get an action on how best proceed with this, do we:

    1. auto-increment waitFor intelligently
    2. Change inner workings of ByRole to be asynchronous
    3. Provide documentation around this known issue with suitable guidance

    etc.

  9. nickserv commented on Nov 13, 2020

    @nickserv
    Member

    1 would be cool to experiment with but as I said previously, I'm not sure how difficult to implement and useful it would be, so it may not be worth it. I'm fine with either 2 or 3.

    @kentcdodds @eps1lon What do you think about an asyncronous version of *byRole (2) to work around performance issues like this?

  10. kentcdodds commented on Nov 13, 2020

    @kentcdodds
    Member

    One question to make sure I understand. What is your test waiting on that takes more than a single 50ms interval? Most/All tests using this utility should mock out any async code so it resolves on the next tick or two of the event loop 🤔

  11. kentcdodds commented on Nov 13, 2020

    @kentcdodds
    Member

    Just noticed the screenshot, sorry about that. Looks like it's getting mocked by MSW, so I wouldn't expect this to take longer than a few milliseconds. Something is up there.

    The *ByRole query is definitely the performance hog of the bunch, but it's a cost with the benefit of confidence which is why it's the top most recommended query. I suppose we could add a note to the docs warning about this. Probably a good idea. I'd love to make that query faster, but I still recommend it.

    In this situation, there's something especially fishy going on. I think we'd need a reproduction to dive any deeper though.

  12. herecydev commented on Nov 13, 2020

    @herecydev
    Author

    @kentcdodds yer to confirm MSW is registering is ~1ms so it's very fast. It's the *ByRole that's exceeding the waitFor interval window.

    Reproduction will take some time, is it not clear that exceeding the waitFor interval window will cause this?

  13. eps1lon commented on Nov 13, 2020

    @eps1lon
    Member

    If you want to optimize tests for performance go through these steps until you're satisfied. They're ordered by their confidence impact (lower impact, higher ranked) then perf impact (higher impact, higher ranked).

    1. Make sure you use the latest version of jsdom
    2. don't use JSDOM but modern browsers
    3. getByRole(role, { hidden: true }) (WARNING: includes inaccessible elements - avoids expensive visibility check)
    4. getByTestId instead of getByRole
    5. don't filter by name in getByRole (Can rely on order in getAllByRole. Order of elements in the DOM is in most cases not an implementation detail.)

    Not using jsdom at all is the best solution since it has better perf and higher confidence.

  14. kentcdodds commented on Nov 13, 2020

    @kentcdodds
    Member

    @eps1lon great walkthrough 👍 though I think I'd prefer to try another query before testIDs. I think it makes sense to add this to the docs.

  15. eps1lon commented on Nov 13, 2020

    @eps1lon
    Member

    though I think I'd prefer to try another query before testIDs. I think it makes sense to add this to the docs.

    Definitely! It was easier to say since "use another query" might not be that helpful. If you're really blocked by perf problems I would just suggest a quick escape hatch and revisit it once the test breaks on unrelated changes (though that might be even harder to judge). It's just really hard to give actionable advise that people can apply to different scenarios.

    I think the most generic advise is to re-evaluate your waitFor usage. If you're in a unit-test-like environment you should be able to mock the timers so that you can deterministically forward to the point where your assertion should be true (skipping the wasteful polling approach). In e2e tests (with real timers/requests) I'd like to see a reproduction since I would expect a browser to not have perf problems with getByRole(role, { name }).

    And to give an optimistic outlook: The ByRole implementation is just a polyfill at the moment. Long term we'll use actual DOM APIs that are hopefully faster (see https://wicg.github.io/aom/spec/#computed-accessibility-tree).

  16. 78 remaining items

  17. foxaltus commented on May 28, 2024

    @foxaltus

    Hey! I want to share my experience here as I've been struggling with a few slow tests that were consistently timing out.

    In my case, the slow tests were the result of using screen.findByRole("cell", { name: 'xxx' }) on tables (with less than 100 cells).

    I tried migrating to happy-dom, which was pretty much effortless (minus a few issues caused by poorly written tests on my side). However, I only obtained marginal gains with happy-dom. Tests that were timing out with jest-dom were also timing out with happy-dom.

    I tried using the hidden option too (or the global defaultHidden) of testing-library with marginal gains again, but not enough to solve the timeout.

    The only thing that worked for me was to replace findByRole with findByText. Not ideal, but it makes a huge difference.

    I am not sure if there is a particular issue with tables, but I would advise to stay away from accessible queries when working with them...

  18. TomPridham commented on Jun 5, 2024

    @TomPridham

    @eps1lon Have you considered replace filter with find in queryAllByRole when we use getByRole?

    .filter(element => {
    if (name === undefined) {
    // Don't care
    return true
    }
    return matches(
    computeAccessibleName(element, {
    computedStyleSupportsPseudoElements:
    getConfig().computedStyleSupportsPseudoElements,
    }),
    element,
    name as MatcherFunction,
    text => text,
    )
    })

    It will allow us to skip some expensive computeAccessibleName after we find our tartget.

    i don't think we want to do that because then the single element matchers wouldn't throw if there was more than one element

  19. frankandrobot commented on Jun 21, 2024

    @frankandrobot

    We've been exploring a possible workaround. Here's some context:

    • An element's inferred name is the inferred aria label. Ex: <button>A button</button> has an inferred aria label of "a button". There are also inferred roles. The button above has a role of "button", but almost anything can have a button role by just assigning role attribute a value of "button".
    • getByRole has quicker cousins, namely getByText and getByAriaLabel. (However, by themselves they don't check for role.)
    • The workaround is to use one of these cousins to search by label or inferred name and then use within on the parent element to assert the role. In testing, this has been almost as fast as plain getByText!

    But of course there are caveats:

    1. Users need to know whether or not the aria-label is inferred.
    2. The inferred name can only be in the top-level of the component. Otherwise, selection will fail. For example, in <td><div>a cell</div></td>, the inferred name of the cell is "a cell", so the workaround fails---it gets the DIV and then checks whether or not it's a cell, so it fails.
    3. The role check is done by calling within#getAllByRole (w/ no name) on the parent element. In edge cases, it's DOM tree can be just as complex as the root DOM tree, so there will be no speed up. (It may be possible to improve on this by temporarily detaching the node from the DOM.)

    Notes:

    1. Yes, you do need to know implementation details but IMO it isn't a big of a deal. It's a bigger issue w/ third party libraries but IMO there should be test selector helpers for these anyway.
    2. At least in our case, we use a shared component library, so instances of caveat 1) are limited to a handful of cases.

    The code:

    type GetByRoleFullOpts = Parameters<typeof original.getByRole>;
    type GetByRoleOpts = GetByRoleFullOpts[1] & { nameType?: "inferred" | "aria-label" }
    
    export const getByRole = (role: GetByRoleFullOpts[0], opts: GetByRoleOpts): GetByRoleReturn => {
      const { name, nameType, ...restOpts } = opts ?? {};
    
      if (name && typeof name !== "function") {
        const getter = nameType === "inferred" ? original.getByText : original.getByLabelText;
        const foundElement = getter(name, restOpts);
        const elements = within(foundElement.parentElement ?? document.documentElement).getAllByRole(role, restOpts);
        const match = elements.find((elem) => elem === foundElement);
        expect(match).toBeDefined();
        return foundElement;
      } else {
      return original.getByRole(role, opts);
    };
  20. benlesh commented on Aug 1, 2024

    @benlesh

    I've profiled issues with this in our codebase locally, and 90% of the speed issues are coming from checking to see if the element's subtree is "accessible", meaning if it's potentially visible to the user. The problem is it recursively checks every element, and worse, runs a getComputedStyle on it to see if its visible.

    There's a new API for that now checkVisibility, but that's probably not going to help much in node.

    It's probably prudent to add an escape hatch here to skip this check. something like getAllByRole('option', { strictVisibilityCheck: false })

    Honestly, I think even having this check to begin with is a bit pedantic given the nature of these tests. If it was my project, I would default to not even checking that unless someone had it on getAllByRole('option', { strictVisibilityCheck: true }) or the like.

  21. MatanBobi commented on Aug 1, 2024

    @MatanBobi
    Member

    Thanks @benlesh for the investigation.
    This is already an option as stated by @eps1lon in this comment by using getByRole('option', { hidden: true }), which will skip the isInaccessible checks. This can also be turned off by default by calling configure({defaultHidden: true}), more info in our docs.

  22. hwride commented on Aug 1, 2024

    @hwride

    Last I checked hidden: true was not a particularly great improvement for performance of getByRole. So even if it does skip the accessibility checks, it doesn't seem to be the whole story/solution.

    For us getByText or getting by a specific role attribute was a better trade off. Yeah you might have to tweak a selector without implicit role finding from time to time, but in reality those changes are very infrequent and small compared to the rest of time spent on test writing, and well worth the performance benefit.

  23. benlesh commented on Aug 1, 2024

    @benlesh

    For record keeping, I did submit a PR: #1331

    I'll explore other options for our needs. These tests are honestly slower than just running Playwright or Cypress tests at this point.

  24. benlesh commented on Aug 1, 2024

    @benlesh

    @hwride you might want to check again. hidden: true improved performance of getAllByRole at least 10x in my problem test.

  25. eps1lon commented on Aug 2, 2024

    @eps1lon
    Member

    I'll explore other options for our needs. These tests are honestly slower than just running Playwright or Cypress tests at this point.

    That is what we recommend for high confidence testing. For lower confidence testing, disabling the inaccessible check via hidden: true is recommended for local testing and then enabling this check in CI.

    There's a new API for that now checkVisibility, but that's probably not going to help much in node.

    Keep in mind that the slowness is mostly not coming from the implementation but the environment. Switching to checkVisibility will likely have no effect in Node.js since implementations of the DOM in Node.js will need to run style calculation anyway. This may improve performance in Browsers though. I left an issue in eps1lon/dom-accessibility-api#1060 for anybody to explore.

    Node.js implementations of the DOM (e.g. JSDOM) are just slower than browsers and less feature complete. Both of these statements apply especially to CSS in Node.js which is why the library is perceived to be slow.

  26. Malgalad commented on Oct 22, 2025

    @Malgalad

    @benlesh problem with hidden: true is that it does nothing for description or name options. If we have name, the code calls computeAccessibleName which eventually calls isHidden which gets computed styles anyway and causes the slowdown. So while simple *ByRole(role, { hidden: true }) gets speed up, using *ByRole(role, { description, hidden: true }) or *ByRole(role, { name, hidden: true }) is as slow as without hidden.

    CPU profiling graph depicting queryAllByRole function taking 180ms
  27. github-actions commented on Oct 22, 2025

    @github-actions

    Uh oh! @Malgalad, the image you shared is missing helpful alt text. Check #820 (comment).

    Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

    Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

  28. github-actions commented on Oct 22, 2025

    @github-actions

    Uh oh! @Malgalad, the image you shared is missing helpful alt text. Check #820 (comment).

    Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

    Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

  29. foxaltus commented on Jan 14, 2026

    @foxaltus

    @benlesh problem with hidden: true is that it does nothing for description or name options. If we have name, the code calls computeAccessibleName which eventually calls isHidden which gets computed styles anyway and causes the slowdown. So while simple *ByRole(role, { hidden: true }) gets speed up, using *ByRole(role, { description, hidden: true }) or *ByRole(role, { name, hidden: true }) is as slow as without hidden.

    hey @Malgalad that's good insights and it explains why my *ByRole + name requests are still as slow as ever...

    All hopes are on eps1lon/dom-accessibility-api#1011 then. I've tried running testing-library with their latest release 0.7.1 (using pnpm overrides) as it is supposed to include the first performance enhancement, but it didn't make any difference.

  30. foxaltus commented on Jan 14, 2026

    @foxaltus

    Going down the rabbit hole... so *ByRole + name queries are slow because getComputedStyles is still getting called under the hood.

    In happy-dom, which I used, the method is notably slow, as reported in capricorn86/happy-dom#599. One of the workarounds given is:

    Another solution is to override getComputedStyle() with window.getComputedStyle = () => new CSSStyleDeclaration();

    Although this did not work as such (Illegal constructor error), adding the following to my tests did work and cut the execution time by a factor of 10!

    const dummyStyle = document.createElement('span').style;
    window.getComputedStyle = () => dummyStyle;

    It's not pretty, but that would be a fair workaround for a slow test (mine would take 10 secs... which got reduced to 1sec).

  31. scttcper commented on Sep 17, 2026

    @scttcper

    latest jsdom 30.1 has a bunch of fixes around performance in this area and i've opened #1391 to speed it up slightly more across all dom implementations.

    css visibility still probably pretty slow to check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationAn update needs to be made to documentation

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions