Skip to content

Conversation

@JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Dec 3, 2025

Follow-up to #33194

What I did

Svelte and Vite ecosystem CIs where failing (eg. https://github.com/sveltejs/svelte-ecosystem-ci/actions/runs/19788151506/job/56697448056), due to us now creating sandboxes outside of the root. when you then run yarn set version berry in that sandbox dir, Yarn will apparently ignore the sandbox' package.json and find the git root's (ecosystem CI) package.json, which specifies pnpm as the package manager.
The fix is to create the sandboxes nested inside the Storybook git root, as it was before.

I tested this by cloning the Svelte ecosystem CI down locally and running its pnpm run test storybook, pointing to this branch.

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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

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

Summary by CodeRabbit

  • Chores

    • Enhanced CI and build infrastructure for ecosystem testing with improved sandbox environment configuration.
    • Streamlined path resolution for sandbox package management during integration tests.
  • Style

    • Updated installation status messages for clearer feedback during setup processes.

✏️ Tip: You can customize this high-level summary in your review settings.

@JReinhold JReinhold self-assigned this Dec 3, 2025
@yannbf yannbf added build Internal-facing build tooling & test updates ci:docs Run the CI jobs for documentation checks only. labels Dec 3, 2025
@nx-cloud
Copy link

nx-cloud bot commented Dec 3, 2025

View your CI Pipeline Execution ↗ for commit a9db713

Command Status Duration Result
nx run-many -t compile --parallel=3 ✅ Succeeded 43s View ↗

☁️ Nx Cloud last updated this comment at 2025-12-03 13:32:05 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

📝 Walkthrough

Walkthrough

Updates CI ecosystem scripts to introduce STORYBOOK_SANDBOX_ROOT environment variable and adjust sandbox paths from ../../../storybook-sandboxes/ to ../../storybook-sandboxes/. Updates Yarn installation user-facing messages from "Yarn 2" to "Yarn". Reorganizes task execution contexts for svelte and vite ecosystem CI workflows.

Changes

Cohort / File(s) Summary
CI Ecosystem Scripts
package.json
Added svelte-ecosystem-ci:before-test and vite-ecosystem-ci:before-test scripts; updated svelte-ecosystem-ci:build, svelte-ecosystem-ci:test, vite-ecosystem-ci:build, and vite-ecosystem-ci:test to include STORYBOOK_SANDBOX_ROOT=./storybook-sandboxes environment variable and adjust task execution contexts and start-from references for sandbox workflows.
Sandbox Path Resolution
scripts/ecosystem-ci/before-test.js
Updated relative path for sandbox package.json resolution from ../../../storybook-sandboxes/ to ../../storybook-sandboxes/.
Installation Messages
scripts/utils/yarn.ts
Updated user-facing messages for Yarn installation from "Installing Yarn 2" / "Installing Yarn 2 failed" to "Installing Yarn" / "Installing Yarn failed".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify path adjustments in before-test.js match the new directory structure
  • Confirm STORYBOOK_SANDBOX_ROOT environment variable is consistently applied across all ecosystem CI scripts
  • Validate task start-from references are correct for each ecosystem variant (svelte vs. vite)

Possibly related PRs

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a60c55c and a9db713.

📒 Files selected for processing (3)
  • package.json (1 hunks)
  • scripts/ecosystem-ci/before-test.js (1 hunks)
  • scripts/utils/yarn.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use ESLint and Prettier configurations that are enforced in the codebase

Files:

  • scripts/utils/yarn.ts
  • scripts/ecosystem-ci/before-test.js
  • package.json
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode

Files:

  • scripts/utils/yarn.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Use Yarn 4.9.1 as the package manager

Applied to files:

  • scripts/utils/yarn.ts
  • package.json
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.

Applied to files:

  • scripts/ecosystem-ci/before-test.js
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.

Applied to files:

  • scripts/ecosystem-ci/before-test.js
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.

Applied to files:

  • scripts/ecosystem-ci/before-test.js
  • package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
scripts/utils/yarn.ts (1)

75-76: LGTM!

The message update removes outdated "Yarn 2" terminology. Since the repository uses Yarn 4.x and yarn set version berry installs Berry (which includes versions 2-4), the generic "Installing Yarn" message is more accurate.

scripts/ecosystem-ci/before-test.js (1)

22-22: LGTM! Core fix for the sandbox path issue.

This change correctly moves the sandbox location from outside the git root (../../../storybook-sandboxes/) to inside the root (../../storybook-sandboxes/). This ensures that yarn set version berry respects the sandbox's package.json rather than falling back to the repository root's package.json.

package.json (1)

29-38: Environment variable is properly consumed by task runner; path concern is unfounded.

The STORYBOOK_SANDBOX_ROOT environment variable is indeed consumed by the task runner and other scripts (task.ts, prepare-sandbox.ts, upload-bench.ts, etc.) via scripts/utils/constants.ts. The constants module correctly handles both absolute and relative paths: relative paths are joined with ROOT_DIRECTORY (the repository root), not the current working directory.

The path consistency concern on line 29 is invalid. After the cd command, the environment variable is set before yarn install, but yarn install does not consume STORYBOOK_SANDBOX_ROOT. The variable is only used by Node.js scripts (like the task runner), which read it in the context of the repository root through the constants module's path resolution logic, not relative to the current working directory.


Comment @coderabbitai help to get the list of available commands and usage tips.

@JReinhold JReinhold merged commit 3517e92 into next Dec 3, 2025
24 of 25 checks passed
@JReinhold JReinhold deleted the fix-ecosystem-ci branch December 3, 2025 13:32
@coderabbitai coderabbitai bot mentioned this pull request Dec 4, 2025
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Internal-facing build tooling & test updates ci:docs Run the CI jobs for documentation checks only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants