Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React: Use Act wrapper in Storybook for component rendering #30037

Open
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Dec 12, 2024

Closes #

What I did

Previously, we have wrapped mounting and unmounting of React component, as well as test interactions via userEvent by act in a portable stories environment. Now we are adapting this behaviour to configure @storybook/test to wrap all component interactions and mounting/unmounting with act. This will make sure, that we can actually await a components rendering properly before we continue to the next Storybook render lifecycle phase.

Background:
The A11y addon has issues to find the right timing to trigger an automated test run if a component uses Suspense (e.g. RSC). With act we can actually properly await the complete rendering of a React component also in the Storybook preview, so that addon a11y can properly await its final rendering to trigger an a11y test run.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make 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.

🦋 Canary release

This pull request has been released as version 0.0.0-pr-30037-sha-13e3869b. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-30037-sha-13e3869b
Triggered by @valentinpalkovic
Repository storybookjs/storybook
Branch valentin/act-wrapping-of-components
Commit 13e3869b
Datetime Thu Dec 12 13:10:36 UTC 2024 (1734009036)
Workflow run 12297097884

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=30037

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.7 MB 77.7 MB 0 B 1.53 0%
initSize 136 MB 136 MB 7.76 kB 1.53 0%
diffSize 58.4 MB 58.4 MB 7.76 kB 1.53 0%
buildSize 7.24 MB 7.41 MB 177 kB 2.21 2.4%
buildSbAddonsSize 1.88 MB 1.87 MB -7.22 kB 1.18 -0.4%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.86 MB 1.86 MB 0 B 1.11 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.93 MB 3.93 MB -7.22 kB 1.18 -0.2%
buildPreviewSize 3.3 MB 3.49 MB 184 kB 2542.43 🔺5.3%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 6.9s 22.7s 15.7s 1.09 69.4%
generateTime 21.5s 24.3s 2.8s 1.75 🔺11.5%
initTime 14.6s 19.2s 4.5s 3.18 🔺23.7%
buildTime 8.8s 9.6s 878ms 0.3 9.1%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 4.2s 4.4s 165ms -1.07 3.7%
devManagerResponsive 3.3s 3.3s -3ms -1.07 -0.1%
devManagerHeaderVisible 500ms 518ms 18ms -0.76 3.5%
devManagerIndexVisible 529ms 548ms 19ms -1 3.5%
devStoryVisibleUncached 1.7s 1.8s 125ms 0.26 6.8%
devStoryVisible 528ms 547ms 19ms -1 3.5%
devAutodocsVisible 497ms 467ms -30ms -0.83 -6.4%
devMDXVisible 456ms 464ms 8ms -0.7 1.7%
buildManagerHeaderVisible 520ms 535ms 15ms -0.67 2.8%
buildManagerIndexVisible 593ms 629ms 36ms -0.63 5.7%
buildStoryVisible 468ms 497ms 29ms -0.6 5.8%
buildAutodocsVisible 404ms 424ms 20ms -0.37 4.7%
buildMDXVisible 391ms 434ms 43ms -0.29 9.9%

Greptile Summary

Based on the provided files and changes, here's a concise summary of the pull request:

Implements proper React act() wrapping for component rendering in Storybook, ensuring consistent behavior between development and testing environments.

  • Added production mode bypass in act-compat.ts to skip act wrapping in production builds
  • Updated renderToCanvas.tsx to wrap both render and unmount operations in act() for proper component lifecycle handling
  • Added __isPortableStory flag in render context to control act wrapping behavior
  • Modified bundle configuration to preserve process.env.NODE_ENV for proper environment-specific behavior
  • Implemented testing library integration with act() for event handling and async operations

Copy link

nx-cloud bot commented Dec 12, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit a52aec2. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@valentinpalkovic valentinpalkovic force-pushed the valentin/act-wrapping-of-components branch from b4a1714 to 4c40b5f Compare December 12, 2024 09:37
@valentinpalkovic valentinpalkovic added the ci:daily Run the CI jobs that normally run in the daily job. label Dec 12, 2024
@valentinpalkovic valentinpalkovic force-pushed the valentin/act-wrapping-of-components branch from 4c40b5f to 95e2429 Compare December 12, 2024 11:05
@valentinpalkovic valentinpalkovic force-pushed the valentin/act-wrapping-of-components branch from 95e2429 to 13e3869 Compare December 12, 2024 13:06
@storybook-pr-benchmarking
Copy link

storybook-pr-benchmarking bot commented Dec 12, 2024

Package Benchmarks

Commit: a52aec2, ran on 15 December 2024 at 10:48:44 UTC

No significant changes detected, all good. 👏

@valentinpalkovic valentinpalkovic marked this pull request as ready for review December 13, 2024 12:36
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile

code/renderers/react/src/act-compat.ts Show resolved Hide resolved
code/renderers/react/src/entry-preview.tsx Show resolved Hide resolved
Comment on lines +70 to +75
let result;
act(() => {
result = cb();
return result;
});
return result;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: result variable could be undefined if act throws an error

code/renderers/react/src/entry-preview.tsx Show resolved Hide resolved
code/renderers/react/src/entry-preview.tsx Show resolved Hide resolved
@valentinpalkovic valentinpalkovic force-pushed the valentin/act-wrapping-of-components branch 2 times, most recently from 76e10e5 to 32b52ee Compare December 15, 2024 06:19
@valentinpalkovic valentinpalkovic force-pushed the valentin/act-wrapping-of-components branch from 32b52ee to a52aec2 Compare December 15, 2024 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ci:daily Run the CI jobs that normally run in the daily job. react
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant