causes waitForElement to throw an error if callback is not defined. changes tests to account for new behavior. removes the documentation around the default callback. adds MutationObserver documentation for waitForDomChange

Alex · Alex · commit f972da806f16 · 2018-10-10T17:44:52.000-05:00
diff --git a/README.md b/README.md
@@ -488,7 +488,7 @@ intervals.
 
 ```typescript
 function waitForElement<T>(
-  callback?: () => T | null | undefined,
+  callback: () => T,
   options?: {
     container?: HTMLElement
     timeout?: number
@@ -531,9 +531,6 @@ const [usernameElement, passwordElement] = waitForElement(
 
 Using [`MutationObserver`](https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver) is more efficient than polling the DOM at regular intervals with `wait`. This library sets up a [`'mutationobserver-shim'`](https://github.com/megawac/MutationObserver.js) on the global `window` object for cross-platform compatibility with older browsers and the [`jsdom`](https://github.com/jsdom/jsdom/issues/639) that is usually used in Node-based tests.
 
-The default `callback` is a no-op function (used like `await waitForElement()`). This can
-be helpful if you only need to wait for the next DOM change (see [`mutationObserverOptions`](#mutationobserveroptions) to learn which changes are detected).
-
 The default `container` is the global `document`. Make sure the elements you wait for will be attached to it, or set a different `container`.
 
 The default `timeout` is `4500ms` which will keep you under
@@ -589,6 +586,16 @@ container.setAttribute('data-cool', 'false')
 */
 ```
 
