Vue3: CSF factories support #33365
Vue3: CSF factories support #33365
Conversation
|
View your CI Pipeline Execution ↗ for commit 6e5a012
☁️ Nx Cloud last updated this comment at |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughChanged CSF story/test dispatch and story detection logic; tightened React preview meta/story typings and tests; added a new Vue 3 preview surface with strongly-typed meta/story APIs and associated tests and re-exports. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
Possibly related PRs
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
code/renderers/vue3/src/preview.tsx (2)
20-20: Use type-only import forComponentProps.
ComponentPropsfromvue-component-type-helpersappears to be used only as a type utility. Usingimport typeensures it's erased at compile time and prevents potential bundling issues.-import { ComponentProps } from 'vue-component-type-helpers'; +import type { ComponentProps } from 'vue-component-type-helpers';
24-24: Use type-only import forVueTypes.
VueTypesis only used in type positions (type parameters, extends clauses). Usingimport typeis more appropriate.-import { VueTypes } from './types'; +import type { VueTypes } from './types';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
code/core/src/csf/csf-factories.ts(1 hunks)code/core/src/preview-api/modules/store/csf/processCSFFile.ts(2 hunks)code/renderers/react/src/csf-factories.test.tsx(2 hunks)code/renderers/react/src/preview.tsx(4 hunks)code/renderers/vue3/src/preview.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/renderers/vue3/src/preview.tsxcode/renderers/react/src/csf-factories.test.tsxcode/core/src/csf/csf-factories.tscode/renderers/react/src/preview.tsxcode/core/src/preview-api/modules/store/csf/processCSFFile.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/renderers/vue3/src/preview.tsxcode/renderers/react/src/csf-factories.test.tsxcode/core/src/csf/csf-factories.tscode/renderers/react/src/preview.tsxcode/core/src/preview-api/modules/store/csf/processCSFFile.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/vue3/src/preview.tsxcode/renderers/react/src/csf-factories.test.tsxcode/core/src/csf/csf-factories.tscode/renderers/react/src/preview.tsxcode/core/src/preview-api/modules/store/csf/processCSFFile.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/renderers/vue3/src/preview.tsxcode/renderers/react/src/csf-factories.test.tsxcode/core/src/csf/csf-factories.tscode/renderers/react/src/preview.tsxcode/core/src/preview-api/modules/store/csf/processCSFFile.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/renderers/vue3/src/preview.tsxcode/renderers/react/src/csf-factories.test.tsxcode/core/src/csf/csf-factories.tscode/renderers/react/src/preview.tsxcode/core/src/preview-api/modules/store/csf/processCSFFile.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/renderers/react/src/csf-factories.test.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/renderers/react/src/csf-factories.test.tsx
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/renderers/react/src/csf-factories.test.tsx
🧠 Learnings (16)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/renderers/vue3/src/preview.tsxcode/renderers/react/src/preview.tsxcode/core/src/preview-api/modules/store/csf/processCSFFile.ts
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/renderers/vue3/src/preview.tsxcode/renderers/react/src/preview.tsx
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/renderers/react/src/csf-factories.test.tsxcode/renderers/react/src/preview.tsx
📚 Learning: 2025-11-05T09:36:55.944Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Applied to files:
code/renderers/react/src/csf-factories.test.tsx
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/core/src/csf/csf-factories.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/core/src/csf/csf-factories.ts
🧬 Code graph analysis (4)
code/renderers/vue3/src/preview.tsx (1)
code/core/src/csf/csf-factories.ts (5)
PreviewAddon(64-65)InferTypes(34-36)Meta(77-96)story(114-118)Story(122-154)
code/core/src/csf/csf-factories.ts (1)
code/frameworks/angular/src/client/types.ts (1)
StoryContext(49-49)
code/renderers/react/src/preview.tsx (1)
code/core/src/csf/csf-factories.ts (1)
story(114-118)
code/core/src/preview-api/modules/store/csf/processCSFFile.ts (2)
code/core/src/csf/csf-factories.ts (1)
isStory(156-158)code/core/src/core-server/presets/common-preset.ts (1)
title(136-137)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (8)
code/core/src/preview-api/modules/store/csf/processCSFFile.ts (1)
52-88: LGTM! Clean separation between CSF4 factory stories and legacy CSF exports.The refactored logic properly:
- Identifies factory stories using
isStory<TRenderer>type guard- Validates exports with both
isExportStoryandisStorychecks- Sources meta configuration from
factoryStory.meta.input- Falls back gracefully to legacy CSF processing when no factory story is found
code/renderers/react/src/csf-factories.test.tsx (2)
388-403: LGTM! Good test coverage formeta.input.playcomposition.The test correctly validates that:
- A play function defined at the meta level can be accessed via
meta.input.play- Extended stories can compose the meta-level play with their own interactions
- Optional chaining is used appropriately since
playmay be undefined
79-81: LGTM! Updated test to validate the new variadic overload.The change from
meta.story({})tometa.story()properly tests the new zero-argument story invocation pattern, verifying it correctly fails when required args are not provided.code/renderers/vue3/src/preview.tsx (2)
26-35: LGTM! Vue preview factory follows established patterns.The implementation correctly:
- Prepends Vue-specific annotations before user addons
- Uses the base
definePreviewBasefrom storybook internals- Casts to
VuePreviewtype for proper type inferenceThis mirrors the React implementation pattern appropriately.
119-120: Consider whetherVueStoryneeds additional properties likeReactStory.
ReactStory(incode/renderers/react/src/preview.tsx) includes aComponentproperty:export interface ReactStory<...> extends Story<T, TInput> { Component: ComponentType<Partial<T['args']>>; }If Vue stories should expose a similar construct for portable story compatibility, this interface may need to be extended. Otherwise, if intentionally minimal, this is fine as-is.
code/renderers/react/src/preview.tsx (2)
80-93: LGTM! Improved type inference for meta args.The refactored return type using
Omit<..., 'args'> & { args: ... }pattern provides clearer type boundaries:
- Omits the original
argsfromComponentAnnotations- Re-attaches a computed
argstype that is{}when meta provides all required args, orTMetaArgsotherwiseThis enables better TypeScript inference for story definitions.
128-135: LGTM! Clever variadic overload for conditional zero-argument stories.The conditional type check elegantly enforces that
story()can only be called without arguments when all required args are satisfied by the meta:..._args: Partial<T['args']> extends SetOptional<...> ? [] : [never]
- Returns
[](empty tuple) when valid → allowsstory()with no args- Returns
[never]when invalid → makes overload unmatchableThis provides excellent developer experience with proper type errors.
code/core/src/csf/csf-factories.ts (1)
205-216: The code is correct — no destructuring issue exists.The
StoryContextinterface defines acontext: thisproperty (line 262) that is a self-reference to the full context object. When destructuring{ mount, context }fromStoryContext<TRenderer>, both properties are extracted correctly:mount(the function) andcontext(the full context via self-reference). Both code branches pass the same complete context object tothis.playandtestFunction, making them functionally equivalent. Themountdestructuring is intentional — themountDestructuredutility checks if a function parameter includesmount, requiring the context to be destructured in this form formountto be accessible as a named parameter.
There was a problem hiding this comment.
Pull request overview
This PR adds CSF (Component Story Format) factories support for Vue3, bringing it to feature parity with the React renderer. The changes introduce a new preview.tsx file for Vue3 with factory functions and type definitions, improve the CSF file processing logic to better detect factory stories, and add support for explicit mount destructuring in test functions.
Key Changes
- New CSF factories API for Vue3 with
__definePreview,VuePreview,VueMeta, andVueStoryinterfaces - Enhanced
processCSFFileto use.find()for locating factory stories and add additional validation - Fixed mount parameter handling in test functions by explicitly destructuring it
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| code/renderers/vue3/src/preview.tsx | Introduces new CSF factories support for Vue3 with preview, meta, and story type definitions |
| code/renderers/react/src/preview.tsx | Refactors type definitions for better consistency and adds third story overload for no-args cases |
| code/renderers/react/src/csf-factories.test.tsx | Removes optional parameter syntax and adds test for meta.input.play composition |
| code/core/src/preview-api/modules/store/csf/processCSFFile.ts | Improves factory story detection using .find() and adds explicit isStory() check |
| code/core/src/csf/csf-factories.ts | Explicitly destructures mount parameter in test play functions to satisfy linting |
|
|
||
| import * as vueAnnotations from './entry-preview'; | ||
| import * as vueDocsAnnotations from './entry-preview-docs'; | ||
| import { VueTypes } from './types'; |
There was a problem hiding this comment.
The import statement references VueTypes which does not exist in the types.ts file. The types.ts file only exports VueRenderer.
Looking at the React implementation, it has a ReactTypes interface that extends ReactRenderer with additional parameters. Vue3 needs a similar VueTypes type/interface to be defined in types.ts, or this code should use VueRenderer instead.
If following the React pattern, you would need to add to types.ts:
export interface VueTypes extends VueRenderer {
// Add any Vue-specific parameters here if needed
}Alternatively, if no additional types are needed, replace all references to VueTypes with VueRenderer throughout this file.
| export function __definePreview<Addons extends PreviewAddon<never>[]>( | ||
| input: { addons: Addons } & ProjectAnnotations<VueTypes & InferTypes<Addons>> | ||
| ): VuePreview<VueTypes & InferTypes<Addons>> { | ||
| const preview = definePreviewBase({ | ||
| ...input, | ||
| addons: [vueAnnotations, vueDocsAnnotations, ...(input.addons ?? [])], | ||
| }) as unknown as VuePreview<VueTypes & InferTypes<Addons>>; | ||
|
|
||
| return preview; | ||
| } | ||
|
|
||
| /** @ts-expect-error We cannot implement the meta faithfully here, but that is okay. */ | ||
| export interface VuePreview<T extends AddonTypes> extends Preview<VueTypes & T> { | ||
| meta< | ||
| C, | ||
| Decorators extends DecoratorFunction<VueTypes & T, any>, | ||
| // Try to make Exact<Partial<TArgs>, TMetaArgs> work | ||
| TMetaArgs extends Partial<ComponentProps<C>>, | ||
| >( | ||
| meta: { | ||
| render?: ArgsStoryFn<VueTypes & T, ComponentProps<C>>; | ||
| component?: C; | ||
| decorators?: Decorators | Decorators[]; | ||
| args?: TMetaArgs; | ||
| } & Omit< | ||
| ComponentAnnotations<VueTypes & T, ComponentProps<C>>, | ||
| 'decorators' | 'component' | 'args' | 'render' | ||
| > | ||
| ): VueMeta< | ||
| VueTypes & | ||
| T & { | ||
| args: Simplify< | ||
| ComponentProps<C> & | ||
| Simplify<RemoveIndexSignature<DecoratorsArgs<VueTypes & T, Decorators>>> | ||
| >; | ||
| }, | ||
| Omit< | ||
| ComponentAnnotations< | ||
| VueTypes & | ||
| T & { | ||
| args: Simplify< | ||
| ComponentProps<C> & | ||
| Simplify<RemoveIndexSignature<DecoratorsArgs<VueTypes & T, Decorators>>> | ||
| >; | ||
| } | ||
| >, | ||
| 'args' | ||
| > & { | ||
| args: Partial<ComponentProps<C>> extends TMetaArgs ? {} : TMetaArgs; | ||
| } | ||
| >; | ||
| } | ||
|
|
||
| type DecoratorsArgs<TRenderer extends Renderer, Decorators> = UnionToIntersection< | ||
| Decorators extends DecoratorFunction<TRenderer, infer TArgs> ? TArgs : unknown | ||
| >; | ||
|
|
||
| export interface VueMeta<T extends VueTypes, MetaInput extends ComponentAnnotations<T>> | ||
| /** @ts-expect-error hard */ | ||
| extends Meta<T, MetaInput> { | ||
| // Required args don't need to be provided when the user uses an empty render | ||
| story< | ||
| TInput extends | ||
| | (() => VueTypes['storyResult']) | ||
| | (StoryAnnotations<T, T['args']> & { | ||
| render: () => VueTypes['storyResult']; | ||
| }), | ||
| >( | ||
| story: TInput | ||
| ): VueStory<T, TInput extends () => VueTypes['storyResult'] ? { render: TInput } : TInput>; | ||
|
|
||
| story< | ||
| TInput extends Simplify< | ||
| StoryAnnotations< | ||
| T, | ||
| T['args'], | ||
| SetOptional<T['args'], keyof T['args'] & keyof MetaInput['args']> | ||
| > | ||
| >, | ||
| >( | ||
| story: TInput | ||
| ): VueStory<T, TInput>; | ||
|
|
||
| story( | ||
| ..._args: Partial<T['args']> extends SetOptional< | ||
| T['args'], | ||
| keyof T['args'] & keyof MetaInput['args'] | ||
| > | ||
| ? [] | ||
| : [never] | ||
| ): VueStory<T, {}>; | ||
| } | ||
|
|
||
| export interface VueStory<T extends VueTypes, TInput extends StoryAnnotations<T, T['args']>> | ||
| extends Story<T, TInput> {} |
There was a problem hiding this comment.
This PR adds CSF factories support for Vue3 by introducing a new preview.tsx file with __definePreview, VuePreview, VueMeta, and VueStory interfaces. However, there are no corresponding test files to verify this functionality works correctly.
The React renderer has comprehensive tests in csf-factories.test.tsx that verify:
- Args can be provided in multiple ways
- Type safety for required vs optional args
- Story composition and inheritance
- Decorator args inference
- Play function composition (as shown in the test added to React)
Vue3 should have similar test coverage to ensure the CSF factories implementation works correctly and maintains API consistency with React. Consider adding a csf-factories.test.ts file for Vue3 that mirrors the React test structure.
| }); | ||
| // @ts-expect-error disabled not provided ❌ | ||
| const Basic = meta.story({}); | ||
| const Basic = meta.story(); |
There was a problem hiding this comment.
Unused variable Basic.
| const ExtendedInteractionsStory = meta.story({ | ||
| play: async ({ canvas, ...rest }) => { | ||
| await meta.input.play?.({ canvas, ...rest }); | ||
|
|
||
| /** Do some extra interactions */ | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Unused variable ExtendedInteractionsStory.
| const ExtendedInteractionsStory = meta.story({ | |
| play: async ({ canvas, ...rest }) => { | |
| await meta.input.play?.({ canvas, ...rest }); | |
| /** Do some extra interactions */ | |
| }, | |
| }); |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 127 | 128 | 🚨 +1 🚨 |
| Self size | 1.12 MB | 1.12 MB | 🎉 -7 B 🎉 |
| Dependency size | 21.97 MB | 21.96 MB | 🎉 -10 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 159 | 160 | 🚨 +1 🚨 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 23.15 MB | 23.14 MB | 🎉 -10 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 117 | 118 | 🚨 +1 🚨 |
| Self size | 35 KB | 35 KB | 🎉 -10 B 🎉 |
| Dependency size | 19.76 MB | 19.75 MB | 🎉 -10 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
code/renderers/vue3/src/csf-factories.test.ts (2)
42-47: Consider using underscore prefix for intentionally unused variables.Several variables like
meta(line 46) are declared solely for type-checking purposes. To avoid potential lint warnings and signal intent, consider prefixing with underscore.it('Generic parameter of Meta can be a component', () => { - const meta = preview.meta({ + const _meta = preview.meta({ component: Button, args: { label: 'good', disabled: false }, }); });Similar pattern applies to other intentionally unused declarations (e.g.,
Story,Basicvariables throughout the file).
177-184: Consider adding runtime assertion to the mount test.This test only validates types but doesn't execute the play function. If runtime behavior matters, consider adding an integration test that actually calls
play.For type-only validation, this is sufficient. If you want to also verify runtime behavior in the future, you could mark this with a TODO comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code/renderers/vue3/src/csf-factories.test.ts(1 hunks)code/renderers/vue3/src/index.ts(1 hunks)code/renderers/vue3/src/preview.tsx(1 hunks)code/renderers/vue3/src/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/renderers/vue3/src/preview.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/renderers/vue3/src/types.tscode/renderers/vue3/src/index.tscode/renderers/vue3/src/csf-factories.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/renderers/vue3/src/types.tscode/renderers/vue3/src/index.tscode/renderers/vue3/src/csf-factories.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/vue3/src/types.tscode/renderers/vue3/src/index.tscode/renderers/vue3/src/csf-factories.test.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/renderers/vue3/src/types.tscode/renderers/vue3/src/index.tscode/renderers/vue3/src/csf-factories.test.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/renderers/vue3/src/types.tscode/renderers/vue3/src/index.tscode/renderers/vue3/src/csf-factories.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/renderers/vue3/src/csf-factories.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/renderers/vue3/src/csf-factories.test.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/renderers/vue3/src/csf-factories.test.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/renderers/vue3/src/index.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/renderers/vue3/src/index.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/renderers/vue3/src/csf-factories.test.ts
🧬 Code graph analysis (1)
code/renderers/vue3/src/csf-factories.test.ts (2)
code/renderers/vue3/src/preview.tsx (1)
__definePreview(26-35)code/core/src/csf/csf-factories.ts (2)
test(197-227)Story(122-154)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (4)
code/renderers/vue3/src/types.ts (1)
36-37: LGTM!The empty interface pattern for
VueTypesextendingVueRendereris appropriate for creating a public type alias that can be extended in the future without breaking changes.code/renderers/vue3/src/index.ts (1)
6-7: LGTM!The re-export follows the existing barrel export pattern and properly exposes the new Vue3 preview API surface.
code/renderers/vue3/src/csf-factories.test.ts (2)
1-2: LGTM!Good practice documenting upfront that this file primarily validates TypeScript types. The use of
expectTypeOffrom Vitest is appropriate for compile-time type assertions.
85-101: Good use of@ts-expect-errorfor negative type testing.The pattern of using
@ts-expect-errorto verify that TypeScript correctly rejects invalid configurations is effective for ensuring type safety. This validates that the type system catches missing required args.
Closes #
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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.