From 2ceb1a09245d2f88f1d91b804a52ce4eed35750e Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Fri, 13 Dec 2019 11:09:11 -0700 Subject: [PATCH] feat(wait): wait will now also run your callback on DOM changes Closes #376 --- package.json | 3 +- src/__tests__/fake-timers.js | 52 ++++++++ .../wait-for-element-to-be-removed.js | 2 +- src/wait-for-dom-change.js | 3 + src/wait-for-element-to-be-removed.js | 113 ++++++------------ src/wait-for-element.js | 82 +++---------- src/wait.js | 77 +++++++++++- 7 files changed, 185 insertions(+), 147 deletions(-) diff --git a/package.json b/package.json index 87f2a314..df2e1ca3 100644 --- a/package.json +++ b/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", diff --git a/src/__tests__/fake-timers.js b/src/__tests__/fake-timers.js index 50f1c676..d3148528 100644 --- a/src/__tests__/fake-timers.js +++ b/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 +}) diff --git a/src/__tests__/wait-for-element-to-be-removed.js b/src/__tests__/wait-for-element-to-be-removed.js index e52dfd21..5cf99384 100644 --- a/src/__tests__/wait-for-element-to-be-removed.js +++ b/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"`, ) }) diff --git a/src/wait-for-dom-change.js b/src/wait-for-dom-change.js index 09b742bf..6184de0e 100644 --- a/src/wait-for-dom-change.js +++ b/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, diff --git a/src/wait-for-element-to-be-removed.js b/src/wait-for-element-to-be-removed.js index 8bcbccb2..1f288191 100644 --- a/src/wait-for-element-to-be-removed.js +++ b/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" +*/ diff --git a/src/wait-for-element.js b/src/wait-for-element.js index 30fe0e9a..cde9db2c 100644 --- a/src/wait-for-element.js +++ b/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" +*/ diff --git a/src/wait.js b/src/wait.js index 193aa057..801d7e9f 100644 --- a/src/wait.js +++ b/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) {