feat(wait): wait will now also run your callback on DOM changes

Closes #376

Author: kentcdodds

package.json

@@ -45,8 +45,7 @@
     "@types/testing-library__dom": "^6.12.1",
     "aria-query": "^4.0.2",
     "dom-accessibility-api": "^0.3.0",
-    "pretty-format": "^25.1.0",
-    "wait-for-expect": "^3.0.2"
+    "pretty-format": "^25.1.0"
   },
   "devDependencies": {
     "@testing-library/jest-dom": "^5.1.1",

src/__tests__/fake-timers.js

@@ -24,6 +24,7 @@ jest.useFakeTimers()
 jest.resetModules()
 
 const {
+  wait,
   waitForElement,
   waitForDomChange,
   waitForElementToBeRemoved,
@@ -42,6 +43,15 @@ test('waitForElementToBeRemoved: times out after 4500ms by default', () => {
   return promise
 })
 
+test('wait: can time out', async () => {
+  const promise = wait(() => {
+    // eslint-disable-next-line no-throw-literal
+    throw undefined
+  })
+  jest.advanceTimersByTime(4600)
+  await expect(promise).rejects.toThrow(/timed out/i)
+})
+
 test('waitForElement: can time out', async () => {
   const promise = waitForElement(() => {})
   jest.advanceTimersByTime(4600)
@@ -85,3 +95,45 @@ test('waitForDomChange: can specify our own timeout time', async () => {
   // timed out
   await expect(promise).rejects.toThrow(/timed out/i)
 })
+
+test('wait: ensures the interval is greater than 0', async () => {
+  // Arrange
+  const spy = jest.fn()
+  spy.mockImplementationOnce(() => {
+    throw new Error('first time does not work')
+  })
+  const promise = wait(spy, {interval: 0})
+  expect(spy).toHaveBeenCalledTimes(1)
+  spy.mockClear()
+
+  // Act
+  // this line will throw an error if wait does not make the interval 1 instead of 0
+  // which is why it does that!
+  jest.advanceTimersByTime(0)
+
+  // Assert
+  expect(spy).toHaveBeenCalledTimes(0)
+  spy.mockImplementationOnce(() => 'second time does work')
+
+  // Act
+  jest.advanceTimersByTime(1)
+  await promise
+
+  // Assert
+  expect(spy).toHaveBeenCalledTimes(1)
+})
+
+test('wait: times out if it runs out of attempts', () => {
+  const spy = jest.fn(() => {
+    throw new Error('example error')
+  })
+  // there's a bug with this rule here...
+  // eslint-disable-next-line jest/valid-expect
+  const promise = expect(
+    wait(spy, {interval: 1, timeout: 3}),
+  ).rejects.toThrowErrorMatchingInlineSnapshot(`"example error"`)
+  jest.advanceTimersByTime(1)
+  jest.advanceTimersByTime(1)
+  jest.advanceTimersByTime(1)
+  return promise
+})

src/__tests__/wait-for-element-to-be-removed.js

@@ -49,7 +49,7 @@ test('requires a function as the first parameter', () => {
   return expect(
     waitForElementToBeRemoved(),
   ).rejects.toThrowErrorMatchingInlineSnapshot(
-    `"waitForElementToBeRemoved requires a function as the first parameter"`,
+    `"waitForElementToBeRemoved requires a callback as the first parameter"`,
   )
 })
 

src/config.js

@@ -5,7 +5,7 @@ import {prettyDOM} from './pretty-dom'
 // './queries' are query functions.
 let config = {
   testIdAttribute: 'data-testid',
-  asyncUtilTimeout: 4500,
+  asyncUtilTimeout: 1000,
   // this is to support React's async `act` function.
   // forcing react-testing-library to wrap all async functions would've been
   // a total nightmare (consider wrapping every findBy* query and then also

src/wait-for-dom-change.js

@@ -8,6 +8,9 @@ import {
 } from './helpers'
 import {getConfig} from './config'
 
+// deprecated... TODO: remove this method. People should use wait instead
+// the reasoning is that waiting for just any DOM change is an implementation
+// detail. People should be waiting for a specific thing to change.
 function waitForDomChange({
   container = getDocument(),
   timeout = getConfig().asyncUtilTimeout,

src/wait-for-element-to-be-removed.js

@@ -1,85 +1,48 @@
-import {
-  getDocument,
-  newMutationObserver,
-  setImmediate,
-  setTimeout,
-  clearTimeout,
-  runWithRealTimers,
-} from './helpers'
-import {getConfig} from './config'
+import {wait} from './wait'
 
-function waitForElementToBeRemoved(
-  callback,
-  {
-    container = getDocument(),
-    timeout = getConfig().asyncUtilTimeout,
-    mutationObserverOptions = {
-      subtree: true,
-      childList: true,
-      attributes: true,
-      characterData: true,
-    },
-  } = {},
-) {
-  return new Promise((resolve, reject) => {
-    if (typeof callback !== 'function') {
-      reject(
-        new Error(
-          'waitForElementToBeRemoved requires a function as the first parameter',
-        ),
-      )
-    }
-    const timer = setTimeout(onTimeout, timeout)
-    const observer = newMutationObserver(onMutation)
+const isRemoved = result => !result || (Array.isArray(result) && !result.length)
+
+async function waitForElementToBeRemoved(callback, options) {
+  if (!callback) {
+    return Promise.reject(
+      new Error(
+        'waitForElementToBeRemoved requires a callback as the first parameter',
+      ),
+    )
+  }
+
+  // Check if the element is not present synchronously,
+  // As the name implies, waitForElementToBeRemoved should check `present` --> `removed`
+  if (isRemoved(callback())) {
+    throw new Error(
+      'The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist before waiting for removal.',
+    )
+  }
 
-    // Check if the element is not present synchronously,
-    // As the name waitForElementToBeRemoved should check `present` --> `removed`
+  return wait(() => {
+    let result
     try {
-      const result = callback()
-      if (!result || (Array.isArray(result) && !result.length)) {
-        onDone(
-          new Error(
-            'The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist before waiting for removal.',
-          ),
-        )
-      } else {
-        // Only observe for mutations only if there is element while checking synchronously
-        runWithRealTimers(() =>
-          observer.observe(container, mutationObserverOptions),
-        )
-      }
+      result = callback()
     } catch (error) {
-      onDone(error)
-    }
-
-    function onDone(error, result) {
-      clearTimeout(timer)
-      setImmediate(() => observer.disconnect())
-      if (error) {
-        reject(error)
-      } else {
-        resolve(result)
-      }
-    }
-    function onMutation() {
-      try {
-        const result = callback()
-        if (!result || (Array.isArray(result) && !result.length)) {
-          onDone(null, true)
-        }
-        // If `callback` returns truthy value, wait for the next mutation or timeout.
-      } catch (error) {
-        onDone(null, true)
+      if (error.message && error.message.startsWith('Unable to find')) {
+        // All of our get* queries throw an error that starts with "Unable to find"
+        // when it fails to find an element.
+        // TODO: make the queries throw a special kind of error
+        // so we can be more explicit about the check.
+        return true
       }
+      throw error
     }
-    function onTimeout() {
-      onDone(new Error('Timed out in waitForElementToBeRemoved.'), null)
+    if (!isRemoved(result)) {
+      throw new Error('Timed out in waitForElementToBeRemoved.')
     }
-  })
+    return true
+  }, options)
 }
 
-function waitForElementToBeRemovedWrapper(...args) {
-  return getConfig().asyncWrapper(() => waitForElementToBeRemoved(...args))
-}
+export {waitForElementToBeRemoved}
 
-export {waitForElementToBeRemovedWrapper as waitForElementToBeRemoved}
+/*
+eslint
+  require-await: "off"
+*/

src/wait-for-element.js

@@ -1,71 +1,21 @@
-import {
-  newMutationObserver,
-  getDocument,
-  setImmediate,
-  setTimeout,
-  clearTimeout,
-  runWithRealTimers,
-} from './helpers'
-import {getConfig} from './config'
+import {wait} from './wait'
 
-function waitForElement(
-  callback,
-  {
-    container = getDocument(),
-    timeout = getConfig().asyncUtilTimeout,
-    mutationObserverOptions = {
-      subtree: true,
-      childList: true,
-      attributes: true,
-      characterData: true,
-    },
-  } = {},
-) {
-  return new Promise((resolve, reject) => {
-    if (typeof callback !== 'function') {
-      reject(
-        new Error('waitForElement requires a callback as the first parameter'),
-      )
-      return
+async function waitForElement(callback, options) {
+  if (!callback) {
+    throw new Error('waitForElement requires a callback as the first parameter')
+  }
+  return wait(() => {
+    const result = callback()
+    if (!result) {
+      throw new Error('Timed out in waitForElement.')
     }
-    let lastError
-    const timer = setTimeout(onTimeout, timeout)
-
-    const observer = newMutationObserver(onMutation)
-    runWithRealTimers(() =>
-      observer.observe(container, mutationObserverOptions),
-    )
-    function onDone(error, result) {
-      clearTimeout(timer)
-      setImmediate(() => observer.disconnect())
-      if (error) {
-        reject(error)
-      } else {
-        resolve(result)
-      }
-    }
-    function onMutation() {
-      try {
-        const result = callback()
-        if (result) {
-          onDone(null, result)
-        }
-        // If `callback` returns falsy value, wait for the next mutation or timeout.
-      } catch (error) {
-        // Save the callback error to reject the promise with it.
-        lastError = error
-        // If `callback` throws an error, wait for the next mutation or timeout.
-      }
-    }
-    function onTimeout() {
-      onDone(lastError || new Error('Timed out in waitForElement.'), null)
-    }
-    onMutation()
-  })
+    return result
+  }, options)
 }
 
-function waitForElementWrapper(...args) {
-  return getConfig().asyncWrapper(() => waitForElement(...args))
-}
+export {waitForElement}
 
-export {waitForElementWrapper as waitForElement}
+/*
+eslint
+  require-await: "off"
+*/