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

Closes #376

Author: kentcdodds

package.json

@@ -44,8 +44,7 @@
     "@sheerun/mutationobserver-shim": "^0.3.2",
     "@types/testing-library__dom": "^6.0.0",
     "aria-query": "3.0.0",
-    "pretty-format": "^24.9.0",
-    "wait-for-expect": "^3.0.0"
+    "pretty-format": "^24.9.0"
   },
   "devDependencies": {
     "@testing-library/jest-dom": "^4.1.0",

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/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"
+*/

src/wait.js

@@ -1,8 +1,79 @@
-import waitForExpect from 'wait-for-expect'
+import {
+  newMutationObserver,
+  getDocument,
+  setImmediate,
+  setTimeout,
+  clearTimeout,
+  runWithRealTimers,
+} from './helpers'
 import {getConfig} from './config'
 
-function wait(callback = () => {}, {timeout = getConfig().asyncUtilTimeout, interval = 50} = {}) {
-  return waitForExpect(callback, timeout, interval)
+function wait(
+  callback = () => {},
+  {
+    container = getDocument(),
+    timeout = getConfig().asyncUtilTimeout,
+    interval = 50,
+    mutationObserverOptions = {
+      subtree: true,
+      childList: true,
+      attributes: true,
+      characterData: true,
+    },
+  } = {},
+) {
+  if (interval < 1) interval = 1
+  const maxTries = Math.ceil(timeout / interval)
+  let tries = 0
+  return new Promise((resolve, reject) => {
+    let lastError, lastTimer
+    const overallTimeoutTimer = setTimeout(onTimeout, timeout)
+
+    const observer = newMutationObserver(checkCallback)
+    runWithRealTimers(() =>
+      observer.observe(container, mutationObserverOptions),
+    )
+
+    function onDone(error, result) {
+      clearTimeout(overallTimeoutTimer)
+      clearTimeout(lastTimer)
+      setImmediate(() => observer.disconnect())
+      if (error) {
+        reject(error)
+      } else {
+        resolve(result)
+      }
+    }
+
+    function checkCallback() {
+      try {
+        onDone(null, callback())
+        // If `callback` throws, wait for the next mutation or timeout.
+      } catch (error) {
+        // Save the callback error to reject the promise with it.
+        lastError = error
+      }
+    }
+
+    function onTimeout() {
+      onDone(lastError || new Error('Timed out in wait.'), null)
+    }
+
+    function startTimer() {
+      lastTimer = setTimeout(() => {
+        tries++
+        checkCallback()
+        if (tries > maxTries) {
+          onTimeout()
+          return
+        }
+        startTimer()
+      }, interval)
+    }
+
+    checkCallback()
+    startTimer()
+  })
 }
 
 function waitWrapper(...args) {