feat(ByRole): Improve perf if multiple elements are found (#395)

* Add benchmmark

Still has +-20% in median runtime

* Extract isSubtreeInaccessible

Useful to inject more performant implementations

* Cache isSubtreeInaccessible calls

* Remove benchmark

Author: eps1lon

src/__tests__/role-helpers.js

@@ -171,7 +171,7 @@ test.each([
   ['<div style="visibility: visible" />', false],
   ['<div hidden />', true],
   ['<div style="display: none;"/>', true],
-  ['<div style="visibility: hidden;"/>', true],
+  ['<div style="visibility: hidden;"/>', false], // bug in jsdom < 15.2
   ['<div aria-hidden="true" />', true],
 ])('shouldExcludeFromA11yTree for %s returns %p', (html, expected) => {
   const {container} = render(html)

src/__tests__/role.js

@@ -115,18 +115,20 @@ There are no accessible roles. But there might be some inaccessible roles. If yo
 })
 
 test('by default excludes elements which have visibility hidden', () => {
-  const {getByRole} = render('<div style="visibility: hidden;"><ul /></div>')
+  // works in jsdom < 15.2 only when the actual element in question has this
+  // css property. only jsdom@^15.2 implements inheritance for `visibility`
+  const {getByRole} = render('<div><ul style="visibility: hidden;" /></div>')
 
   expect(() => getByRole('list')).toThrowErrorMatchingInlineSnapshot(`
 "Unable to find an accessible element with the role "list"
 
 There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the \`hidden\` option to \`true\`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole
 
 <div>
-  <div
-    style="visibility: hidden;"
-  >
-    <ul />
+  <div>
+    <ul
+      style="visibility: hidden;"
+    />
   </div>
 </div>"
 `)

src/queries/role.js

@@ -2,6 +2,7 @@ import {
   getImplicitAriaRoles,
   prettyRoles,
   isInaccessible,
+  isSubtreeInaccessible,
 } from '../role-helpers'
 import {buildQueries, fuzzyMatches, makeNormalizer, matches} from './all-utils'
 
@@ -13,6 +14,15 @@ function queryAllByRole(
   const matcher = exact ? matches : fuzzyMatches
   const matchNormalizer = makeNormalizer({collapseWhitespace, trim, normalizer})
 
+  const subtreeIsInaccessibleCache = new WeakMap()
+  function cachedIsSubtreeInaccessible(element) {
+    if (!subtreeIsInaccessibleCache.has(element)) {
+      subtreeIsInaccessibleCache.set(element, isSubtreeInaccessible(element))
+    }
+
+    return subtreeIsInaccessibleCache.get(element)
+  }
+
   return Array.from(container.querySelectorAll('*'))
     .filter(node => {
       const isRoleSpecifiedExplicitly = node.hasAttribute('role')
@@ -28,7 +38,11 @@ function queryAllByRole(
       )
     })
     .filter(element => {
-      return hidden === false ? isInaccessible(element) === false : true
+      return hidden === false
+        ? isInaccessible(element, {
+            isSubtreeInaccessible: cachedIsSubtreeInaccessible,
+          }) === false
+        : true
     })
 }
 

src/role-helpers.js

@@ -3,6 +3,27 @@ import {prettyDOM} from './pretty-dom'
 
 const elementRoleList = buildElementRoleList(elementRoles)
 
+/**
+ * @param {Element} element -
+ * @returns {boolean} - `true` if `element` and its subtree are inaccessible
+ */
+function isSubtreeInaccessible(element) {
+  if (element.hidden === true) {
+    return true
+  }
+
+  if (element.getAttribute('aria-hidden') === 'true') {
+    return true
+  }
+
+  const window = element.ownerDocument.defaultView
+  if (window.getComputedStyle(element).display === 'none') {
+    return true
+  }
+
+  return false
+}
+
 /**
  * Partial implementation https://www.w3.org/TR/wai-aria-1.2/#tree_exclusion
  * which should only be used for elements with a non-presentational role i.e.
@@ -12,47 +33,27 @@ const elementRoleList = buildElementRoleList(elementRoles)
  * Ignores "Child Presentational: True" characteristics
  *
  * @param {Element} element -
+ * @param {object} [options] -
+ * @param {function (element: Element): boolean} options.isSubtreeInaccessible -
+ * can be used to return cached results from previous isSubtreeInaccessible calls
  * @returns {boolean} true if excluded, otherwise false
  */
-function isInaccessible(element) {
+function isInaccessible(element, options = {}) {
+  const {
+    isSubtreeInaccessible: isSubtreeInaccessibleImpl = isSubtreeInaccessible,
+  } = options
   const window = element.ownerDocument.defaultView
-  const computedStyle = window.getComputedStyle(element)
   // since visibility is inherited we can exit early
-  if (computedStyle.visibility === 'hidden') {
+  if (window.getComputedStyle(element).visibility === 'hidden') {
     return true
   }
 
-  // Remove once https://github.com/jsdom/jsdom/issues/2616 is fixed
-  const supportsStyleInheritance = computedStyle.visibility !== ''
-  let visibility = computedStyle.visibility
-
   let currentElement = element
   while (currentElement) {
-    if (currentElement.hidden === true) {
+    if (isSubtreeInaccessibleImpl(currentElement)) {
       return true
     }
 
-    if (currentElement.getAttribute('aria-hidden') === 'true') {
-      return true
-    }
-
-    const currentComputedStyle = window.getComputedStyle(currentElement)
-
-    if (currentComputedStyle.display === 'none') {
-      return true
-    }
-
-    // this branch is temporary code until jsdom fixes a bug
-    // istanbul ignore else
-    if (supportsStyleInheritance === false) {
-      // we go bottom-up for an inheritable property so we can only set it
-      // if it wasn't set already i.e. the parent can't overwrite the child
-      if (visibility === '') visibility = currentComputedStyle.visibility
-      if (visibility === 'hidden') {
-        return true
-      }
-    }
-
     currentElement = currentElement.parentElement
   }
 
@@ -155,6 +156,13 @@ function prettyRoles(dom, {hidden}) {
 const logRoles = (dom, {hidden = false} = {}) =>
   console.log(prettyRoles(dom, {hidden}))
 
-export {getRoles, logRoles, getImplicitAriaRoles, prettyRoles, isInaccessible}
+export {
+  getRoles,
+  logRoles,
+  getImplicitAriaRoles,
+  isSubtreeInaccessible,
+  prettyRoles,
+  isInaccessible,
+}
 
 /* eslint no-console:0 */