Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/__tests__/wait-for.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,42 @@ test('the real timers => fake timers error shows the original stack trace when c

expect((await waitForError).stack).not.toMatch(__dirname)
})

test('allow further async tasks to complete after the MutationObserver callback fired', async () => {
jest.useRealTimers()
renderIntoDocument(`<div id="async">a</div>`)
let waitForCount = 0
const el = document.getElementById('async')
const update = () => {
setTimeout(() => {
el.textContent += 'a'
}, 1)
}

update()
update()
update()

await waitFor(() => {
waitForCount++
expect(el).toHaveTextContent('aaaa')
})

// initial sync check + mutation check
expect(waitForCount).toBe(2)
})

test('avoid running the next check when the previous check is still pending', async () => {
jest.useFakeTimers()
let currentlyRunningCount = 0

await waitFor(async () => {
currentlyRunningCount++
await new Promise(resolve => {
setTimeout(resolve, 1)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be greater than interval to illustrate that the callback always completed before another one is started?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test fails as expected when this line is removed 🤷‍♂️:

if (promiseStatus === 'pending') return

When using fake timers, we use setTimeout(r, 0) in a while loop, which is why this test works:

setTimeout(r, 0)

I can still increase the timer if you'd prefer.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how this is related. What are you testing and why is it related to flushing microtasks? My original statement would still apply to real timer usage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add this test to reach 100% testing coverage

if (promiseStatus === 'pending') return

☝️ this line isn't covered anymore without this test.

I've tweaked the test now, please take another look.

})
currentlyRunningCount--
})

expect(currentlyRunningCount).toBe(0)
})
39 changes: 33 additions & 6 deletions src/wait-for.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ function copyStackTrace(target, source) {
target.stack = source.stack.replace(source.message, target.message)
}

// `requestIdleCallback` is not available in Node/JSDOM environments,
// so we fallback to setTimeout instead with a little bit of timeout
// to allow non-test async tasks to run.
const requestIdleCallback =
// eslint-disable-next-line no-undef
globalThis.requestIdleCallback ?? (fn => setTimeout(fn, 5))
// eslint-disable-next-line no-undef
const cancelIdleCallback = globalThis.cancelIdleCallback ?? clearTimeout

function waitFor(
callback,
{
Expand Down Expand Up @@ -43,7 +52,7 @@ function waitFor(
}

return new Promise(async (resolve, reject) => {
let lastError, intervalId, observer
let lastError, timerId, observer, idleCallbackId
let finished = false
let promiseStatus = 'idle'

Expand Down Expand Up @@ -101,9 +110,15 @@ function waitFor(
reject(e)
return
}
intervalId = setInterval(checkRealTimersCallback, interval)
Comment thread
eps1lon marked this conversation as resolved.
const {MutationObserver} = getWindowFromNode(container)
observer = new MutationObserver(checkRealTimersCallback)
observer = new MutationObserver(() => {
// It's not unlikely for multiple async tasks to generate multiple DOM mutations in quick successions,
// so we run the check when idle to give async tasks enough room to run to completion.
// If each async tasks has to wait for an expensive `waitFor` check to complete, it can lead to timeouts.
if (idleCallbackId === undefined) {
idleCallbackId = requestIdleCallback(checkRealTimersCallback)
}
})
observer.observe(container, mutationObserverOptions)
checkCallback()
}
Expand All @@ -113,7 +128,8 @@ function waitFor(
clearTimeout(overallTimeoutTimer)

if (!usingJestFakeTimers) {
clearInterval(intervalId)
clearTimeout(timerId)
cancelIdleCallback(idleCallbackId)
observer.disconnect()
}

Expand All @@ -125,6 +141,10 @@ function waitFor(
}

function checkRealTimersCallback() {
clearTimeout(timerId)
cancelIdleCallback(idleCallbackId)
idleCallbackId = undefined

if (jestFakeTimersAreEnabled()) {
const error = new Error(
`Changed from using real timers to fake timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to fake timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
Expand All @@ -149,7 +169,7 @@ function waitFor(
},
rejectedValue => {
promiseStatus = 'rejected'
lastError = rejectedValue
handleError(rejectedValue)
},
)
} else {
Expand All @@ -158,7 +178,14 @@ function waitFor(
// If `callback` throws, wait for the next mutation, interval, or timeout.
} catch (error) {
// Save the most recent callback error to reject the promise with it in the event of a timeout
lastError = error
handleError(error)
}
}

function handleError(error) {
lastError = error
if (!usingJestFakeTimers) {
timerId = setTimeout(checkRealTimersCallback, interval)
}
}

Expand Down