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

Addon-docs: Fix react-dom/server imports breaking stories and docs #26557

Merged
merged 19 commits into from
Mar 24, 2024

Conversation

JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Mar 18, 2024

Closes #26532

To do

  • Fix E2E docs tests for resolved react version
  • Add a story showing the resolved react version

What I did

  • Added an explicit alias to react-dom/server, so Vite wouldn't resolve that to PATH/react-dom/server.js (which just redirects to server.node.js but instead PATH/react-dom/server.browser.js.
  • Improved tests to ensure React is correct in stories and autodocs too, and that they are the same version.

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

  1. Create a sandbox
  2. At the top of Button.stories.ts, add import * as ReactDomServer from 'react-dom/server';
  3. See that the story still works.

A. Try in a React-based and non-React sandbox
B. ... and try across Yarn, Yarn PnP and pnpm as well

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/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-26557-sha-7290a2c2. 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-26557-sha-7290a2c2
Triggered by @JReinhold
Repository storybookjs/storybook
Branch jeppe/fix-react-dom-server-resolution
Commit 7290a2c2
Datetime Mon Mar 18 15:14:28 UTC 2024 (1710774868)
Workflow run 8329053228

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

@JReinhold JReinhold self-assigned this Mar 18, 2024
@JReinhold JReinhold added bug addon: docs ci:merged Run the CI jobs that normally run when merged. ci:daily Run the CI jobs that normally run in the daily job. builder-vite labels Mar 18, 2024
@JReinhold JReinhold changed the title add explicit react-dom/server alias for vite Addon-docs: Fix react-dom/server imports breaking stories and docs Mar 18, 2024
Copy link

nx-cloud bot commented Mar 18, 2024

☁️ Nx Cloud Report

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

@JReinhold JReinhold removed the ci:daily Run the CI jobs that normally run in the daily job. label Mar 20, 2024
@JReinhold JReinhold marked this pull request as ready for review March 21, 2024 07:48
…orybookjs/storybook into jeppe/fix-react-dom-server-resolution
@JReinhold JReinhold added ci:merged Run the CI jobs that normally run when merged. and removed ci:merged Run the CI jobs that normally run when merged. labels Mar 21, 2024
@JReinhold JReinhold added the ci:daily Run the CI jobs that normally run in the daily job. label Mar 21, 2024
@JReinhold JReinhold removed the ci:merged Run the CI jobs that normally run when merged. label Mar 21, 2024
@JReinhold JReinhold merged commit d6eaf19 into next Mar 24, 2024
95 of 111 checks passed
@JReinhold JReinhold deleted the jeppe/fix-react-dom-server-resolution branch March 24, 2024 22:19
@github-actions github-actions bot mentioned this pull request Mar 24, 2024
22 tasks
@JReinhold JReinhold added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Mar 24, 2024
storybook-bot pushed a commit that referenced this pull request Mar 25, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
@github-actions github-actions bot mentioned this pull request Mar 25, 2024
9 tasks
storybook-bot pushed a commit that referenced this pull request Mar 25, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 25, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 25, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 25, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 25, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 26, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 26, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 26, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 26, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 26, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
storybook-bot pushed a commit that referenced this pull request Mar 26, 2024
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: docs bug builder-vite ci:daily Run the CI jobs that normally run in the daily job. patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: 8.0.0-alpha.4 broke stories that use ag-grid-react
3 participants