+Using [`MutationObserver`](https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver) is more efficient than polling the DOM at regular intervals with `wait`. This library sets up a [`'mutationobserver-shim'`](https://github.com/megawac/MutationObserver.js) on the global `window` object for cross-platform compatibility with older browsers and the [`jsdom`](https://github.com/jsdom/jsdom/issues/639) that is usually used in Node-based tests.
+
+The default `container` is the global `document`. Make sure the elements you wait for will be attached to it, or set a different `container`.
+
+The default `timeout` is `4500ms` which will keep you under
+[Jest's default timeout of `5000ms`](https://facebook.github.io/jest/docs/en/jest-object.html#jestsettimeouttimeout).
+
+<a name="mutationobserveroptions"></a>The default `mutationObserverOptions` is `{subtree: true, childList: true, attributes: true, characterData: true}` which will detect
+additions and removals of child elements (including text nodes) in the `container` and any of its descendants. It will also detect attribute changes.
+
 ### `fireEvent`
 
 ```typescript
diff --git a/src/__tests__/__snapshots__/wait-for-element.js.snap b/src/__tests__/__snapshots__/wait-for-element.js.snap
@@ -79,15 +79,3 @@ exports[`it waits for the callback to return a value and only reacts to DOM muta
   />
 </div>
 `;
-
-exports[`it waits for the next DOM mutation with default callback 1`] = `
-<body>
-  <div />
-</body>
-`;
-
-exports[`it waits for the next DOM mutation with default callback 2`] = `
-<body>
-  <div />
-</body>
-`;
diff --git a/src/__tests__/wait-for-element.js b/src/__tests__/wait-for-element.js
@@ -3,14 +3,12 @@ import {waitForElement, wait} from '../'
 import 'jest-dom/extend-expect'
 import {render} from './helpers/test-utils'
 
-async function skipSomeTime(delayMs) {
-  await new Promise(resolve => setTimeout(resolve, delayMs))
-}
+const skipSomeTime = delayMs =>
+  new Promise(resolve => setTimeout(resolve, delayMs))
 
-async function skipSomeTimeForMutationObserver(delayMs = 50) {
-  // Using `setTimeout` >30ms instead of `wait` here because `mutationobserver-shim` uses `setTimeout` ~30ms.
-  await skipSomeTime(delayMs, 50)
-}
+// Using `setTimeout` >30ms instead of `wait` here because `mutationobserver-shim` uses `setTimeout` ~30ms.
+const skipSomeTimeForMutationObserver = (delayMs = 50) =>
+  skipSomeTime(delayMs, 50)
 
 test('it waits for the callback to return a value and only reacts to DOM mutations', async () => {
   const {container, getByTestId} = render(
@@ -66,10 +64,7 @@ test('it waits for the callback to return a value and only reacts to DOM mutatio
   const successHandler = jest.fn().mockName('successHandler')
   const errorHandler = jest.fn().mockName('errorHandler')
 
-  const promise = waitForElement(callback, {container}).then(
-    successHandler,
-    errorHandler,
-  )
+  waitForElement(callback, {container}).then(successHandler, errorHandler)
 
   // One synchronous `callback` call is expected.
   expect(callback).toHaveBeenCalledTimes(1)
@@ -96,37 +91,6 @@ test('it waits for the callback to return a value and only reacts to DOM mutatio
   expect(errorHandler).toHaveBeenCalledTimes(0)
   expect(container).toMatchSnapshot()
   expect(testEl.parentNode).toBe(container)
-
-  return promise
-})
-
-test('it waits for the next DOM mutation with default callback', async () => {
-  const successHandler = jest.fn().mockName('successHandler')
-  const errorHandler = jest.fn().mockName('errorHandler')
-
-  const promise = waitForElement().then(successHandler, errorHandler)
-
-  // Promise callbacks are always asynchronous.
-  expect(successHandler).toHaveBeenCalledTimes(0)
-  expect(errorHandler).toHaveBeenCalledTimes(0)
-
-  await skipSomeTimeForMutationObserver()
-
-  // No more expected calls without DOM mutations.
-  expect(successHandler).toHaveBeenCalledTimes(0)
-  expect(errorHandler).toHaveBeenCalledTimes(0)
-
-  document.body.appendChild(document.createElement('div'))
-  expect(document.body).toMatchSnapshot()
-
-  await skipSomeTimeForMutationObserver()
-
-  expect(successHandler).toHaveBeenCalledTimes(1)
-  expect(successHandler).toHaveBeenCalledWith(undefined)
-  expect(errorHandler).toHaveBeenCalledTimes(0)
-  expect(document.body).toMatchSnapshot()
-
-  return promise
 })
 
 test('it waits characterData mutation', async () => {
@@ -138,10 +102,7 @@ test('it waits characterData mutation', async () => {
   const successHandler = jest.fn().mockName('successHandler')
   const errorHandler = jest.fn().mockName('errorHandler')
 
-  const promise = waitForElement(callback, {container}).then(
-    successHandler,
-    errorHandler,
-  )
+  waitForElement(callback, {container}).then(successHandler, errorHandler)
 
   // Promise callbacks are always asynchronous.
   expect(successHandler).toHaveBeenCalledTimes(0)
@@ -162,8 +123,6 @@ test('it waits characterData mutation', async () => {
   expect(errorHandler).toHaveBeenCalledTimes(0)
   expect(callback).toHaveBeenCalledTimes(2)
   expect(container).toMatchSnapshot()
-
-  return promise
 })
 
 test('it waits for the attributes mutation', async () => {
@@ -175,7 +134,7 @@ test('it waits for the attributes mutation', async () => {
   const successHandler = jest.fn().mockName('successHandler')
   const errorHandler = jest.fn().mockName('errorHandler')
 
-  const promise = waitForElement(callback, {
+  waitForElement(callback, {
     container,
   }).then(successHandler, errorHandler)
 
@@ -196,8 +155,6 @@ test('it waits for the attributes mutation', async () => {
   expect(successHandler).toHaveBeenCalledTimes(1)
   expect(successHandler).toHaveBeenCalledWith('PASSED')
   expect(errorHandler).toHaveBeenCalledTimes(0)
-
-  return promise
 })
 
 test('it throws if timeout is exceeded', async () => {
@@ -207,7 +164,7 @@ test('it throws if timeout is exceeded', async () => {
   const successHandler = jest.fn().mockName('successHandler')
   const errorHandler = jest.fn().mockName('errorHandler')
 
-  const promise = waitForElement(callback, {
+  waitForElement(callback, {
     container,
     timeout: 70,
     mutationObserverOptions: {attributes: true},
@@ -232,8 +189,6 @@ test('it throws if timeout is exceeded', async () => {
   expect(errorHandler).toHaveBeenCalledTimes(1)
   expect(errorHandler.mock.calls[0]).toMatchSnapshot()
   expect(container).toMatchSnapshot()
-
-  return promise
 })
 
 test('it throws the same error that the callback has thrown if timeout is exceeded', async () => {
@@ -243,7 +198,7 @@ test('it throws the same error that the callback has thrown if timeout is exceed
   const successHandler = jest.fn().mockName('successHandler')
   const errorHandler = jest.fn().mockName('errorHandler')
 
-  const promise = waitForElement(callback, {
+  waitForElement(callback, {
     container,
     timeout: 70,
     mutationObserverOptions: {attributes: true},
@@ -268,8 +223,6 @@ test('it throws the same error that the callback has thrown if timeout is exceed
   expect(errorHandler).toHaveBeenCalledTimes(1)
   expect(errorHandler.mock.calls[0]).toMatchSnapshot()
   expect(container).toMatchSnapshot()
-
-  return promise
 })
 
 test('it returns immediately if the callback returns the value before any mutations', async () => {
@@ -283,7 +236,7 @@ test('it returns immediately if the callback returns the value before any mutati
   const successHandler = jest.fn().mockName('successHandler')
   const errorHandler = jest.fn().mockName('errorHandler')
 
-  const promise = waitForElement(callback, {
+  waitForElement(callback, {
     container,
     timeout: 70,
     mutationObserverOptions: {attributes: true},
@@ -309,8 +262,6 @@ test('it returns immediately if the callback returns the value before any mutati
   expect(errorHandler).toHaveBeenCalledTimes(0)
 
   expect(container).toMatchSnapshot()
-
-  return promise
 })
 
 test('does not get into infinite setTimeout loop after MutationObserver notification', async () => {
@@ -356,4 +307,47 @@ test('does not get into infinite setTimeout loop after MutationObserver notifica
   // Expect no more setTimeout calls
   jest.advanceTimersByTime(100)
   expect(setTimeout).toHaveBeenCalledTimes(3)
+  jest.useRealTimers()
+})
+
+test('works if a container is not defined', async () => {
+  render(``)
+  const el = document.createElement('p')
+  document.body.appendChild(el)
+  const callback = jest
+    .fn(() => el.textContent === 'I changed!')
+    .mockName('callback')
+  const successHandler = jest.fn().mockName('successHandler')
+  const errorHandler = jest.fn().mockName('errorHandler')
+
+  waitForElement(callback).then(successHandler, errorHandler)
+
+  await skipSomeTimeForMutationObserver()
+
+  expect(callback).toHaveBeenCalledTimes(1)
+  expect(successHandler).toHaveBeenCalledTimes(0)
+  expect(errorHandler).toHaveBeenCalledTimes(0)
+
+  el.innerHTML = 'I changed!'
+  await skipSomeTimeForMutationObserver()
+
+  expect(callback).toHaveBeenCalledTimes(2)
+  expect(successHandler).toHaveBeenCalledTimes(1)
+  expect(errorHandler).toHaveBeenCalledTimes(0)
+
+  document.getElementsByTagName('html')[0].innerHTML = '' // cleans the document
+})
+
+test('throws an error if callback is not a function', async () => {
+  const successHandler = jest.fn().mockName('successHandler')
+  const errorHandler = jest.fn().mockName('errorHandler')
+
+  waitForElement().then(successHandler, errorHandler)
+
+  await skipSomeTimeForMutationObserver()
+
+  expect(errorHandler).toHaveBeenLastCalledWith(
+    'waitForElement requires a callback as the first parameter',
+  )
+  expect(successHandler).toHaveBeenCalledTimes(0)
 })
diff --git a/src/wait-for-element.js b/src/wait-for-element.js
@@ -1,7 +1,7 @@
 import 'mutationobserver-shim'
 
 function waitForElement(
-  callback = undefined,
+  callback,
   {
     container = document,
     timeout = 4500,
@@ -14,8 +14,13 @@ function waitForElement(
   } = {},
 ) {
   return new Promise((resolve, reject) => {
-    // Disabling eslint prefer-const below: either prefer-const or no-use-before-define triggers.
-    let lastError, observer, timer // eslint-disable-line prefer-const
+    if (typeof callback !== 'function') {
+      reject('waitForElement requires a callback as the first parameter')
+    }
+    let lastError
+    const timer = setTimeout(onTimeout, timeout)
+    const observer = new window.MutationObserver(onMutation)
+    observer.observe(container, mutationObserverOptions)
     function onDone(error, result) {
       clearTimeout(timer)
       setImmediate(() => observer.disconnect())
@@ -26,10 +31,6 @@ function waitForElement(
       }
     }
     function onMutation() {
-      if (callback === undefined) {
-        onDone(null, undefined)
-        return
-      }
       try {
         const result = callback()
         if (result) {
@@ -45,12 +46,7 @@ function waitForElement(
     function onTimeout() {
       onDone(lastError || new Error('Timed out in waitForElement.'), null)
     }
-    timer = setTimeout(onTimeout, timeout)
-    observer = new window.MutationObserver(onMutation)
-    observer.observe(container, mutationObserverOptions)
-    if (callback !== undefined) {
-      onMutation()
-    }
+    onMutation()
   })
 }
 
diff --git a/typings/wait-for-element.d.ts b/typings/wait-for-element.d.ts
@@ -1,8 +1,8 @@
 export function waitForElement<T>(
-  callback?: () => T,
+  callback: () => T,
   options?: {
     container?: HTMLElement
     timeout?: number
     mutationObserverOptions?: MutationObserverInit
   },
-): Promise<T | undefined>
+): Promise<T>
