Skip to content

Commit efd21b4

Browse files
committed
Defer refactoring to later
1 parent ce51eec commit efd21b4

File tree

2 files changed

+60
-65
lines changed

2 files changed

+60
-65
lines changed

src/__tests__/wait-for.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ test('if you switch from fake timers to real timers during the wait period you g
207207
const error = await waitForError
208208

209209
expect(error.message).toMatchInlineSnapshot(
210-
`Changed from using fake timers to real 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 what timers your test is using. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
210+
`Changed from using fake timers to real 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 real timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
211211
)
212212
// stack trace has this file in it
213213
expect(error.stack).toMatch(__dirname)
@@ -223,7 +223,7 @@ test('if you switch from real timers to fake timers during the wait period you g
223223
const error = await waitForError
224224

225225
expect(error.message).toMatchInlineSnapshot(
226-
`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 what timers your test is using. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
226+
`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`,
227227
)
228228
// stack trace has this file in it
229229
expect(error.stack).toMatch(__dirname)

src/wait-for.js

+58-63
Original file line numberDiff line numberDiff line change
@@ -43,72 +43,79 @@ function waitFor(
4343
}
4444

4545
return new Promise(async (resolve, reject) => {
46-
let lastError
46+
let lastError, intervalId, observer
4747
let finished = false
4848
let promiseStatus = 'idle'
4949

50-
const overallTimeout = setTimeout(handleTimeout, timeout)
51-
const intervalId = setInterval(handleInterval, interval)
50+
const overallTimeoutTimer = setTimeout(handleTimeout, timeout)
5251

53-
try {
54-
checkContainerType(container)
55-
} catch (e) {
56-
reject(e)
57-
return
58-
}
59-
const {MutationObserver} = getWindowFromNode(container)
60-
const observer = new MutationObserver(handleInterval)
61-
observer.observe(container, mutationObserverOptions)
62-
63-
checkCallback()
64-
65-
const wasUsingJestFakeTimers = jestFakeTimersAreEnabled()
66-
if (wasUsingJestFakeTimers) {
52+
const usingJestFakeTimers = jestFakeTimersAreEnabled()
53+
if (usingJestFakeTimers) {
6754
const {unstable_advanceTimersWrapper: advanceTimersWrapper} = getConfig()
68-
55+
checkCallback()
6956
// this is a dangerous rule to disable because it could lead to an
7057
// infinite loop. However, eslint isn't smart enough to know that we're
7158
// setting finished inside `onDone` which will be called when we're done
7259
// waiting or when we've timed out.
7360
// eslint-disable-next-line no-unmodified-loop-condition
7461
while (!finished) {
75-
try {
76-
// jest.advanceTimersByTime will not throw if
77-
ensureInvariantTimers()
78-
79-
advanceTimersWrapper(() => {
80-
// we *could* (maybe should?) use `advanceTimersToNextTimer` but it's
81-
// possible that could make this loop go on forever if someone is using
82-
// third party code that's setting up recursive timers so rapidly that
83-
// the user's timer's don't get a chance to resolve. So we'll advance
84-
// by an interval instead. (We have a test for this case).
85-
jest.advanceTimersByTime(interval)
86-
})
87-
88-
// In this rare case, we *need* to wait for in-flight promises
89-
// to resolve before continuing. We don't need to take advantage
90-
// of parallelization so we're fine.
91-
// https://stackoverflow.com/a/59243586/971592
92-
// eslint-disable-next-line no-await-in-loop
93-
await advanceTimersWrapper(async () => {
94-
await new Promise(r => {
95-
setTimeout(r, 0)
96-
jest.advanceTimersByTime(0)
97-
})
98-
})
99-
} catch (error) {
62+
if (!jestFakeTimersAreEnabled()) {
63+
const error = new Error(
64+
`Changed from using fake timers to real 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 real timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
65+
)
66+
if (!showOriginalStackTrace) copyStackTrace(error, stackTraceError)
10067
reject(error)
10168
return
10269
}
70+
// we *could* (maybe should?) use `advanceTimersToNextTimer` but it's
71+
// possible that could make this loop go on forever if someone is using
72+
// third party code that's setting up recursive timers so rapidly that
73+
// the user's timer's don't get a chance to resolve. So we'll advance
74+
// by an interval instead. (We have a test for this case).
75+
advanceTimersWrapper(() => {
76+
jest.advanceTimersByTime(interval)
77+
})
78+
79+
// It's really important that checkCallback is run *before* we flush
80+
// in-flight promises. To be honest, I'm not sure why, and I can't quite
81+
// think of a way to reproduce the problem in a test, but I spent
82+
// an entire day banging my head against a wall on this.
83+
checkCallback()
84+
85+
// In this rare case, we *need* to wait for in-flight promises
86+
// to resolve before continuing. We don't need to take advantage
87+
// of parallelization so we're fine.
88+
// https://stackoverflow.com/a/59243586/971592
89+
// eslint-disable-next-line no-await-in-loop
90+
await advanceTimersWrapper(async () => {
91+
await new Promise(r => {
92+
setTimeout(r, 0)
93+
jest.advanceTimersByTime(0)
94+
})
95+
})
96+
}
97+
} else {
98+
try {
99+
checkContainerType(container)
100+
} catch (e) {
101+
reject(e)
102+
return
103103
}
104+
intervalId = setInterval(checkRealTimersCallback, interval)
105+
const {MutationObserver} = getWindowFromNode(container)
106+
observer = new MutationObserver(checkRealTimersCallback)
107+
observer.observe(container, mutationObserverOptions)
108+
checkCallback()
104109
}
105110

106111
function onDone(error, result) {
107112
finished = true
108-
clearTimeout(overallTimeout)
109-
clearInterval(intervalId)
113+
clearTimeout(overallTimeoutTimer)
110114

111-
observer.disconnect()
115+
if (!usingJestFakeTimers) {
116+
clearInterval(intervalId)
117+
observer.disconnect()
118+
}
112119

113120
if (error) {
114121
reject(error)
@@ -117,27 +124,15 @@ function waitFor(
117124
}
118125
}
119126

120-
function ensureInvariantTimers() {
121-
const isUsingJestFakeTimers = jestFakeTimersAreEnabled()
122-
if (wasUsingJestFakeTimers !== isUsingJestFakeTimers) {
127+
function checkRealTimersCallback() {
128+
if (jestFakeTimersAreEnabled()) {
123129
const error = new Error(
124-
`Changed from using ${
125-
wasUsingJestFakeTimers ? 'fake timers' : 'real timers'
126-
} to ${
127-
isUsingJestFakeTimers ? 'fake timers' : 'real timers'
128-
} 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 what timers your test is using. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
130+
`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`,
129131
)
130132
if (!showOriginalStackTrace) copyStackTrace(error, stackTraceError)
131-
throw error
132-
}
133-
}
134-
135-
function handleInterval() {
136-
try {
137-
ensureInvariantTimers()
138-
return checkCallback()
139-
} catch (error) {
140133
return reject(error)
134+
} else {
135+
return checkCallback()
141136
}
142137
}
143138

0 commit comments

Comments
 (0)