Skip to content

Commit 14d34e0

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 14d34e0

File tree

4 files changed

+172
-55
lines changed

4 files changed

+172
-55
lines changed

‎src/__tests__/new-act.js

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import {asyncAct} from '../act-compat'
2+
3+
jest.mock('react-dom/test-utils', () => ({
4+
act: cb => {
5+
return cb()
6+
},
7+
}))
8+
9+
test('async act works when it does not exist (older versions of react)', async () => {
10+
jest.spyOn(console, 'error').mockImplementation(() => {})
11+
const callback = jest.fn()
12+
await asyncAct(async () => {
13+
await Promise.resolve()
14+
await callback()
15+
})
16+
expect(console.error).toHaveBeenCalledTimes(0)
17+
expect(callback).toHaveBeenCalledTimes(1)
18+
19+
callback.mockClear()
20+
console.error.mockClear()
21+
22+
await asyncAct(async () => {
23+
await Promise.resolve()
24+
await callback()
25+
})
26+
expect(console.error).toHaveBeenCalledTimes(0)
27+
expect(callback).toHaveBeenCalledTimes(1)
28+
29+
console.error.mockRestore()
30+
})
31+
32+
/* eslint no-console:0 */

‎src/__tests__/no-act.js

+26
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {asyncAct} from '../act-compat'
12
import {act} from '..'
23

34
jest.mock('react-dom/test-utils', () => ({}))
@@ -7,3 +8,28 @@ test('act works even when there is no act from test utils', () => {
78
act(callback)
89
expect(callback).toHaveBeenCalledTimes(1)
910
})
11+
12+
test('async act works when it does not exist (older versions of react)', async () => {
13+
jest.spyOn(console, 'error').mockImplementation(() => {})
14+
const callback = jest.fn()
15+
await asyncAct(async () => {
16+
await Promise.resolve()
17+
await callback()
18+
})
19+
expect(console.error).toHaveBeenCalledTimes(0)
20+
expect(callback).toHaveBeenCalledTimes(1)
21+
22+
callback.mockClear()
23+
console.error.mockClear()
24+
25+
await asyncAct(async () => {
26+
await Promise.resolve()
27+
await callback()
28+
})
29+
expect(console.error).toHaveBeenCalledTimes(0)
30+
expect(callback).toHaveBeenCalledTimes(1)
31+
32+
console.error.mockRestore()
33+
})
34+
35+
/* eslint no-console:0 */

‎src/__tests__/old-act.js

+20-9
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@ jest.mock('../react-dom-16.9.0-is-released', () => ({
66

77
jest.mock('react-dom/test-utils', () => ({
88
act: cb => {
9-
const promise = cb()
9+
cb()
1010
return {
1111
then() {
12-
console.error('blah, do not do this')
13-
return promise
12+
console.error(
13+
'Warning: Do not await the result of calling ReactTestUtils.act(...), it is not a Promise.',
14+
)
1415
},
1516
}
1617
},
@@ -20,16 +21,26 @@ test('async act works even when the act is an old one', async () => {
2021
jest.spyOn(console, 'error').mockImplementation(() => {})
2122
const callback = jest.fn()
2223
await asyncAct(async () => {
24+
console.error('sigil')
2325
await Promise.resolve()
2426
await callback()
27+
console.error('sigil')
2528
})
2629
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-
`)
30+
Array [
31+
Array [
32+
Array [
33+
"sigil",
34+
],
35+
],
36+
Array [
37+
"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.",
38+
],
39+
Array [
40+
"sigil",
41+
],
42+
]
43+
`)
3344
expect(callback).toHaveBeenCalledTimes(1)
3445

3546
// and it doesn't warn you twice

‎src/act-compat.js

+94-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,101 @@ 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+
62+
if (isAsyncActSupported === false) {
63+
console.error = originalConsoleError
64+
/* istanbul-ignore-next */
65+
if (!youHaveBeenWarned && reactDomSixteenPointNineIsReleased) {
66+
// if act is supported and async act isn't and they're trying to use async
67+
// act, then they need to upgrade from 16.8 to 16.9.
68+
// This is a seemless upgrade, so we'll add a warning
69+
console.error(
70+
`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.`,
71+
)
72+
youHaveBeenWarned = true
73+
}
74+
75+
cbReturn.then(() => {
76+
// a faux-version.
77+
// todo - copy https://github.com/facebook/react/blob/master/packages/shared/enqueueTask.js
78+
Promise.resolve().then(() => {
79+
// use sync act to flush effects
80+
act(() => {})
81+
resolve()
82+
})
83+
})
84+
}
85+
})
86+
} else if (isAsyncActSupported === false) {
87+
// use the polyfill directly
88+
let result
89+
act(() => {
90+
result = cb()
91+
})
92+
return result.then(() => {
93+
return Promise.resolve().then(() => {
94+
// use sync act to flush effects
95+
act(() => {})
96+
})
97+
})
98+
}
99+
// all good! regular act
100+
return act(cb)
58101
}
59-
await cb()
60-
// make all effects resolve after
61-
act(() => {})
102+
// use the polyfill
103+
let result
104+
act(() => {
105+
result = cb()
106+
})
107+
return result.then(() => {
108+
return Promise.resolve().then(() => {
109+
// use sync act to flush effects
110+
act(() => {})
111+
})
112+
})
62113
}
63114

64-
// istanbul ignore next
65-
const asyncAct = asyncActSupported ? reactAct : asyncActPolyfill
66-
67115
export default act
68116
export {asyncAct}
69117

0 commit comments

Comments
 (0)