Skip to content

React: Avoid 'Dynamic require of react is not possible' issue#28730

Merged
valentinpalkovic merged 2 commits intonextfrom
valentin/fix-dynamic-require-react-issue-in-vitest
Jul 29, 2024
Merged

React: Avoid 'Dynamic require of react is not possible' issue#28730
valentinpalkovic merged 2 commits intonextfrom
valentin/fix-dynamic-require-react-issue-in-vitest

Conversation

@valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Jul 28, 2024

Closes N/A

What I did

It closes an issue when Vitest runs portable stories. The following error occurs:

Error: Dynamic require of "react" is not supported
 ❯ ../../lib/react-dom-shim/dist/react-18.js ../node_modules/.vite/deps/@storybook_nextjs.js:6298:33
 ❯ ../node_modules/.vite/deps/@storybook_nextjs.js:6103:50
 ❯ ../node_modules/.vite/deps/@storybook_nextjs.js:6327:38

This issue at tsup seems to be related:
egoist/tsup#927

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

Manual testing is not necessary. Automated tests cover it.

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/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-28730-sha-8f30c5c8. Try it out in a new sandbox by running npx storybook@0.0.0-pr-28730-sha-8f30c5c8 sandbox or in an existing project with npx storybook@0.0.0-pr-28730-sha-8f30c5c8 upgrade.

More information
Published version 0.0.0-pr-28730-sha-8f30c5c8
Triggered by @valentinpalkovic
Repository storybookjs/storybook
Branch valentin/fix-dynamic-require-react-issue-in-vitest
Commit 8f30c5c8
Datetime Mon Jul 29 09:00:49 UTC 2024 (1722243649)
Workflow run 10141272720

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=28730

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 76.4 MB 76.4 MB 0 B -0.59 0%
initSize 198 MB 198 MB -279 B 2.33 0%
diffSize 122 MB 122 MB -279 B 2.71 0%
buildSize 7.6 MB 7.6 MB 207 B 35 0%
buildSbAddonsSize 1.63 MB 1.63 MB -31 B -0.23 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 2.3 MB 2.3 MB 0 B - 0%
buildSbPreviewSize 349 kB 349 kB 0 B - 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 4.47 MB 4.47 MB -31 B -0.23 0%
buildPreviewSize 3.12 MB 3.12 MB 238 B Infinity 0%
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 7.8s 8.4s 621ms -0.73 7.3%
generateTime 23.3s 23.8s 559ms 0.49 2.3%
initTime 22.1s 23.2s 1s 0.27 4.7%
buildTime 14.3s 16s 1.6s 1.22 10.5%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 8.4s 7.5s -926ms -0.97 -12.3%
devManagerResponsive 5.6s 5.1s -487ms -1.13 -9.5%
devManagerHeaderVisible 899ms 773ms -126ms -0.77 -16.3%
devManagerIndexVisible 939ms 799ms -140ms -0.76 -17.5%
devStoryVisibleUncached 1.4s 1s -372ms -1.35 🔰-34.3%
devStoryVisible 973ms 817ms -156ms -0.84 -19.1%
devAutodocsVisible 796ms 655ms -141ms -1.13 -21.5%
devMDXVisible 786ms 647ms -139ms -1.13 -21.5%
buildManagerHeaderVisible 855ms 698ms -157ms -1.02 -22.5%
buildManagerIndexVisible 905ms 704ms -201ms -1.02 -28.6%
buildStoryVisible 915ms 745ms -170ms -1.03 -22.8%
buildAutodocsVisible 888ms 665ms -223ms -0.87 -33.5%
buildMDXVisible 725ms 645ms -80ms -0.73 -12.4%

Greptile Summary

This pull request addresses the issue of dynamic require errors for React in Vitest by modifying import statements and dynamic imports.

  • Updated code/lib/react-dom-shim/src/react-16.tsx to use namespace imports for react-dom.
  • Modified code/lib/react-dom-shim/src/react-18.tsx to use namespace imports for both React and ReactDOM, ensuring compatibility with Vitest.
  • Adjusted code/renderers/react/src/renderToCanvas.tsx to dynamically import renderElement and unmountElement from @storybook/react-dom-shim within the renderToCanvas function, preventing dynamic require errors.

@nx-cloud
Copy link

nx-cloud bot commented Jul 28, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 11f43cc. 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 1 target

Sent with 💌 from NxCloud.

@yannbf yannbf added the ci:merged Run the CI jobs that normally run when merged. label Jul 29, 2024
@valentinpalkovic valentinpalkovic force-pushed the valentin/fix-dynamic-require-react-issue-in-vitest branch from 1991726 to c76facf Compare July 29, 2024 07:46
@valentinpalkovic valentinpalkovic marked this pull request as ready for review July 29, 2024 07:49
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.

LGTM

3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@valentinpalkovic valentinpalkovic force-pushed the valentin/fix-dynamic-require-react-issue-in-vitest branch from c76facf to 8f30c5c Compare July 29, 2024 08:36
@valentinpalkovic valentinpalkovic merged commit df101c4 into next Jul 29, 2024
@valentinpalkovic valentinpalkovic deleted the valentin/fix-dynamic-require-react-issue-in-vitest branch July 29, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:merged Run the CI jobs that normally run when merged. portable stories react

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants