Skip to content

Commit a0436d4

Browse files
grigaspKent C. Dodds
authored and
Kent C. Dodds
committed
fix: ensure waitForElement disconnects MutationObserver at the right time (#102)
<!-- What changes are being made? (What feature/bug is being fixed here?) --> **What**: #99 waitForElement gets into infinite setTimeout loop <!-- Why are these changes necessary? --> **Why**: without the change MutationObserver gets into infinite setTimeout loop <!-- How were these changes implemented? --> **How**: disconnect the MutationObserver from setImmediate callback to ensure we're not in MutationObserver's `onMutate` callback when disconnecting it. <!-- Have you done all of these things? --> **Checklist**: <!-- add "N/A" to the end of each line that's irrelevant to your changes --> <!-- to check an item, place an "x" in the box like so: "- [x] Documentation" --> - [ ] Documentation - [x] Tests - [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? --> - [ ] Added myself to contributors table <!-- this is optional, see the contributing guidelines for instructions --> <!-- feel free to add additional comments --> Closes #99
1 parent 09f1709 commit a0436d4

File tree

2 files changed

+46
-1
lines changed

2 files changed

+46
-1
lines changed

src/__tests__/wait-for-element.js

+45
Original file line numberDiff line numberDiff line change
@@ -312,3 +312,48 @@ test('it returns immediately if the callback returns the value before any mutati
312312

313313
return promise
314314
})
315+
316+
test('does not get into infinite setTimeout loop after MutationObserver notification', async () => {
317+
const {container} = render(`<div data-testid="initial-element"></div>`)
318+
319+
let didMakeMutation = false
320+
const callback = jest.fn(() => didMakeMutation).mockName('callback')
321+
const successHandler = jest.fn().mockName('successHandler')
322+
const errorHandler = jest.fn().mockName('errorHandler')
323+
jest.useFakeTimers()
324+
325+
const promise = waitForElement(callback, {
326+
container,
327+
timeout: 70,
328+
mutationObserverOptions: {attributes: true},
329+
}).then(successHandler, errorHandler)
330+
331+
// Expect 2 timeouts to be scheduled:
332+
// - waitForElement timeout
333+
// - MutationObserver timeout
334+
expect(setTimeout).toHaveBeenCalledTimes(2)
335+
336+
// One synchronous `callback` call is expected.
337+
expect(callback).toHaveBeenCalledTimes(1)
338+
expect(successHandler).toHaveBeenCalledTimes(0)
339+
expect(errorHandler).toHaveBeenCalledTimes(0)
340+
341+
// Make a mutation so MutationObserver calls out callback
342+
container.setAttribute('data-test-attribute', 'something changed')
343+
didMakeMutation = true
344+
345+
// Advance timer to expire MutationObserver timeout
346+
jest.advanceTimersByTime(50)
347+
jest.runAllImmediates()
348+
await promise
349+
expect(setTimeout).toHaveBeenCalledTimes(3)
350+
351+
// Expect callback and successHandler to be called
352+
expect(callback).toHaveBeenCalledTimes(2)
353+
expect(successHandler).toHaveBeenCalledTimes(1)
354+
expect(errorHandler).toHaveBeenCalledTimes(0)
355+
356+
// Expect no more setTimeout calls
357+
jest.advanceTimersByTime(100)
358+
expect(setTimeout).toHaveBeenCalledTimes(3)
359+
})

src/wait-for-element.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ function waitForElement(
1818
let lastError, observer, timer // eslint-disable-line prefer-const
1919
function onDone(error, result) {
2020
clearTimeout(timer)
21-
observer.disconnect()
21+
setImmediate(() => observer.disconnect())
2222
if (error) {
2323
reject(error)
2424
} else {

0 commit comments

Comments
 (0)