Skip to content

Conversation

@yannbf
Copy link
Member

@yannbf yannbf commented May 23, 2025

Closes #31517

What I did

This PR completely revamps our upgrade process to introduce better mono repository and introduces a visually improved CLI experience for the upgrade.

Improved upgrade experience for mono-repositories

Storybook's upgrade CLI (npx storybook@<next|latest> upgrade) should now always be executed in your project root. The CLI automatically detects all of your Storybook projects. If the Storybook projects are truly encapsulated, you can choose to upgrade one by one. Otherwise, we will upgrade all of your Storybooks at once.

image

Upgrade: Slimed down messaging and collecting results

The previous upgrade experience was packed with a lot of messages and links. Also, the upgrade CLI was shining in a variety of colors. We have reduced messaging to a minimum and outsourced debugging messages, warnings and errors to a debug log.

image

Additionally: We have also introduced new flags to introduce different log levels and to write all upgrade logs to a log file:

  • --write-logs: Write all debug logs to a file at the end of the run
  • --loglevel: <trace | debug | info | warn | error | silent> Define log level (default: "info")

Improved the doctor integration inside of upgrade

The doctor command was fine-tuned to collect results from multiple projects simultaneously. A user upgrading multiple Storybooks at once will get exact information of what kind of conflicts they have for a particular Storybook.

image

Introducing the technical foundation to adjust other parts of our CLI

We have revived the @storybook/node-logger package to encapsulate tooling for logging. Via a simple mechanism, we can switch the internal implementation of our loggers and prompt functions from the prompt and boxen library to the new modern package clack.

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-31557-sha-a944b962. 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-31557-sha-a944b962
Triggered by @yannbf
Repository storybookjs/storybook
Branch valentin/monorepo-enhancements
Commit a944b962
Datetime Tue Jun 10 14:52:13 UTC 2025 (1749567133)
Workflow run 15562859477

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

Greptile Summary

Comprehensive overhaul of Storybook's upgrade process to improve monorepo support and CLI experience, focusing on centralized logging and user interaction improvements.

  • Revamped upgrade CLI to detect and manage multiple Storybook instances in monorepos through a new @storybook/node-logger package
  • Streamlined CLI messaging with configurable log levels (trace/debug/info/warn/error/silent) and optional log file output via --write-logs flag
  • Enhanced doctor command to collect and display diagnostics from multiple projects simultaneously
  • Introduced new Clack-based prompts system with improved visual feedback and error handling
  • Added monorepo-aware package management with intelligent project root detection and dependency resolution

@yannbf yannbf marked this pull request as ready for review June 10, 2025 13:54
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.

192 files reviewed, 33 comments
Edit PR Review Bot Settings | Greptile

Comment on lines +15 to +22
const cleanLog = (str) => {
const pattern = [
'[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]+)*|[a-zA-Z\\d]+(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)',
'(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))',
].join('|');

return str.replace(new RegExp(pattern, 'g'), '');
};
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider making the regex pattern a constant to avoid recompiling it on every function call. This would improve performance when cleanLog is called frequently.

Suggested change
const cleanLog = (str) => {
const pattern = [
'[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]+)*|[a-zA-Z\\d]+(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)',
'(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))',
].join('|');
return str.replace(new RegExp(pattern, 'g'), '');
};
const ANSI_PATTERN = new RegExp([
'[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]+)*|[a-zA-Z\\d]+(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)',
'(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))',
].join('|'), 'g');
const cleanLog = (str) => {
return str.replace(ANSI_PATTERN, '');
};

Comment on lines 4 to 7
import {
getIncompatiblePackagesSummary,
getIncompatibleStorybookPackages,
} from '../../../../lib/cli-storybook/src/doctor/getIncompatibleStorybookPackages';
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider using direct imports instead of importing through 4 directory levels with '../../../../'

Comment on lines 80 to 84
const storyFilePath =
doesStoryFileExist(join(cwd, dir), storyFileName) && componentExportCount > 1
? join(cwd, dir, alternativeStoryFileNameWithExtension)
: join(cwd, dir, storyFileNameWithExtension);
doesStoryFileExist(join(getProjectRoot(), dir), storyFileName) && componentExportCount > 1
? join(getProjectRoot(), dir, alternativeStoryFileNameWithExtension)
: join(getProjectRoot(), dir, storyFileNameWithExtension);

Copy link
Contributor

Choose a reason for hiding this comment

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

style: Repetitive getProjectRoot() calls could be optimized by storing result in a variable

Suggested change
const storyFilePath =
doesStoryFileExist(join(cwd, dir), storyFileName) && componentExportCount > 1
? join(cwd, dir, alternativeStoryFileNameWithExtension)
: join(cwd, dir, storyFileNameWithExtension);
doesStoryFileExist(join(getProjectRoot(), dir), storyFileName) && componentExportCount > 1
? join(getProjectRoot(), dir, alternativeStoryFileNameWithExtension)
: join(getProjectRoot(), dir, storyFileNameWithExtension);
const projectRoot = getProjectRoot();
const storyFilePath =
doesStoryFileExist(join(projectRoot, dir), storyFileName) && componentExportCount > 1
? join(projectRoot, dir, alternativeStoryFileNameWithExtension)
: join(projectRoot, dir, storyFileNameWithExtension);

