WIP: CSF factories #30197
Draft
WIP: CSF factories #30197
+5,613
−1,518
Conversation
This file contains 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
…ries' into kasper/csf-factories
…ries' into kasper/csf-factories
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
20 file(s) reviewed, 19 comment(s)
Edit PR Review Bot Settings | Greptile
Comment on lines
17
to
19
| const [previewFileUrl, ...previewAnnotationURLs] = [...previewAnnotations, previewOrConfigFile] | ||
| .filter(Boolean) | ||
| .map((path) => processPreviewAnnotation(path, projectRoot)); |
There was a problem hiding this comment.
logic: Destructuring could fail if previewAnnotations and previewOrConfigFile are both empty arrays after filtering. Add a check to handle this case.
Comment on lines
26
to
29
| const preview = await import('${previewFileUrl}'); | ||
| const csfFactoryPreview = Object.values(preview).find(module => { | ||
| return 'isCSFFactoryPreview' in module | ||
| }); |
There was a problem hiding this comment.
logic: Object.values() and find() could return undefined. Need type checking and error handling for when no CSF factory preview is found.
| const configs = await Promise.all([${previewAnnotationURLs | ||
| .map( | ||
| (previewAnnotation, index) => | ||
| // Prefer the updated module from an HMR update, otherwise import the original module | ||
| `hmrPreviewAnnotationModules[${index}] ?? import('${previewAnnotation}')` | ||
| ) | ||
| .join(',\n')}]) | ||
| return composeConfigs(configs); | ||
| return composeConfigs([...configs, preview]); |
There was a problem hiding this comment.
logic: preview is used in composeConfigs even when csfFactoryPreview is found, which could cause conflicts. Should only include preview in non-factory case.
| @@ -27,11 +27,12 @@ export const testStory = ( | |||
| return async (context: TestContext & TaskContext & { story: ComposedStoryFn }) => { | |||
| const composedStory = composeStory( | |||
| story, | |||
| meta, | |||
| 'isCSFFactory' in story ? (meta as any).annotations : meta, | |||
There was a problem hiding this comment.
style: Type casting to 'any' should be avoided. Consider creating proper type definitions for CSF factories to maintain type safety.
| { initialGlobals: (await getInitialGlobals?.()) ?? {} }, | ||
| undefined, | ||
| exportName | ||
| ); | ||
|
|
||
| if (composedStory === undefined || skipTags?.some((tag) => composedStory.tags.includes(tag))) { |
There was a problem hiding this comment.
logic: Optional chaining on skipTags is unnecessary since it's a required parameter
Comment on lines
182
to
190
| const div = document.createElement('div'); | ||
| document.body.appendChild(div); | ||
|
|
||
| await CSF3InputFieldFilled.run({ canvasElement: div }); | ||
|
|
||
| const input = screen.getByTestId('input') as HTMLInputElement; | ||
| expect(input.value).toEqual('Hello world!'); | ||
|
|
||
| document.body.removeChild(div); |
There was a problem hiding this comment.
style: Test cleanup could fail if test errors before removeChild. Consider using try/finally
| import { Button } from './Button'; | ||
|
|
||
| // eslint-disable-next-line storybook/default-exports | ||
| const config = defineConfig({ args: { children: 'TODO: THIS IS NOT WORKING YET' } }); |
There was a problem hiding this comment.
logic: TODO comment indicates unfinished/broken functionality. This should be resolved before merging.
| }); | ||
|
|
||
| export const CSF2StoryWithParamsAndDecorator = meta.story({ | ||
| render: (args: any) => { |
There was a problem hiding this comment.
style: Using any type here loses type safety. Consider using proper Button component prop types.
code/renderers/react/src/preview.tsx
Outdated
Comment on lines
31
to
33
| readonly meta = <TComponent extends ComponentType<any>, TMetaArgs extends Args>( | ||
| meta: ComponentAnnotations<TRenderer, any> & { component: TComponent; args: TMetaArgs } | ||
| ) => { |
There was a problem hiding this comment.
logic: The meta method accepts any for TRenderer annotations but then casts to TMetaArgs. This could lead to type safety issues. Consider constraining the any type.
code/renderers/react/src/preview.tsx
Outdated
| public meta: Meta<TRenderer, TArgs>, | ||
| public config: PreviewConfig<TRenderer> | ||
| ) { | ||
| Object.assign(this, annotations); |
There was a problem hiding this comment.
style: Using Object.assign to copy all properties from annotations could lead to property collisions with the class's own methods/properties. Consider explicit property copying instead.
|
View your CI Pipeline Execution ↗ for commit 876fbc0.
☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 59 | 59 | 0 |
| Self size | 228 KB | 234 KB | 🚨 +6 KB 🚨 |
| Dependency size | 13.46 MB | 13.45 MB | 🎉 -10 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-backgrounds
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 4 | 4 | 0 |
| Self size | 27 KB | 44 KB | 🚨 +17 KB 🚨 |
| Dependency size | 99 KB | 99 KB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-docs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 17 | 17 | 0 |
| Self size | 2.20 MB | 2.20 MB | 🚨 +2 KB 🚨 |
| Dependency size | 7.92 MB | 7.91 MB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-essentials
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 36 | 36 | 0 |
| Self size | 12 KB | 18 KB | 🚨 +6 KB 🚨 |
| Dependency size | 13.87 MB | 13.93 MB | 🚨 +60 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-highlight
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 1 | 1 | 0 |
| Self size | 9 KB | 12 KB | 🚨 +2 KB 🚨 |
| Dependency size | 5 KB | 5 KB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-interactions
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 56 | 56 | 0 |
| Self size | 128 KB | 129 KB | 🚨 +1 KB 🚨 |
| Dependency size | 12.73 MB | 12.72 MB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-links
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 4 | 4 | 0 |
| Self size | 19 KB | 20 KB | 🚨 +1 KB 🚨 |
| Dependency size | 284 KB | 272 KB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-measure
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 2 | 0 |
| Self size | 32 KB | 60 KB | 🚨 +28 KB 🚨 |
| Dependency size | 20 KB | 20 KB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-outline
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 2 | 0 |
| Self size | 23 KB | 41 KB | 🚨 +19 KB 🚨 |
| Dependency size | 32 KB | 32 KB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-storysource
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 7 | 7 | 0 |
| Self size | 1.89 MB | 1.89 MB | 0 B |
| Dependency size | 10.74 MB | 10.73 MB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/experimental-addon-test
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 60 | 60 | 0 |
| Self size | 832 KB | 834 KB | 🚨 +2 KB 🚨 |
| Dependency size | 14.19 MB | 14.17 MB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/core
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 54 | 54 | 0 |
| Self size | 19.04 MB | 19.06 MB | 🚨 +20 KB 🚨 |
| Dependency size | 14.44 MB | 14.43 MB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/experimental-nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 153 | 153 | 0 |
| Self size | 232 KB | 234 KB | 🚨 +2 KB 🚨 |
| Dependency size | 44.74 MB | 45.53 MB | 🚨 +787 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 15 | 15 | 0 |
| Self size | 5 KB | 6 KB | 🚨 +1 KB 🚨 |
| Dependency size | 1.91 MB | 1.91 MB | 🚨 +648 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 247 | 247 | 0 |
| Self size | 6 KB | 8 KB | 🚨 +1 KB 🚨 |
| Dependency size | 31.70 MB | 31.70 MB | 🚨 +380 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 592 | 592 | 0 |
| Self size | 472 KB | 474 KB | 🚨 +1 KB 🚨 |
| Dependency size | 83.44 MB | 84.22 MB | 🚨 +786 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 245 | 245 | 0 |
| Self size | 6 KB | 8 KB | 🚨 +1 KB 🚨 |
| Dependency size | 31.27 MB | 31.27 MB | 🚨 +380 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 137 | 137 | 0 |
| Self size | 43 KB | 44 KB | 🚨 +1 KB 🚨 |
| Dependency size | 19.90 MB | 20.70 MB | 🚨 +800 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 87 | 87 | 0 |
| Self size | 13 KB | 14 KB | 🚨 +1 KB 🚨 |
| Dependency size | 16.07 MB | 16.87 MB | 🚨 +798 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 323 | 323 | 0 |
| Self size | 6 KB | 8 KB | 🚨 +1 KB 🚨 |
| Dependency size | 42.48 MB | 43.28 MB | 🚨 +798 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 255 | 255 | 0 |
| Self size | 14 KB | 15 KB | 🚨 +1 KB 🚨 |
| Dependency size | 32.68 MB | 32.66 MB | 🎉 -12 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 310 | 310 | 0 |
| Self size | 6 KB | 7 KB | 🚨 +1 KB 🚨 |
| Dependency size | 39.26 MB | 39.26 MB | 🚨 +380 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 495 | 495 | 0 |
| Self size | 6 KB | 7 KB | 🚨 +1 KB 🚨 |
| Dependency size | 55.68 MB | 55.68 MB | 🚨 +380 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 16 | 16 | 0 |
| Self size | 5 KB | 6 KB | 🚨 +1 KB 🚨 |
| Dependency size | 1.94 MB | 1.94 MB | 🚨 +648 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 245 | 245 | 0 |
| Self size | 5 KB | 7 KB | 🚨 +1 KB 🚨 |
| Dependency size | 31.32 MB | 31.32 MB | 🚨 +380 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/blocks
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 4 | 4 | 0 |
| Self size | 607 KB | 607 KB | 🚨 +18 B 🚨 |
| Dependency size | 1.53 MB | 1.52 MB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 388 | 390 | 🚨 +2 🚨 |
| Self size | 503 KB | 530 KB | 🚨 +27 KB 🚨 |
| Dependency size | 75.28 MB | 75.30 MB | 🚨 +26 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/source-loader
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 5 | 5 | 0 |
| Self size | 41 KB | 41 KB | 🚨 +18 B 🚨 |
| Dependency size | 10.68 MB | 10.67 MB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/test
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 53 | 53 | 0 |
| Self size | 1.82 MB | 1.82 MB | 🚨 +18 B 🚨 |
| Dependency size | 8.09 MB | 8.08 MB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 191 | 191 | 0 |
| Self size | 24 KB | 24 KB | 0 B |
| Dependency size | 33.54 MB | 34.34 MB | 🚨 +798 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-server-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 18 | 18 | 0 |
| Self size | 10 KB | 10 KB | 0 B |
| Dependency size | 1.48 MB | 1.47 MB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 6 | 6 | 0 |
| Self size | 932 KB | 1.73 MB | 🚨 +798 KB 🚨 |
| Dependency size | 24 KB | 24 KB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/server
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 9 | 9 | 0 |
| Self size | 13 KB | 13 KB | 🚨 +18 B 🚨 |
| Dependency size | 974 KB | 961 KB | 🎉 -13 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
…ion-exports Csf: Support named exports as functions
Maintenance: Many refactorings
…euse-codemod Codemod: Add Story.xyz to Story.input.xyz migration
…view-api Preview API: Add csf factory utilities
…rom-csf-2 CLI: Enhance csf factory codemod to migrate from csf2
…ons' into yann/addon-annotations-as-functions
…functions Add default export as function annotation from core addons
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #30166
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
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-30197-sha-a3d613d5. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-30197-sha-a3d613d5 sandboxor in an existing project withnpx storybook@0.0.0-pr-30197-sha-a3d613d5 upgrade.More information
0.0.0-pr-30197-sha-a3d613d5kasper/csf-factoriesa3d613d51738163319)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 canary-release-pr.yml --field pr=30197Greptile Summary
Based on the provided files and changes, here's a concise summary of the CSF factories pull request:
Introduces CSF (Component Story Format) factories to provide a more structured and type-safe way to define Storybook configurations and stories.
defineConfigandmeta.story()pattern for type-safe story definitions in/code/renderers/react/src/preview.tsxprocessCSFFileto detect and handle CSF4 factory format viaisCSFFactoryPreviewflagThe changes provide better type safety and structure while maintaining backward compatibility with existing CSF formats. The implementation appears solid but requires careful testing around project annotation overrides and factory pattern adoption.