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

CLI: Add CSF3 to CSF4 migration #30194

Merged
merged 5 commits into from
Jan 10, 2025
Merged

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Jan 6, 2025

Closes #

What I did

This is a tentative approach to migrating CSF3 to CSF4, having as a prerequisite that have already migrated from CSF1/2 to CSF3.

npx storybook migrate csf-3-to-4 --glob="**/*.stories.*"

This PR also introduces a step in the sandbox creation which runs codemods only for a react-vite sandbox, so it turns its base stories (the ones used in E2E) to CSF4 so we can fully test them in our E2E suite.

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>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.8 MB 77.8 MB 0 B -1.45 0%
initSize 134 MB 134 MB 0 B 0.23 0%
diffSize 55.8 MB 55.8 MB 0 B 0.27 0%
buildSize 7.19 MB 7.19 MB 0 B 0.48 0%
buildSbAddonsSize 1.85 MB 1.85 MB 0 B 0.96 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.87 MB 1.87 MB 0 B - 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.91 MB 3.91 MB 0 B 0.96 0%
buildPreviewSize 3.28 MB 3.28 MB 0 B 0.46 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 24.1s 18.1s -5s -950ms -0.31 -32.7%
generateTime 18.4s 20.3s 1.9s -0.34 9.4%
initTime 13s 13.7s 754ms -0.4 5.5%
buildTime 9.2s 8.4s -755ms -0.86 -8.9%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 4.4s 7.4s 3s 5.27 🔺41.1%
devManagerResponsive 3.2s 5.3s 2s 5 🔺38.6%
devManagerHeaderVisible 511ms 888ms 377ms 4.09 🔺42.5%
devManagerIndexVisible 518ms 913ms 395ms 3.78 🔺43.3%
devStoryVisibleUncached 1.8s 2.7s 953ms 4.79 🔺34.5%
devStoryVisible 544ms 912ms 368ms 3.87 🔺40.4%
devAutodocsVisible 442ms 1s 603ms 8.69 🔺57.7%
devMDXVisible 424ms 963ms 539ms 2.96 🔺56%
buildManagerHeaderVisible 499ms 895ms 396ms 4.72 🔺44.2%
buildManagerIndexVisible 500ms 899ms 399ms 4.71 🔺44.4%
buildStoryVisible 492ms 849ms 357ms 4.3 🔺42%
buildAutodocsVisible 446ms 769ms 323ms 6.05 🔺42%
buildMDXVisible 422ms 651ms 229ms 3.71 🔺35.2%

Greptile Summary

Based on the provided files, I'll create a concise summary of the changes in this PR:

Adds a new codemod to transform Storybook stories from CSF3 to CSF4 format, introducing factory functions for meta and story definitions.

  • Added csf-3-to-4.ts transform that automatically adds config import from '#.storybook/preview'
  • Added transformation logic to wrap meta objects with config.meta() calls
  • Added story object wrapping with meta.story() calls
  • Added comprehensive test suite covering JavaScript and TypeScript scenarios
  • Updated package.json to include new transform in exports and bundler entries

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.

3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +171 to +178
// Generate the transformed code
const { code } = babel.transformFromAstSync(fileNode, info.source, {
parserOpts: { sourceType: 'module' },
});
return code;
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: No error handling around transformFromAstSync. Should catch potential transform failures and provide meaningful errors.

Comment on lines +7 to +10
expect.addSnapshotSerializer({
serialize: (val: any) => (typeof val === 'string' ? val : val.toString()),
test: () => true,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: This serializer always returns true and could potentially cause issues with unexpected values. Consider adding type checking or more specific test conditions.

Copy link

nx-cloud bot commented Jan 6, 2025

View your CI Pipeline Execution ↗ for commit 76b1b6c.

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

☁️ Nx Cloud last updated this comment at 2025-01-10 08:44:10 UTC

@yannbf yannbf force-pushed the yann/csf3-to-4-codemod branch from eea150a to 23bcf14 Compare January 10, 2025 08:17
@yannbf yannbf changed the title CLI: Make a start on defineConfig, meta and story factory CLI: Add CSF3 to CSF4 migration Jan 10, 2025
@yannbf yannbf merged commit 4d2b108 into kasper/csf-factories Jan 10, 2025
26 of 30 checks passed
@yannbf yannbf deleted the yann/csf3-to-4-codemod branch January 10, 2025 13:17
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