Eval: Add sync-storybook-version harness + --skip-automigrations upgrade flag#34612
Merged
yannbf merged 4 commits intoApr 28, 2026
Merged
Conversation
Adds `scripts/eval/sync-storybook-version.ts`, which bumps every benchmark repo to a given Storybook version (stable or PR canary) in one pass: clone missing repos, fail fast on dirty trees, run `npx storybook@<version> upgrade --skip-automigrations` from each repo root, then commit and push the resulting dep bumps. To keep those commits free of source rewrites, adds a `--skip-automigrations` flag to `storybook upgrade` that bypasses `runAutomigrations` entirely while still producing a clean upgrade (package.json + lockfile only).
|
View your CI Pipeline Execution ↗ for commit 448d4d4
☁️ Nx Cloud last updated this comment at |
The default `runUpgrade` and `installProjectDeps` hooks were `const` arrow expressions declared below the top-level `if (esMain(...))` block. When the script is invoked via `node`, the CLI path calls `syncStorybookVersion(...)` while the module body is still executing, hitting the temporal dead zone. Tests didn't catch this because `import` evaluates the whole module first. Switch both to hoisted `async function` declarations so the CLI entry can reach them regardless of source order.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 186 | 186 | 0 |
| Self size | 76 KB | 76 KB | 🎉 -48 B 🎉 |
| Dependency size | 32.75 MB | 32.78 MB | 🚨 +31 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 140 KB | 140 KB | 0 B |
| Dependency size | 30.41 MB | 30.44 MB | 🚨 +31 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 190 | 190 | 0 |
| Self size | 15 KB | 15 KB | 0 B |
| Dependency size | 29.47 MB | 29.50 MB | 🚨 +31 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 535 | 535 | 0 |
| Self size | 651 KB | 650 KB | 🎉 -240 B 🎉 |
| Dependency size | 60.77 MB | 60.80 MB | 🚨 +31 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 273 | 273 | 0 |
| Self size | 23 KB | 23 KB | 0 B |
| Dependency size | 45.38 MB | 45.41 MB | 🚨 +31 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 16 KB | 16 KB | 🚨 +12 B 🚨 |
| Dependency size | 34.01 MB | 34.04 MB | 🚨 +31 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 184 | 184 | 0 |
| Self size | 819 KB | 819 KB | 🚨 +286 B 🚨 |
| Dependency size | 68.22 MB | 68.24 MB | 🚨 +24 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 177 | 177 | 0 |
| Self size | 32 KB | 32 KB | 🚨 +36 B 🚨 |
| Dependency size | 66.74 MB | 66.76 MB | 🚨 +24 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 166 | 166 | 0 |
| Self size | 18 KB | 18 KB | 0 B |
| Dependency size | 31.75 MB | 31.78 MB | 🚨 +31 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
When sync-storybook-version is first run with --skip-push, a second run without that flag should publish the already-created local upgrade commit instead of reporting a no-op and requiring a manual git push in every benchmark repo. Teach the no-diff path to detect when the repo is ahead of origin and, if pushing is enabled, push that existing commit. Add a regression test for the two-step skip-push -> push workflow and document it in the eval README.
8 tasks
kasperpeulen
changed the title
--skip-automigrations upgrade flag
8 tasks
Contributor
📝 WalkthroughWalkthroughThis PR adds a Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI/User
participant Sync as syncStorybookVersion
participant Git as Git Repo
participant Deps as Dependency Manager
participant Upgrade as Upgrade Command
CLI->>Sync: sync-storybook-version --version X --project Y
loop For each selected project
Sync->>Git: Clone/ensure repo exists
Sync->>Git: Checkout target branch
Sync->>Git: Pull origin (--ff-only)
Sync->>Git: Check for dirty state
Sync->>Deps: installProjectDeps()
Sync->>Upgrade: runUpgrade(version, configDir)
Sync->>Deps: reinstall dependencies
Sync->>Git: Stage changes
alt No staged changes
Sync->>Git: Push if local ahead of origin
else Changes detected
Sync->>Git: Commit with version message
Sync->>Git: Conditionally push to origin
end
end
Sync->>CLI: Return results (commit SHAs, changed status)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/eval/sync-storybook-version.test.ts (1)
451-453: Consider using Node.jsmkdirSyncfor cross-platform compatibility.The shell
mkdir -pcommand works on Unix systems but not Windows. UsingmkdirSyncwith therecursiveoption would be more portable.♻️ Proposed refactor
+import { mkdirSync } from 'node:fs'; + function mkdirSyncRecursive(path: string) { - execFileSync('mkdir', ['-p', path]); + mkdirSync(path, { recursive: true }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/eval/sync-storybook-version.test.ts` around lines 451 - 453, The helper function mkdirSyncRecursive currently shells out with execFileSync('mkdir', ['-p', path]) which is not cross-platform; update the function mkdirSyncRecursive to use Node's fs.mkdirSync(path, { recursive: true }) instead and ensure fs is imported (require or import fs) so the function works on Windows and Unix without invoking a shell.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/lib/cli-storybook/src/upgrade.ts`:
- Around line 417-427: The signal handler closure handleInterruption captures
the outer automigrationResults/detectedAutomigrations but the inner block
redeclares them with let, causing shadowing and empty telemetry; fix by removing
the inner declarations and assigning into the outer variables instead (either
change the outer declaration to let so you can reassign the result of
runAutomigrations(storybookProjects, options) to automigrationResults and
detectedAutomigrations, or keep outer const and mutate it with Object.assign
into automigrationResults and set detectedAutomigrations accordingly), and
ensure the code path that skips automigrations still leaves the outer variables
in the correct state so handleInterruption sees the real results.
---
Nitpick comments:
In `@scripts/eval/sync-storybook-version.test.ts`:
- Around line 451-453: The helper function mkdirSyncRecursive currently shells
out with execFileSync('mkdir', ['-p', path]) which is not cross-platform; update
the function mkdirSyncRecursive to use Node's fs.mkdirSync(path, { recursive:
true }) instead and ensure fs is imported (require or import fs) so the function
works on Windows and Unix without invoking a shell.
🪄 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: f657669c-c6de-446a-8c5d-52595a951ab0
📒 Files selected for processing (7)
code/lib/cli-storybook/src/bin/run.tscode/lib/cli-storybook/src/upgrade.tsscripts/eval/README.mdscripts/eval/lib/utils.tsscripts/eval/sync-storybook-version.test.tsscripts/eval/sync-storybook-version.tsscripts/package.json
Comment on lines
+417
to
+427
| // Run automigrations for all projects (unless explicitly skipped) | ||
| let automigrationResults: Record<string, AutomigrationResult> = {}; | ||
| let detectedAutomigrations: AutomigrationCheckResult[] = []; | ||
| if (options.skipAutomigrations) { | ||
| logger.log('Skipping automigrations (--skip-automigrations).'); | ||
| } else { | ||
| ({ automigrationResults, detectedAutomigrations } = await runAutomigrations( | ||
| storybookProjects, | ||
| options | ||
| )); | ||
| } |
Contributor
There was a problem hiding this comment.
Variable shadowing causes signal handler to always see empty automigration results.
Line 341 declares const automigrationResults in the outer scope, which is captured by the handleInterruption closure (line 348). The new let automigrationResults at line 418 creates a shadowing variable in the inner scope.
If a user interrupts (SIGINT/SIGTERM) after automigrations complete, the signal handler will send telemetry with empty automigrationResults because it references the outer empty object, not the inner populated one.
🐛 Proposed fix: Remove shadowing by reassigning the outer variable
- // Run automigrations for all projects (unless explicitly skipped)
- let automigrationResults: Record<string, AutomigrationResult> = {};
- let detectedAutomigrations: AutomigrationCheckResult[] = [];
- if (options.skipAutomigrations) {
+ // Run automigrations for all projects (unless explicitly skipped)
+ let detectedAutomigrations: AutomigrationCheckResult[] = [];
+ if (options.skipAutomigrations) {
logger.log('Skipping automigrations (--skip-automigrations).');
} else {
- ({ automigrationResults, detectedAutomigrations } = await runAutomigrations(
+ const result = await runAutomigrations(
storybookProjects,
options
- ));
+ );
+ Object.assign(automigrationResults, result.automigrationResults);
+ detectedAutomigrations = result.detectedAutomigrations;
}Also change line 341 from const to let to allow reassignment, or use Object.assign as shown above to mutate the existing object in place.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/lib/cli-storybook/src/upgrade.ts` around lines 417 - 427, The signal
handler closure handleInterruption captures the outer
automigrationResults/detectedAutomigrations but the inner block redeclares them
with let, causing shadowing and empty telemetry; fix by removing the inner
declarations and assigning into the outer variables instead (either change the
outer declaration to let so you can reassign the result of
runAutomigrations(storybookProjects, options) to automigrationResults and
detectedAutomigrations, or keep outer const and mutate it with Object.assign
into automigrationResults and set detectedAutomigrations accordingly), and
ensure the code path that skips automigrations still leaves the outer variables
in the correct state so handleInterruption sees the real results.
yannbf
approved these changes
Apr 27, 2026
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
What I did
This PR adds a helper for updating the benchmark repositories used by the eval suite to a chosen Storybook version.
scripts/eval/sync-storybook-version.ts, exposed asyarn eval:sync-storybook-version.package.jsoneven when.storybooklives in a subpackage.--skip-automigrationstostorybook upgrade, which keeps these benchmark-repo commits focused on dependency changes while still running the rest of the upgrade flow.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
scripts/eval/sync-storybook-version.test.tscovers install/upgrade ordering, no-op runs when the target version is already installed, dirty-tree failure, auto-cloning missing repos, and push behavior.Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
node scripts/eval/sync-storybook-version.ts --version 0.0.0-pr-34612-sha-d8de7419 --project mealdropfrom the repository root.$REPOS_DIR/mealdropand confirm the script updates Storybook dependencies, creates a single upgrade commit, and pushes it.--project mealdropto apply the same update to the full benchmark set.Documentation
MIGRATION.MD
Updated
scripts/eval/README.mdwith the new sync workflow.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.🦋 Canary release
This pull request has been released as version
0.0.0-pr-34612-sha-d8de7419. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34612-sha-d8de7419 sandboxor in an existing project withnpx storybook@0.0.0-pr-34612-sha-d8de7419 upgrade.More information
0.0.0-pr-34612-sha-d8de7419kasper/eval-sync-storybook-versiond8de74191776862565)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=34612