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

Closes #376
Closes #416

BREAKING CHANGE: `waitForElement` is deprecated in favor of `find*` queries or `wait`.
BREAKING CHANGE: `waitForDomChange` is deprecated in favor of `wait`
BREAKING CHANGE: default timeout for async utilities is now 1000ms rather than 4500ms. This can be configured: https://testing-library.com/docs/dom-testing-library/api-configuration

Author: Kent C. Dodds

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",
@@ -61,7 +60,8 @@
     "rules": {
       "import/prefer-default-export": "off",
       "import/no-unassigned-import": "off",
-      "import/no-useless-path-segments": "off"
+      "import/no-useless-path-segments": "off",
+      "no-console": "off"
     }
   },
   "eslintIgnore": [

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__/pretty-dom.js

@@ -77,5 +77,3 @@ describe('prettyDOM fails with first parameter without outerHTML field', () => {
     )
   })
 })
-
-/* eslint no-console:0 */

src/__tests__/role-helpers.js

@@ -184,5 +184,3 @@ test.each([
 
   expect(isInaccessible(container.querySelector('button'))).toBe(expected)
 })
-
-/* eslint no-console:0 */

src/__tests__/screen.js

@@ -61,5 +61,3 @@ test('exposes debug method', () => {
   `)
   console.log.mockClear()
 })
-
-/* eslint no-console:0 */

src/__tests__/wait-for-dom-change.js

@@ -10,6 +10,7 @@ beforeEach(() => {
   jest.useRealTimers()
   jest.resetModules()
   waitForDomChange = importModule()
+  console.warn.mockClear()
 })
 
 test('waits for the dom to change in the document', async () => {
@@ -34,6 +35,13 @@ test('waits for the dom to change in the document', async () => {
       },
     ]
   `)
+  expect(console.warn.mock.calls).toMatchInlineSnapshot(`
+Array [
+  Array [
+    "\`waitForDomChange\` has been deprecated. Use \`wait\` instead: https://testing-library.com/docs/dom-testing-library/api-async#wait.",
+  ],
+]
+`)
 })
 
 test('waits for the dom to change in a specified container', async () => {

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

@@ -49,23 +49,23 @@ 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"`,
   )
 })
 
 test('requires an element to exist first', () => {
   return expect(
     waitForElementToBeRemoved(() => null),
   ).rejects.toThrowErrorMatchingInlineSnapshot(
-    `"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."`,
+    `"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal."`,
   )
 })
 
 test('requires an unempty array of elements to exist first', () => {
   return expect(
     waitForElementToBeRemoved(() => []),
   ).rejects.toThrowErrorMatchingInlineSnapshot(
-    `"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."`,
+    `"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal."`,
   )
 })
 
@@ -117,3 +117,18 @@ test("doesn't change jest's timers value when importing the module", () => {
 
   expect(window.setTimeout._isMockFunction).toEqual(true)
 })
+
+test('rethrows non-testing-lib errors', () => {
+  let throwIt = false
+  const div = document.createElement('div')
+  const error = new Error('my own error')
+  return expect(
+    waitForElementToBeRemoved(() => {
+      if (throwIt) {
+        throw error
+      }
+      throwIt = true
+      return div
+    }),
+  ).rejects.toBe(error)
+})

src/__tests__/wait-for-element.js

@@ -10,6 +10,7 @@ beforeEach(() => {
   jest.useRealTimers()
   jest.resetModules()
   waitForElement = importModule()
+  console.warn.mockClear()
 })
 
 test('waits for element to appear in the document', async () => {
@@ -18,6 +19,13 @@ test('waits for element to appear in the document', async () => {
   setTimeout(() => rerender('<div data-testid="div" />'))
   const element = await promise
   expect(element).toBeInTheDocument()
+  expect(console.warn.mock.calls).toMatchInlineSnapshot(`
+    Array [
+      Array [
+        "\`waitForElement\` has been deprecated. Use a \`find*\` query (preferred: https://testing-library.com/docs/dom-testing-library/api-queries#findby) or use \`wait\` instead (it's the same API, so you can find/replace): https://testing-library.com/docs/dom-testing-library/api-async#wait",
+      ],
+    ]
+  `)
 })
 
 test('waits for element to appear in a specified container', async () => {

src/__tests__/wait.js

@@ -17,3 +17,14 @@ test('wait defaults to a noop callback', async () => {
   await wait()
   expect(handler).toHaveBeenCalledTimes(1)
 })
+
+test('can timeout after the given timeout time', async () => {
+  const error = new Error('throws every time')
+  const result = await wait(
+    () => {
+      throw error
+    },
+    {timeout: 8, interval: 5},
+  ).catch(e => e)
+  expect(result).toBe(error)
+})

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
@@ -19,9 +19,11 @@ let config = {
 
   // called when getBy* queries fail. (message, container) => Error
   getElementError(message, container) {
-    return new Error(
+    const error = new Error(
       [message, prettyDOM(container)].filter(Boolean).join('\n\n'),
     )
+    error.name = 'TestingLibraryElementError'
+    return error
   },
 }
 

src/pretty-dom.js

@@ -18,7 +18,7 @@ const inNode = () =>
 const getMaxLength = dom =>
   inCypress(dom)
     ? 0
-    : typeof process !== 'undefined' && process.env.DEBUG_PRINT_LIMIT || 7000
+    : (typeof process !== 'undefined' && process.env.DEBUG_PRINT_LIMIT) || 7000
 
 const {DOMElement, DOMCollection} = prettyFormat.plugins
 
@@ -64,5 +64,3 @@ function prettyDOM(dom, maxLength, options) {
 const logDOM = (...args) => console.log(prettyDOM(...args))
 
 export {prettyDOM, logDOM}
-
-/* eslint no-console:0 */

src/role-helpers.js

@@ -176,5 +176,3 @@ export {
   prettyRoles,
   isInaccessible,
 }
-
-/* eslint no-console:0 */

src/wait-for-dom-change.js

@@ -8,6 +8,11 @@ import {
 } from './helpers'
 import {getConfig} from './config'
 
+let hasWarned = false
+
+// 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,
@@ -18,6 +23,12 @@ function waitForDomChange({
     characterData: true,
   },
 } = {}) {
+  if (!hasWarned) {
+    hasWarned = true
+    console.warn(
+      `\`waitForDomChange\` has been deprecated. Use \`wait\` instead: https://testing-library.com/docs/dom-testing-library/api-async#wait.`,
+    )
+  }
   return new Promise((resolve, reject) => {
     const timer = setTimeout(onTimeout, timeout)
     const observer = newMutationObserver(onMutation)