Skip to content

Commit 9ef55bb

Browse files
committed
don't auto-disable act, throw error instead
1 parent 1ad050f commit 9ef55bb

File tree

4 files changed

+58
-31
lines changed

4 files changed

+58
-31
lines changed

src/disableActEnvironment.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,34 @@ const dispose: typeof Symbol.dispose =
33
Symbol.dispose ?? Symbol.for('nodejs.dispose')
44

55
/**
6-
* Temporarily disable act warnings.
6+
* Helper to temporarily disable a React 18+ act environment.
77
*
8-
* https://github.com/reactwg/react-18/discussions/102
8+
* This returns a disposable and can be used in combination with `using` to
9+
* automatically restore the state from before this method call after your test.
10+
*
11+
* @example
12+
* ```ts
13+
* test("my test", () => {
14+
* using _disabledAct = disableActEnvironment();
15+
*
16+
* // your test code here
17+
*
18+
* // as soon as this scope is left, the environment will be cleaned up
19+
* })
20+
* ```
21+
*
22+
* If you can not use the explicit resouce management keyword `using`,
23+
* you can also manually call `cleanup`:
24+
*
25+
* @example
26+
* ```ts
27+
* test("my test", () => {
28+
* const { cleanup } = disableActEnvironment();
29+
*
30+
* // your test code here
31+
*
32+
* cleanup();
33+
* })
934
*/
1035
export function disableActEnvironment(): {cleanup: () => void} & Disposable {
1136
const anyThis = globalThis as any as {IS_REACT_ACT_ENVIRONMENT?: boolean}

src/renderStream/createRenderStream.tsx

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -296,11 +296,6 @@ export function createRenderStream<
296296
takeRender: markAssertable(async function takeRender(
297297
options: NextRenderOptions = {},
298298
) {
299-
// In many cases we do not control the resolution of the suspended
300-
// promise which results in noisy tests when the profiler due to
301-
// repeated act warnings.
302-
const disabledAct = disableActEnvironment()
303-
304299
let error: unknown
305300

306301
try {
@@ -317,7 +312,6 @@ export function createRenderStream<
317312
if (!(error && error instanceof WaitForRenderTimeoutError)) {
318313
iteratorPosition++
319314
}
320-
disabledAct.cleanup()
321315
}
322316
}, stream),
323317
getCurrentRender() {

src/renderWithoutAct.tsx

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -173,41 +173,49 @@ export function renderWithoutAct(
173173
function createLegacyRoot(container: ReactDOMClient.Container) {
174174
return {
175175
render(element: React.ReactNode) {
176-
withDisabledActEnvironment(() =>
177-
ReactDOM.render(element as unknown as React.ReactElement, container),
178-
)
176+
ReactDOM.render(element as unknown as React.ReactElement, container)
179177
},
180178
unmount() {
181-
withDisabledActEnvironment(() =>
182-
ReactDOM.unmountComponentAtNode(container),
183-
)
179+
ReactDOM.unmountComponentAtNode(container)
184180
},
185181
}
186182
}
187183

188184
function createConcurrentRoot(container: ReactDOMClient.Container) {
189-
const root = withDisabledActEnvironment(() =>
190-
ReactDOMClient.createRoot(container),
191-
)
185+
const anyThis = globalThis as any as {IS_REACT_ACT_ENVIRONMENT?: boolean}
186+
if (anyThis.IS_REACT_ACT_ENVIRONMENT) {
187+
throw new Error(`Tried to create a React root for a render stream inside a React act environment.
188+
This is not supported. Please use \`disableActEnvironment\` to disable the act environment for this test.`)
189+
}
190+
const root = ReactDOMClient.createRoot(container)
192191

193192
return {
194193
render(element: React.ReactNode) {
195-
withDisabledActEnvironment(() => root.render(element))
194+
if (anyThis.IS_REACT_ACT_ENVIRONMENT) {
195+
throw new Error(`Tried to render a render stream inside a React act environment.
196+
This is not supported. Please use \`disableActEnvironment\` to disable the act environment for this test.`)
197+
}
198+
root.render(element)
196199
},
197200
unmount() {
198-
withDisabledActEnvironment(() => root.unmount())
201+
root.unmount()
199202
},
200203
}
201204
}
202205

203206
export function cleanup() {
204-
mountedRootEntries.forEach(({root, container}) => {
205-
root.unmount()
207+
// there is a good chance this happens outside of a test, where the user
208+
// has no control over enabling or disabling the React Act environment,
209+
// so we do it for them here.
210+
withDisabledActEnvironment(() => {
211+
mountedRootEntries.forEach(({root, container}) => {
212+
root.unmount()
206213

207-
if (container.parentNode === document.body) {
208-
document.body.removeChild(container)
209-
}
214+
if (container.parentNode === document.body) {
215+
document.body.removeChild(container)
216+
}
217+
})
218+
mountedRootEntries.length = 0
219+
mountedContainers.clear()
210220
})
211-
mountedRootEntries.length = 0
212-
mountedContainers.clear()
213221
}

src/withDisabledActEnvironment.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ import {disableActEnvironment} from './disableActEnvironment.js'
22

33
export function withDisabledActEnvironment<T>(cb: () => T): T {
44
const disabledActWarnings = disableActEnvironment()
5-
let result: T
5+
let result: T | undefined
66
try {
77
result = cb()
8-
return result instanceof Promise
9-
? (result.finally(disabledActWarnings.cleanup) as T)
10-
: result
8+
return result
119
} finally {
12-
disabledActWarnings.cleanup()
10+
if (result != null && result instanceof Promise) {
11+
void result.finally(disabledActWarnings.cleanup)
12+
} else disabledActWarnings.cleanup()
1313
}
1414
}

0 commit comments

Comments
 (0)