TaskLogOptions,
TextPromptOptions,
} from './prompt-provider-base';
import { asyncLocalStorage } from './storage';
Copy link
Contributor

Choose a reason for hiding this comment

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

style: asyncLocalStorage is imported but never used

Comment on lines +155 to +156
const allDependencies = packageManager.getAllDependencies();
const { packageJson } = packageManager.primaryPackageJson;
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: getAllDependencies() called without await but appears to be synchronous now - verify this change is intentional

Comment on lines +20 to 21
const configFile = join(getProjectRoot(), monorepoConfigs[monorepo]);
return existsSync(configFile);
Copy link
Contributor

Choose a reason for hiding this comment

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

style: getProjectRoot() called multiple times may introduce overhead if expensive. Consider storing result in a variable.

Comment on lines +34 to +36
function getEnvFromTerminal(key: string): string {
return execaSync('echo', [`$${key}`], { shell: true }).stdout.trim();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Shell command execution can fail in restricted environments. Consider using process.env directly as a safer fallback.

Comment on lines 145 to 151
it('throws an error if command output is not a valid JSON', async () => {
vi.spyOn(yarn1Proxy, 'executeCommand').mockResolvedValueOnce('NOT A JSON');
vi.spyOn(yarn1Proxy, 'executeCommand').mockReturnValue(
Promise.resolve({ stdout: 'NOT A JSON' }) as any
);

await expect(yarn1Proxy.latestVersion('storybook')).rejects.toThrow();
await expect(yarn1Proxy.latestVersion('storybook')).resolves.toBe(null);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Error handling behavior changed from throwing to returning null. Ensure this change was intentional and documented, as it affects error handling in dependent code

Suggested change
it('throws an error if command output is not a valid JSON', async () => {
vi.spyOn(yarn1Proxy, 'executeCommand').mockResolvedValueOnce('NOT A JSON');
vi.spyOn(yarn1Proxy, 'executeCommand').mockReturnValue(
Promise.resolve({ stdout: 'NOT A JSON' }) as any
);
await expect(yarn1Proxy.latestVersion('storybook')).rejects.toThrow();
await expect(yarn1Proxy.latestVersion('storybook')).resolves.toBe(null);
});
it('returns null if command output is not a valid JSON', async () => {
vi.spyOn(yarn1Proxy, 'executeCommand').mockReturnValue(
Promise.resolve({ stdout: 'NOT A JSON' }) as any
);
await expect(yarn1Proxy.latestVersion('storybook')).resolves.toBe(null);
});

migration: any,
{ glob, dryRun, list, rename, parser, configDir: userSpecifiedConfigDir }: CLIOptions
) {
export async function migrate(migration: any, { glob, dryRun, list, rename, parser }: CLIOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: The configDir option is defined in CLIOptions but not destructured in the function parameters

Comment on lines +108 to +110
if (level === 'prompt') {
level = 'info';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Mutating level parameter can cause unexpected behavior. Consider using a new variable

@yannbf yannbf merged commit 9c2f2fd into next Jun 10, 2025
63 checks passed
@yannbf yannbf deleted the valentin/monorepo-enhancements branch June 10, 2025 14:55
@github-actions github-actions bot mentioned this pull request Jun 10, 2025
7 tasks
@valentinpalkovic valentinpalkovic added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Jun 17, 2025
@github-actions github-actions bot mentioned this pull request Jun 17, 2025
15 tasks
valentinpalkovic pushed a commit that referenced this pull request Jun 17, 2025
…ents

CLI: Improve support for upgrading Storybook in monorepos
(cherry picked from commit 9c2f2fd)
@github-actions github-actions bot mentioned this pull request Jun 18, 2025
15 tasks
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jun 18, 2025
@yannbf yannbf added the needs qa Indicates that this needs manual QA during the upcoming minor/major release label Jul 25, 2025
@ndelangen
Copy link
Member

@valentinpalkovic can you please add a manual testing section, so we can do QA?

@valentinpalkovic
Copy link
Contributor

This feature was already properly QAed for 9.1. It was missed to remove the needs QA after the release.

@valentinpalkovic valentinpalkovic removed the needs qa Indicates that this needs manual QA during the upcoming minor/major release label Oct 1, 2025
@ndelangen ndelangen removed the patch:yes Bugfix & documentation PR that need to be picked to main branch label Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:merged Run the CI jobs that normally run when merged. cli feature request monorepos Monorepo support patch:done Patch/release PRs already cherry-picked to main/release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Tracking]: Monorepo-ready Automigrations

4 participants