Skip to content

Commit 89abc88

Browse files
threepointonethreepointone
committed
fix async act detection
The previous version of async act detection left an open hanging act scope, which broke tests and expectations. This PR delays the detection until it's been called at least once.
1 parent 4aa0c56 commit 89abc88

File tree

3 files changed

+169
-64
lines changed

3 files changed

+169
-64
lines changed

‎.watchmanconfig

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

‎src/__tests__/old-act.js

+79-18
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,92 @@ jest.mock('../react-dom-16.9.0-is-released', () => ({
44
reactDomSixteenPointNineIsReleased: true,
55
}))
66

7-
jest.mock('react-dom/test-utils', () => ({
8-
act: cb => {
9-
const promise = cb()
10-
return {
11-
then() {
12-
console.error('blah, do not do this')
13-
return promise
14-
},
15-
}
16-
},
17-
}))
18-
197
test('async act works even when the act is an old one', async () => {
8+
jest.mock('react-dom/test-utils', () => ({
9+
act: cb => {
10+
cb()
11+
return {
12+
then() {
13+
console.error(
14+
'Warning: Do not await the result of calling ReactTestUtils.act(...), it is not a Promise.',
15+
)
16+
},
17+
}
18+
},
19+
}))
20+
2021
jest.spyOn(console, 'error').mockImplementation(() => {})
2122
const callback = jest.fn()
2223
await asyncAct(async () => {
2324
await Promise.resolve()
2425
await callback()
2526
})
2627
expect(console.error.mock.calls).toMatchInlineSnapshot(`
27-
Array [
28-
Array [
29-
"It looks like you're using a version of react-dom that supports the \\"act\\" function, but not an awaitable version of \\"act\\" which you will need. Please upgrade to at least [email protected] to remove this warning.",
30-
],
31-
]
32-
`)
28+
Array [
29+
Array [
30+
"It looks like you're using a version of react-dom that supports the \\"act\\" function, but not an awaitable version of \\"act\\" which you will need. Please upgrade to at least [email protected] to remove this warning.",
31+
],
32+
]
33+
`)
34+
expect(callback).toHaveBeenCalledTimes(1)
35+
36+
// and it doesn't warn you twice
37+
callback.mockClear()
38+
console.error.mockClear()
39+
40+
await asyncAct(async () => {
41+
await Promise.resolve()
42+
await callback()
43+
})
44+
expect(console.error).toHaveBeenCalledTimes(0)
45+
expect(callback).toHaveBeenCalledTimes(1)
46+
47+
console.error.mockRestore()
48+
49+
jest.unmock('react-dom/test-utils')
50+
})
51+
52+
test('should not warn with a version that resolves', async () => {
53+
jest.mock('react-dom/test-utils', () => ({
54+
act: cb => {
55+
return cb()
56+
},
57+
}))
58+
59+
jest.spyOn(console, 'error').mockImplementation(() => {})
60+
const callback = jest.fn()
61+
await asyncAct(async () => {
62+
await Promise.resolve()
63+
await callback()
64+
})
65+
expect(console.error).toHaveBeenCalledTimes(0)
66+
expect(callback).toHaveBeenCalledTimes(1)
67+
68+
// and it doesn't warn you twice
69+
callback.mockClear()
70+
console.error.mockClear()
71+
72+
await asyncAct(async () => {
73+
await Promise.resolve()
74+
await callback()
75+
})
76+
expect(console.error).toHaveBeenCalledTimes(0)
77+
expect(callback).toHaveBeenCalledTimes(1)
78+
79+
console.error.mockRestore()
80+
jest.unmock('react-dom/test-utils')
81+
})
82+
83+
test('async act works when it does not exist (older versions of react)', async () => {
84+
jest.mock('react-dom/test-utils', () => ({}))
85+
86+
jest.spyOn(console, 'error').mockImplementation(() => {})
87+
const callback = jest.fn()
88+
await asyncAct(async () => {
89+
await Promise.resolve()
90+
await callback()
91+
})
92+
expect(console.error).toHaveBeenCalledTimes(0)
3393
expect(callback).toHaveBeenCalledTimes(1)
3494

3595
// and it doesn't warn you twice
@@ -44,6 +104,7 @@ Array [
44104
expect(callback).toHaveBeenCalledTimes(1)
45105

46106
console.error.mockRestore()
107+
jest.unmock('react-dom/test-utils')
47108
})
48109

49110
/* eslint no-console:0 */

‎src/act-compat.js

+89-46
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,8 @@ import React from 'react'
22
import ReactDOM from 'react-dom'
33
import {reactDomSixteenPointNineIsReleased} from './react-dom-16.9.0-is-released'
44

5-
let reactAct
6-
let actSupported = false
7-
let asyncActSupported = false
8-
try {
9-
reactAct = require('react-dom/test-utils').act
10-
actSupported = reactAct !== undefined
11-
12-
const originalError = console.error
13-
let errorCalled = false
14-
console.error = () => {
15-
errorCalled = true
16-
}
17-
console.error.calls = []
18-
/* istanbul ignore next */
19-
reactAct(() => ({then: () => {}})).then(() => {})
20-
/* istanbul ignore next */
21-
if (!errorCalled) {
22-
asyncActSupported = true
23-
}
24-
console.error = originalError
25-
} catch (error) {
26-
// ignore, this is to support old versions of react
27-
}
5+
const reactAct = require('react-dom/test-utils').act
6+
const actSupported = reactAct !== undefined
287

298
// act is supported [email protected]
309
// so for versions that don't have act from test utils
@@ -38,32 +17,96 @@ function actPolyfill(cb) {
3817
const act = reactAct || actPolyfill
3918

4019
let youHaveBeenWarned = false
41-
// this will not avoid warnings that react-dom 16.8.0 logs for triggering
42-
// state updates asynchronously, but at least we can tell people they need
43-
// to upgrade to avoid the warnings.
44-
async function asyncActPolyfill(cb) {
45-
// istanbul-ignore-next
46-
if (
47-
!youHaveBeenWarned &&
48-
actSupported &&
49-
reactDomSixteenPointNineIsReleased
50-
) {
51-
// if act is supported and async act isn't and they're trying to use async
52-
// act, then they need to upgrade from 16.8 to 16.9.
53-
// This is a seemless upgrade, so we'll add a warning
54-
console.error(
55-
`It looks like you're using a version of react-dom that supports the "act" function, but not an awaitable version of "act" which you will need. Please upgrade to at least [email protected] to remove this warning.`,
56-
)
57-
youHaveBeenWarned = true
20+
let isAsyncActSupported = null
21+
22+
function asyncAct(cb) {
23+
if (actSupported === true) {
24+
if (isAsyncActSupported === null) {
25+
return new Promise((resolve, reject) => {
26+
// patch console.error here
27+
const originalConsoleError = console.error
28+
console.error = function error(...args) {
29+
/* if console.error fired *with that specific message* */
30+
if (
31+
args[0].indexOf(
32+
'Warning: Do not await the result of calling ReactTestUtils.act',
33+
) === 0
34+
) {
35+
// v16.8.6
36+
isAsyncActSupported = false
37+
} else if (
38+
args[0].indexOf(
39+
'Warning: The callback passed to ReactTestUtils.act(...) function must not return anything',
40+
) === 0
41+
) {
42+
// no-op
43+
} else {
44+
originalConsoleError.call(console, args)
45+
}
46+
}
47+
let cbReturn
48+
const result = reactAct(() => {
49+
cbReturn = cb()
50+
return cbReturn
51+
})
52+
53+
result.then(() => {
54+
console.error = originalConsoleError
55+
// if it got here, it means async act is supported
56+
isAsyncActSupported = true
57+
resolve()
58+
}, reject)
59+
60+
// 16.8.6's act().then() doesn't call a resolve handler, so we need to manually flush here, sigh
61+
if (isAsyncActSupported === false) {
62+
console.error = originalConsoleError
63+
/* istanbul-ignore-next */
64+
if (!youHaveBeenWarned && reactDomSixteenPointNineIsReleased) {
65+
// if act is supported and async act isn't and they're trying to use async
66+
// act, then they need to upgrade from 16.8 to 16.9.
67+
// This is a seemless upgrade, so we'll add a warning
68+
console.error(
69+
`It looks like you're using a version of react-dom that supports the "act" function, but not an awaitable version of "act" which you will need. Please upgrade to at least [email protected] to remove this warning.`,
70+
)
71+
youHaveBeenWarned = true
72+
}
73+
74+
cbReturn.then(() => {
75+
// a faux-version.
76+
// todo - copy https://github.com/facebook/react/blob/master/packages/shared/enqueueTask.js
77+
Promise.resolve().then(() => {
78+
// use sync act to flush effects
79+
act(() => {})
80+
resolve()
81+
})
82+
})
83+
}
84+
})
85+
} else if (isAsyncActSupported === false) {
86+
// use the polyfill directly
87+
let result
88+
act(() => {
89+
result = cb()
90+
})
91+
return result.then(() => {
92+
return Promise.resolve().then(() => {
93+
// use sync act to flush effects
94+
act(() => {})
95+
})
96+
})
97+
} else if (isAsyncActSupported === true) {
98+
// all good! regular act
99+
return act(cb)
100+
}
58101
}
59-
await cb()
60-
// make all effects resolve after
61-
act(() => {})
102+
// use the polyfill
103+
ReactDOM.unstable_batchedUpdates(cb)
104+
return Promise.resolve().then(() => {
105+
// use sync act to flush effects
106+
act(() => {})
107+
})
62108
}
63109

64-
// istanbul ignore next
65-
const asyncAct = asyncActSupported ? reactAct : asyncActPolyfill
66-
67110
export default act
68111
export {asyncAct}
69112

0 commit comments

Comments
 (0)