Skip to content

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Oct 24, 2019

What:

Improves perf >20% reduction if ByRole returns multiple elements. The more elements the bigger the reduction

Why:

Perf concerns were raised about isInaccessible

How:

An element being inaccessible does not imply every descendant is inaccessible because elements can re-appear by setting visibility: visible in a visibility: hidden subtree. However, they can't override hidden or aria-hidden=true or display: none. So we cache isSubtreeInaccessible return values for each element. For collections of elements (e.g. option or listitem) this reduces getComputedStyle calls from O(n * depth(n)) to O(n) + O(depth(n)) where n is the number of elements of the given role. It enables a very fast best case for collections where the containing element (e.g. the listbox) is hidden. We only have to compute the style once for subsequent listitems and can exit early once we hit the listbox and the cache subtreeIsInaccessible returns true.

To avoid passing the computed style too much around I removed getComputedStyle().visibility compatibility for jsdom < 15.2. Once jest 25 is released this will be fixed for jest users anyway and CSS was one of the lesser concerns of other collaborators.

We use a WeakMap for the cache. Those are supported in IE 11: https://caniuse.com/#search=Weakmap

Checklist:

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 24, 2019

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ca81c9d:

Sandbox Source
relaxed-sammet-p9hlz Configuration

Still has +-20% in median runtime
Useful to inject more performant implementations
@eps1lon eps1lon force-pushed the feat/byRole/collection-perf branch from 4499bf2 to 2b954e2 Compare October 28, 2019 18:48
@eps1lon eps1lon added the enhancement New feature or request label Oct 28, 2019
@kentcdodds
Copy link
Member

An element being inaccessible does not imply every descendant is inaccessible because elements can re-appear by setting visibility: visible in a visibility: hidden subtree.

Ah rats. What a shame! Haha. Thank you so much for working on this @eps1lon! I appreciate your help on this.

@kentcdodds kentcdodds merged commit ea2fc64 into testing-library:master Oct 28, 2019
@kentcdodds
Copy link
Member

🎉 This PR is included in version 6.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@eps1lon eps1lon deleted the feat/byRole/collection-perf branch November 27, 2019 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants