Skip to content

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Sep 5, 2025

Closes #

What I did

This PR makes the csf factories codemod bail out on stories that were only half transformed, to avoid having a mixed csf format in the file and therefore a broken storybook index.
It warns the user regarding which file was not transformed and which stories were in a non supported format

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 canary-release-pr.yml --field pr=<PR_NUMBER>

Greptile Summary

Updated On: 2025-09-05 18:02:05 UTC

This PR improves error handling in the CSF (Component Story Format) factories codemod to prevent partially transformed story files that would break Storybook's indexer. The changes implement a comprehensive tracking system that monitors which stories are successfully transformed during the migration from CSF 2 to CSF 3 format.

The core improvement is in story-to-csf-factory.ts, where the codemod now:

  1. Tracks all detected stories in a file using csf.stories
  2. Records successfully transformed story exports in a Set<string>
  3. Compares the count of detected stories versus transformed stories after processing
  4. If the counts don't match (indicating some stories couldn't be transformed), it bails out entirely and returns the original source unchanged
  5. Warns users with specific information about which file was skipped and which stories were detected

This addresses a critical issue where the codemod could leave files in a mixed state - some stories using the new meta.story() factory syntax while others remained in the old format. Such mixed formatting breaks Storybook's story indexer, which expects consistent CSF format throughout a file. The solution ensures files remain in a working state and provides clear feedback about which stories need manual intervention.

The implementation includes comprehensive test coverage that validates the bailout behavior, mocks the logger to verify warning messages, and tests scenarios with mixed transformable/non-transformable stories.

Confidence score: 4/5

  • This PR addresses a critical bug that could break Storybook functionality during migrations
  • The tracking logic is well-implemented with clear validation and appropriate fallback behavior
  • Pay close attention to the transformation tracking logic in story-to-csf-factory.ts to ensure all transformation paths correctly update the transformedStoryExports Set

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.

2 files reviewed, 1 comment

Edit Code Review Bot Settings | Greptile

`);

expect(transformed).toContain('A = meta.story');
expect(transformed).toContain('B = meta.story');
// @TODO: when we support these, uncomment this line
// expect(transformed).toContain('C = meta.story');
});
it('should not complete transformation if no stories are not transformed', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: Test description has double negative 'should not complete transformation if no stories are not transformed' - should be 'if not all stories are transformed'

Suggested change
it('should not complete transformation if no stories are not transformed', async () => {
it('should not complete transformation if not all stories are transformed', async () => {

Copy link

nx-cloud bot commented Sep 5, 2025

View your CI Pipeline Execution ↗ for commit 46d8c6b

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 13s View ↗

☁️ Nx Cloud last updated this comment at 2025-09-05 18:24:39 UTC

@storybook-pr-benchmarking
Copy link

Package Benchmarks

Commit: 46d8c6b, ran on 5 September 2025 at 18:11:08 UTC

The following packages have significant changes to their size or dependencies:

@storybook/nextjs

Before After Difference
Dependency count 538 538 0
Self size 904 KB 904 KB 0 B
Dependency size 59.37 MB 59.35 MB 🎉 -17 KB 🎉
Bundle Size Analyzer Link Link

@storybook/nextjs-vite

Before After Difference
Dependency count 130 130 0
Self size 3.08 MB 3.08 MB 0 B
Dependency size 22.32 MB 22.31 MB 🎉 -16 KB 🎉
Bundle Size Analyzer Link Link

@storybook/react-native-web-vite

Before After Difference
Dependency count 163 163 0
Self size 32 KB 32 KB 0 B
Dependency size 23.71 MB 23.69 MB 🎉 -16 KB 🎉
Bundle Size Analyzer Link Link

@storybook/react-vite

Before After Difference
Dependency count 120 120 0
Self size 32 KB 32 KB 0 B
Dependency size 20.27 MB 20.25 MB 🎉 -16 KB 🎉
Bundle Size Analyzer Link Link

@storybook/react-webpack5

Before After Difference
Dependency count 286 286 0
Self size 25 KB 25 KB 0 B
Dependency size 43.79 MB 43.77 MB 🎉 -16 KB 🎉
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 219 219 0
Self size 587 KB 588 KB 🚨 +617 B 🚨
Dependency size 97.06 MB 97.04 MB 🎉 -16 KB 🎉
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 188 188 0
Self size 31 KB 31 KB 0 B
Dependency size 81.13 MB 81.11 MB 🎉 -16 KB 🎉
Bundle Size Analyzer Link Link

@storybook/preset-react-webpack

Before After Difference
Dependency count 177 177 0
Self size 24 KB 24 KB 0 B
Dependency size 30.63 MB 30.61 MB 🎉 -16 KB 🎉
Bundle Size Analyzer Link Link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant