Repository navigation
React: Fix act environment leak causing act(async) without await warnings in multi-file Vitest runs - #34999
React: Fix act environment leak causing act(async) without await warnings in multi-file Vitest runs#34999psborul wants to merge 5 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces single-value act flag handling with a ref-counted actEnvironment and makes withGlobalActEnvironment eagerly attach to React’s act thenable; adds Vitest coverage for non-awaited thenables, awaited resolve/reject, sync throws, overlapping/nested acts, and getAct fallback/disable cases. ChangesReact act environment ref-counting and tests
🎯 4 (Complex) | ⏱️ ~45 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
code/renderers/react/src/act-compat.test.ts (2)
21-24: ⚡ Quick winMove the
console.errormock into hooks.This inline
spyOn(...).mockImplementation(...)breaks the repo’s Vitest mocking pattern and makes cleanup depend on the test reachingmockRestore(). Hoist the spy setup intobeforeEachand restore it inafterEach/vi.restoreAllMocks()so a failed assertion can't leak the mock into later tests.♻️ Suggested cleanup
-import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { getAct, getReactActEnvironment, setReactActEnvironment } from './act-compat.ts'; // Regression test for https://github.com/storybookjs/storybook/issues/34708. @@ describe('act-compat', () => { + let errors: string[] = []; + beforeEach(() => { + errors = []; setReactActEnvironment(false); + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + errors.push(args.map((arg) => String(arg)).join(' ')); + }); }); + + afterEach(() => { + vi.restoreAllMocks(); + }); it('restores the act environment and does not warn for an un-awaited async act', async () => { - const errors: string[] = []; - const spy = vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { - errors.push(args.map((arg) => String(arg)).join(' ')); - }); - const act = await getAct(); @@ // Let the act work and React's deferred await-tracking check settle without // the caller ever awaiting the thenable. await new Promise((resolve) => setTimeout(resolve, 0)); - spy.mockRestore(); const actWarnings = errors.filter((error) => error.includes('You called act(async () => ...) without await')As per coding guidelines, "Implement mock behaviors in
beforeEachblocks in Vitest tests" and "Avoid inline mock implementations within test cases in Vitest tests."Also applies to: 40-40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/renderers/react/src/act-compat.test.ts` around lines 21 - 24, Move the inline console.error spy setup out of the test body and into test hooks: create the errors array and call vi.spyOn(console, 'error').mockImplementation(...) inside a beforeEach, and restore it in afterEach (or call vi.restoreAllMocks()) so the mock doesn't leak; update references to the existing errors and spy variables (errors, spy/vi.spyOn) and remove the inline mock from the test to rely on the hooked setup.
60-69: ⚡ Quick winAdd the discarded-reject regression case.
The new implementation has distinct resolve and reject cleanup paths when callers drop the returned thenable, but this file only exercises the discarded path for resolve. A
void act(async () => { throw ... })case would cover the remaining branch and catch any missed cleanup or unhandled-rejection regression.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/renderers/react/src/act-compat.test.ts` around lines 60 - 69, Add a test that mirrors the existing "discarded resolve" regression check but for a thrown rejection: call const act = await getAct(); then invoke void act(async () => { throw new Error('boom'); }); await a microtask flush (e.g. await Promise.resolve()) and assert getReactActEnvironment() is false; this ensures the discarded-reject cleanup path in act (the one that handles dropped thenables) runs and does not leave the act environment or produce unhandled rejections.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@code/renderers/react/src/act-compat.test.ts`:
- Around line 21-24: Move the inline console.error spy setup out of the test
body and into test hooks: create the errors array and call vi.spyOn(console,
'error').mockImplementation(...) inside a beforeEach, and restore it in
afterEach (or call vi.restoreAllMocks()) so the mock doesn't leak; update
references to the existing errors and spy variables (errors, spy/vi.spyOn) and
remove the inline mock from the test to rely on the hooked setup.
- Around line 60-69: Add a test that mirrors the existing "discarded resolve"
regression check but for a thrown rejection: call const act = await getAct();
then invoke void act(async () => { throw new Error('boom'); }); await a
microtask flush (e.g. await Promise.resolve()) and assert
getReactActEnvironment() is false; this ensures the discarded-reject cleanup
path in act (the one that handles dropped thenables) runs and does not leave the
act environment or produce unhandled rejections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4da6ac95-8933-46f0-965f-38fb0d46f147
📒 Files selected for processing (2)
code/renderers/react/src/act-compat.test.tscode/renderers/react/src/act-compat.ts
valentinpalkovic
added
the
ci:normal
valentinpalkovic
moved this from Empathy Queue (prioritized)
to In Progress
in Core Team Projects
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 18 | 18 | 0 |
| Self size | 1.31 MB | 1.28 MB | 🎉 -32 KB 🎉 |
| Dependency size | 9.28 MB | 9.28 MB | 🎉 -62 B 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-vitest
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 2 | 0 |
| Self size | 427 KB | 417 KB | 🎉 -10 KB 🎉 |
| Dependency size | 350 KB | 350 KB | 0 B |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 72 | 🎉 -1 🎉 |
| Self size | 21.32 MB | 20.39 MB | 🎉 -932 KB 🎉 |
| Dependency size | 36.44 MB | 35.94 MB | 🎉 -503 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 187 | 187 | 0 |
| Self size | 13 KB | 15 KB | 🚨 +1 KB 🚨 |
| Dependency size | 30.94 MB | 30.94 MB | 🎉 -1 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 533 | 534 | 🚨 +1 🚨 |
| Self size | 640 KB | 662 KB | 🚨 +22 KB 🚨 |
| Dependency size | 62.27 MB | 62.52 MB | 🚨 +247 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 93 | 93 | 0 |
| Self size | 1.37 MB | 1.38 MB | 🚨 +14 KB 🚨 |
| Dependency size | 23.97 MB | 23.99 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 122 | 122 | 0 |
| Self size | 29 KB | 30 KB | 🚨 +904 B 🚨 |
| Dependency size | 25.04 MB | 25.06 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 83 | 83 | 0 |
| Self size | 32 KB | 36 KB | 🚨 +4 KB 🚨 |
| Dependency size | 21.75 MB | 21.76 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 274 | 275 | 🚨 +1 🚨 |
| Self size | 23 KB | 23 KB | 🚨 +787 B 🚨 |
| Dependency size | 48.05 MB | 48.30 MB | 🚨 +247 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/tanstack-react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 84 | 84 | 0 |
| Self size | 105 KB | 106 KB | 🚨 +1 KB 🚨 |
| Dependency size | 21.78 MB | 21.80 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 102 | 109 | 🚨 +7 🚨 |
| Self size | 34 KB | 36 KB | 🚨 +2 KB 🚨 |
| Dependency size | 43.01 MB | 44.31 MB | 🚨 +1.29 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 205 | 204 | 🎉 -1 🎉 |
| Self size | 827 KB | 908 KB | 🚨 +81 KB 🚨 |
| Dependency size | 91.32 MB | 89.88 MB | 🎉 -1.44 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 197 | 🎉 -1 🎉 |
| Self size | 32 KB | 32 KB | 🎉 -43 B 🎉 |
| Dependency size | 89.80 MB | 88.36 MB | 🎉 -1.43 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 74 | 73 | 🎉 -1 🎉 |
| Self size | 1.09 MB | 1.08 MB | 🎉 -9 KB 🎉 |
| Dependency size | 57.77 MB | 56.33 MB | 🎉 -1.43 MB 🎉 |
| Bundle Size Analyzer | node | node |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 59 | 59 | 0 |
| Self size | 1.49 MB | 1.51 MB | 🚨 +17 KB 🚨 |
| Dependency size | 12.45 MB | 12.45 MB | 🎉 -97 B 🎉 |
| Bundle Size Analyzer | Link | Link |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/renderers/react/src/act-compat.test.ts`:
- Around line 148-170: Move the inline vi.doMock calls and the local
testUtilsAct into top-level spy mocks: replace vi.doMock('react', ...) and
vi.doMock('react-dom/test-utils', ...) inside the it block with vi.mock('react',
{ spy: true }) and vi.mock('react-dom/test-utils', { spy: true }) at module
scope; in beforeEach set the mocked implementations via
vi.mocked(require('react-dom/test-utils')) or
vi.mocked(importedModule).mockImplementation(() => ({ act: () => cb() })) (or
set the act mock directly), remove the locally-scoped testUtilsAct variable, and
change assertions to reference the spy via
vi.mocked(...).toHaveBeenCalledOnce(); also clean up mocks in
afterEach/resetModules as needed. Ensure getAct (getActFresh) import in the test
still triggers module loading after mocks are configured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 261c5836-7b66-4f91-9f00-f4cf781af40e
📒 Files selected for processing (2)
code/renderers/react/src/act-compat.test.tscode/renderers/react/src/act-compat.ts
|
@valentinpalkovic Hey! Could you review the PR again? Thanks! |
valentinpalkovic
added
the
sev:S3
|
@storybook-bot Full three-stage review |
![storybook-contribution-review[bot]](https://avatars.githubusercontent.com/u/22632046?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 I am the Storybook contribution review bot. This message was posted automatically. A human maintainer makes the final decision on this PR.
Review phase: Architecture review (stage 1 of 3) and Thermo-nuclear code quality review (stage 2 of 3)We are trying a three-stage automated review: architecture, code quality, and runtime verification. This message states which stages ran and which were skipped. Messages are posted automatically, and a human maintainer makes the final decision. Please tell us whether the feedback and evidence are useful.
Thanks @psborul for sticking with this one. Moving from per-call save/restore to a ref-counted act environment is the right direction, and the new tests do catch #34708 (2 of the 9 fail against the next version of act-compat.ts). Type check and the new tests pass on this head.
Result for 0208d8f3: changes requested. Two blocking items, both in the inline comments:
- The ref-count is not yet the only code that writes
IS_REACT_ACT_ENVIRONMENT. TheasyncWrapperinentry-preview.tsxstill does its own save/restore, so act-vs-act overlap is fixed but act-vs-waitForoverlap is not. - The new eager Promise wrapper can produce an unhandled rejection when
eventWrapperdiscards the act result.
The other inline comments are marked non-blocking and are optional.
Stages: architecture (1) and code quality (2) ran on this head. Runtime verification (3) did not run, because it starts only after stages 1 and 2 pass. We have not yet reproduced the multi-file Vitest warnings or confirmed the fix at runtime.
| enter() { | ||
| if (this.depth === 0) { | ||
| this.baseline = getReactActEnvironment(); | ||
| } | ||
| this.depth = this.depth + 1; | ||
| setReactActEnvironment(true); | ||
| }, |
There was a problem hiding this comment.
🤖 Automated review bot · Architecture (stage 1) · blocking · S1-1
Problem: The ref-count is meant to be the only thing that decides IS_REACT_ACT_ENVIRONMENT, but asyncWrapper in entry-preview.tsx:62-84 (used by waitFor/findBy*) still does its own raw getReactActEnvironment() → setReactActEnvironment(false) → finally restore. enter()/exit() don't see those writes, and asyncWrapper doesn't see depth/baseline.
Effect (reproduced with real React act and an asyncWrapper copy):
- New regression: an async act is pending,
waitForstarts (flag →false), and a sync act runs inside it (this isunstable_advanceTimersWrapper→act(cb)under fake timers). After that inner act the flag istrueon this head andfalseonnext, becauseexit()doesn't restore whiledepth > 0. React then runs in act mode for the rest of thewaitFor, which is whatasyncWrapperexists to prevent (Proposal: `global.IS_REACT_ACT_ENVIRONMENT` reactwg/react-18#102). - Leak that remains: an async act is pending,
waitForcapturesprevious = true, the act settles during thewaitFor(depth → 0, baselinefalserestored), and thenasyncWrapper'sfinallywritestrueback. The flag endstrueat depth 0 and leaks into the next story, the same kind of leak as [Bug]: @storybook/react + @storybook/addon-vitest emits act(async) without await warnings in multi-file Vitest browser-mode runs #34708.
So "reset only after all acts resolve" holds for act-vs-act overlap, but not for act-vs-waitFor.
Requested change: make act-compat.ts the single owner of the flag and route asyncWrapper through it instead of the raw setters. For example, add a suspend scope to the same model (actDepth, suspendDepth, captured baseline) and derive the flag in one place: suspendDepth > 0 ? false : actDepth > 0 ? true : baseline. Export something like withoutActEnvironment(cb) for entry-preview.tsx. Please add tests for both interleavings above.
| return new Promise((resolve, reject) => { | ||
| thenable.then( | ||
| (returnValue: any) => { | ||
| actEnvironment.exit(); | ||
| resolve(returnValue); | ||
| }, | ||
| (error: any) => { | ||
| actEnvironment.exit(); | ||
| reject(error); | ||
| } | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🤖 Automated review bot · Code quality (stage 2) · blocking · S2-1
Problem: This Promise subscribes to React's thenable straight away, so when the act callback rejects, this Promise rejects too. eventWrapper (entry-preview.tsx:86-92) discards the value act(...) returns and gives the caller only cb()'s own promise, so nothing ever handles this one. Before this change the returned object was a lazy thenable, so no rejection existed unless someone awaited it.
Effect: a user-event interaction that rejects inside eventWrapper (for example the normal await expect(userEvent.click(el)).rejects… pattern) now also raises a global unhandled rejection, even when the test handles its own error. Vitest fails the run. Reproduced:
let result;
act(() => { result = Promise.reject(new Error('click failed')); return result; });
await expect(result).rejects.toThrow('click failed');On this head: Tests 1 passed, Errors 1 error, Unhandled Rejection, exit 1. On next: exit 0.
Requested change: keep the eager single subscription, but mark the wrapper as handled while it still rejects for callers that await it, for example const settled = new Promise(...); settled.catch(() => {}); return settled;. Please add a test that discards a rejecting async act and asserts there is no unhandled rejection and that the flag is restored.
| exit() { | ||
| this.depth = this.depth - 1; | ||
| if (this.depth === 0) { | ||
| setReactActEnvironment(this.baseline); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🤖 Automated review bot · Architecture (stage 1) · non-blocking · S1-2
Problem: with a global counter, one act that never settles (for example a story whose render or teardown promise never resolves, or an aborted test) keeps depth > 0 for the rest of the session. Before, a hung act only affected its own restore.
Effect: every later story in the same iframe runs with IS_REACT_ACT_ENVIRONMENT = true, and the resulting act warnings show up far from their cause.
Suggested change (optional): either document this trade-off in the comment above actEnvironment, or reset/assert the depth at an existing story boundary (for example where renderToCanvas teardown runs) so a leaked depth can't cross stories.
| // clear the flag while others are still in flight. Ref-count instead, and only | ||
| // restore the captured baseline once the last concurrent act settles. | ||
| // See https://github.com/storybookjs/storybook/issues/34708. | ||
| const actEnvironment = { |
There was a problem hiding this comment.
🤖 Automated review bot · Architecture (stage 1) / Code quality (stage 2) · non-blocking · S1-3, S2-6
Two optional notes on how this state is stored. If you rework this owner for the blocking asyncWrapper comment, these fit into that change.
- S1-3: the counter is module-local, but the flag it guards is on
globalThis. With one@storybook/reactinstall this is fine (the browser entries share one chunk). With two module instances (two@storybook/reactcopies, or source anddistmixed), each would keep its owndepth/baselineand fight over one global, and the ref-count would silently stop working. Consider keeping the counter onglobalThisunder aSymbol.for(...)key, or note in the comment that one module instance is assumed. - S2-6:
enter/exitdepend onthis. PassingactEnvironment.exitas a callback would losethisand fail at runtime. Two module-level functions overlet depth = 0; let baseline = false;avoid that and are shorter.
| // Attach to React's act thenable eagerly (the executor runs | ||
| // synchronously): some pipeline callers (e.g. the testing-library | ||
| // `eventWrapper`) discard the result without awaiting, yet the | ||
| // environment reset must still be tied to the act work settling. We wrap | ||
| // it in a real Promise because React's act thenable is non-conformant — | ||
| // its `.then` returns `undefined` rather than a chainable promise. |
There was a problem hiding this comment.
🤖 Automated review bot · Code quality (stage 2) · non-blocking · S2-2
Problem: the comment says the wrapper exists because React's .then "returns undefined". The reasons that actually matter (React 18.3 act implementation) are different: each call to thenable.then pops React's act scope, so the thenable must be subscribed exactly once; and that first then sets wasAwaited = true, which is what suppresses the "act(async) without await" warning for discarded acts.
Effect: an edit that looks harmless, such as thenable.then(exit, exit) and then returning thenable, would double-pop React's act scope, and nothing here warns against it.
Suggested change (optional): state the single-subscriber invariant and the wasAwaited side effect in this comment.
| it('stays set until every interleaved act has settled', async () => { | ||
| const act = await getAct(); | ||
|
|
||
| let resolveFirst: () => void = () => {}; | ||
| let resolveSecond: () => void = () => {}; | ||
|
|
||
| const first = act(() => new Promise<void>((resolve) => (resolveFirst = resolve))); | ||
| const second = act(() => new Promise<void>((resolve) => (resolveSecond = resolve))); | ||
|
|
||
| expect(getReactActEnvironment()).toBe(true); | ||
|
|
||
| resolveFirst(); | ||
| await first; | ||
|
|
||
| // The second act is still in flight, so the flag must remain set. | ||
| expect(getReactActEnvironment()).toBe(true); | ||
|
|
||
| resolveSecond(); | ||
| await second; | ||
|
|
||
| expect(getReactActEnvironment()).toBe(false); | ||
| }); |
There was a problem hiding this comment.
🤖 Automated review bot · Code quality (stage 2) · non-blocking · S2-3
Problem: beforeEach resets the global flag but not the module's depth/baseline. If an expect in this test fails before resolveSecond(), depth stays above 0 for the rest of the file.
Effect: enter() never captures the baseline again and exit() never reaches 0, so unrelated later tests fail too, which hides the real failure.
Suggested change (optional): resolve the pending acts in a try/finally (or onTestFinished) so every test leaves depth at 0.
| it('restores the pre-existing value when a nested act settles', async () => { | ||
| const act = await getAct(); | ||
|
|
||
| // An outer act already enabled the environment; the nested act must leave | ||
| // it enabled rather than hardcode it back to `false`. | ||
| setReactActEnvironment(true); | ||
|
|
||
| await act(async () => { | ||
| expect(getReactActEnvironment()).toBe(true); | ||
| await Promise.resolve(); | ||
| }); | ||
|
|
||
| expect(getReactActEnvironment()).toBe(true); | ||
| }); |
There was a problem hiding this comment.
🤖 Automated review bot · Code quality (stage 2) · non-blocking · S2-4
Problem: no act is nested here. The test sets the flag to true by hand and checks that this baseline is restored. Real nesting (an act called inside another act callback), where the depth counter and React's own act scope interact, is not covered.
Suggested change (optional): rename it to say what it checks (for example "restores a pre-existing true baseline"), and consider adding a test with a sync and an async act nested inside an outer async act.
| void (act(async () => { | ||
| await Promise.resolve(); | ||
| }) as unknown as Promise<unknown>); |
There was a problem hiding this comment.
🤖 Automated review bot · Code quality (stage 2) · non-blocking · S2-5
Problem: the as unknown as Promise<unknown> cast isn't needed: getAct() already returns a function typed => any. With the cast removed (void act(async () => { … });), yarn nx run react:check still passes.
Suggested change (optional): drop the cast.
storybook-contribution-review
Bot
added
the
review:changes-requested
This branch has not been deployed
3 participants
Closes #34708
What I did
Fixed a bug where
@storybook/reactemits spuriousact(async () => ...) without awaitwarnings in multi-file Vitest browser-mode runs (deterministic count, scaling with file count, never in single-file runs).In
act-compat.ts,withGlobalActEnvironmentdid a per-call save/restore of the globalIS_REACT_ACT_ENVIRONMENTflag (as react-testing-library does), and tied the async restore to the caller invoking the returned lazy thenable's.then. Two problems surface in Storybook's environment:eventWrapperwith an async callback) never restored it, leaking the flag into the next story's act scope.The fix:
enter()captures the baseline once at depth 0 and sets the flag;exit()restores the captured baseline only when the last concurrent act settles.Promisewith a synchronous executor, so restoration is tied to the act work settling rather than to the caller awaiting, and React's deferred await-tracking check is satisfied. (React's act thenable is non-conformant — its.thenreturnsundefinedrather than a chainable promise, so it can't be returned directly.)This fixes every caller generically without changing the synchronous
eventWrappercontract.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Added
code/renderers/react/src/act-compat.test.ts, covering:falsegetActresolution:disableActno-op, and thereact-dom/test-utilsfallback whenReact.actis unavailableThese fail on the previous implementation and pass with this fix.
Manual testing
Reproduced in a Storybook 10.3.5 +
vitest/@vitest/browser-playwright4.1.4 +react18.3.1 project (Playwright Chromium):play, drive the real Storybook-configured testing-libraryeventWrapperwith an async callback. A setup-fileconsole.errorinterceptor counts the React warning, since Vitest browser mode doesn't forward browserconsole.errorto the terminal.@storybook/react@10.3.5: the suite reportsact(async) without awaitwarnings.Documentation
MIGRATION.MD
No user-facing docs needed — internal correctness fix.
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.Summary by CodeRabbit
Tests
Bug Fixes