Addon Vitest: Add component info to task meta#33394
Conversation
|
View your CI Pipeline Execution ↗ for commit e54f0de
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughRefactors the vitest test helper to accept a single object parameter (adding optional componentPath and componentName) and updates the transformer/tests to emit object-style calls. Large surrounding changes migrate story-source handling to a StoryIndex-driven, promise-based generator, add manifests generation/serving, and adjust related builder, watcher, and type code. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
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 (1)
code/core/src/csf-tools/vitest-plugin/transformer.ts (1)
155-166: Consider handlingImportNamespaceSpecifierfor completeness.The current implementation extracts
componentNamefromImportSpecifier(named imports) andImportDefaultSpecifier(default imports), but doesn't handleImportNamespaceSpecifier(namespace imports likeimport * as Button from './Button').While namespace imports might be rare in story files, consider whether this case should be supported for consistency.
🔎 Proposed enhancement to handle namespace imports
let componentNameLiteral = null; if ( parsed._componentImportSpecifier && (t.isImportSpecifier(parsed._componentImportSpecifier) || - t.isImportDefaultSpecifier(parsed._componentImportSpecifier)) + t.isImportDefaultSpecifier(parsed._componentImportSpecifier) || + t.isImportNamespaceSpecifier(parsed._componentImportSpecifier)) ) { componentNameLiteral = t.stringLiteral(parsed._componentImportSpecifier.local.name); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/addons/vitest/src/vitest-plugin/test-utils.ts(2 hunks)code/core/src/csf-tools/vitest-plugin/transformer.test.ts(27 hunks)code/core/src/csf-tools/vitest-plugin/transformer.ts(3 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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.test.ts
🧠 Learnings (15)
📚 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-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.test.ts
📚 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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.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/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/core/src/csf-tools/vitest-plugin/transformer.tscode/core/src/csf-tools/vitest-plugin/transformer.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/core/src/csf-tools/vitest-plugin/transformer.tscode/core/src/csf-tools/vitest-plugin/transformer.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 code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/addons/vitest/src/vitest-plugin/test-utils.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: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/csf-tools/vitest-plugin/transformer.test.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/core/src/csf-tools/vitest-plugin/transformer.test.ts
⏰ 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). (4)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: get-branch
🔇 Additional comments (6)
code/core/src/csf-tools/vitest-plugin/transformer.ts (2)
242-262: LGTM! Clean object-based API transition.The refactor from multiple positional arguments to a single object parameter improves maintainability and makes the API self-documenting. The conditional inclusion of
componentPathandcomponentNameis properly handled.
296-320: Consistent implementation across describe blocks.The same object construction pattern is correctly applied in
getDescribeStatementForStory, including the additionaltestNamefield for per-test metadata. This consistency is good for maintainability.code/addons/vitest/src/vitest-plugin/test-utils.ts (2)
34-52: LGTM! Well-structured API refactor.The migration to an object-based parameter makes the API more maintainable and easier to extend in the future. The optional fields
componentPath,testName, andcomponentNameare properly typed.
78-90: Proper metadata propagation.The component identity fields are correctly extended in the task meta type and assigned to the task object, enabling downstream consumers to access component context information.
code/core/src/csf-tools/vitest-plugin/transformer.test.ts (2)
599-649: Good test coverage for component info extraction.The new test suite covers the most common import patterns (named, default, and aliased imports). The tests verify that both
componentPathandcomponentNameare correctly extracted and included in the generated code.If you decide to implement namespace import handling (as suggested in the transformer review), consider adding a corresponding test case:
it('should extract component name from namespace import specifier', async () => { const code = ` import * as Button from './Button'; export default { component: Button, } export const Primary = {}; `; const result = await transform({ code }); expect(result.code).toContain('componentPath: "./Button"'); expect(result.code).toContain('componentName: "Button"'); });
76-82: Snapshot correctly reflects object-based API.The updated snapshots consistently show the new object structure with named properties, making the test assertions clearer and the API more self-documenting.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 181 KB | 🚨 +181 KB 🚨 |
| Dependency size | 0 B | 2.97 MB | 🚨 +2.97 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-docs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 18 | 🚨 +18 🚨 |
| Self size | 0 B | 1.64 MB | 🚨 +1.64 MB 🚨 |
| Dependency size | 0 B | 9.25 MB | 🚨 +9.25 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-links
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 14 KB | 🚨 +14 KB 🚨 |
| Dependency size | 0 B | 5 KB | 🚨 +5 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-onboarding
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 331 KB | 🚨 +331 KB 🚨 |
| Dependency size | 0 B | 670 B | 🚨 +670 B 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook-addon-pseudo-states
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 21 KB | 🚨 +21 KB 🚨 |
| Dependency size | 0 B | 689 B | 🚨 +689 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-themes
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 28 KB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-vitest
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 377 KB | 🚨 +377 KB 🚨 |
| Dependency size | 0 B | 338 KB | 🚨 +338 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 17 | 🚨 +17 🚨 |
| Self size | 0 B | 125 KB | 🚨 +125 KB 🚨 |
| Dependency size | 0 B | 2.00 MB | 🚨 +2.00 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 192 | 🚨 +192 🚨 |
| Self size | 0 B | 75 KB | 🚨 +75 KB 🚨 |
| Dependency size | 0 B | 32.17 MB | 🚨 +32.17 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 49 | 🚨 +49 🚨 |
| Self size | 0 B | 19.92 MB | 🚨 +19.92 MB 🚨 |
| Dependency size | 0 B | 16.52 MB | 🚨 +16.52 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 192 | 🚨 +192 🚨 |
| Self size | 0 B | 118 KB | 🚨 +118 KB 🚨 |
| Dependency size | 0 B | 30.40 MB | 🚨 +30.40 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 196 | 🚨 +196 🚨 |
| Self size | 0 B | 15 KB | 🚨 +15 KB 🚨 |
| Dependency size | 0 B | 28.89 MB | 🚨 +28.89 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 22 KB | 🚨 +22 KB 🚨 |
| Dependency size | 0 B | 2.16 MB | 🚨 +2.16 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 538 | 🚨 +538 🚨 |
| Self size | 0 B | 646 KB | 🚨 +646 KB 🚨 |
| Dependency size | 0 B | 59.11 MB | 🚨 +59.11 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 127 | 🚨 +127 🚨 |
| Self size | 0 B | 1.12 MB | 🚨 +1.12 MB 🚨 |
| Dependency size | 0 B | 21.78 MB | 🚨 +21.78 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 13 KB | 🚨 +13 KB 🚨 |
| Dependency size | 0 B | 2.15 MB | 🚨 +2.15 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 159 | 🚨 +159 🚨 |
| Self size | 0 B | 30 KB | 🚨 +30 KB 🚨 |
| Dependency size | 0 B | 22.96 MB | 🚨 +22.96 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 117 | 🚨 +117 🚨 |
| Self size | 0 B | 35 KB | 🚨 +35 KB 🚨 |
| Dependency size | 0 B | 19.58 MB | 🚨 +19.58 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 278 | 🚨 +278 🚨 |
| Self size | 0 B | 24 KB | 🚨 +24 KB 🚨 |
| Dependency size | 0 B | 44.06 MB | 🚨 +44.06 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 204 | 🚨 +204 🚨 |
| Self size | 0 B | 16 KB | 🚨 +16 KB 🚨 |
| Dependency size | 0 B | 33.43 MB | 🚨 +33.43 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 24 | 🚨 +24 🚨 |
| Self size | 0 B | 55 KB | 🚨 +55 KB 🚨 |
| Dependency size | 0 B | 26.82 MB | 🚨 +26.82 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 25 | 🚨 +25 🚨 |
| Self size | 0 B | 56 KB | 🚨 +56 KB 🚨 |
| Dependency size | 0 B | 26.88 MB | 🚨 +26.88 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 114 | 🚨 +114 🚨 |
| Self size | 0 B | 35 KB | 🚨 +35 KB 🚨 |
| Dependency size | 0 B | 43.98 MB | 🚨 +43.98 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 21 | 🚨 +21 🚨 |
| Self size | 0 B | 19 KB | 🚨 +19 KB 🚨 |
| Dependency size | 0 B | 2.19 MB | 🚨 +2.19 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 183 | 🚨 +183 🚨 |
| Self size | 0 B | 775 KB | 🚨 +775 KB 🚨 |
| Dependency size | 0 B | 66.97 MB | 🚨 +66.97 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 176 | 🚨 +176 🚨 |
| Self size | 0 B | 30 KB | 🚨 +30 KB 🚨 |
| Dependency size | 0 B | 65.55 MB | 🚨 +65.55 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/core-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 11 KB | 🚨 +11 KB 🚨 |
| Dependency size | 0 B | 28 KB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 50 | 🚨 +50 🚨 |
| Self size | 0 B | 999 KB | 🚨 +999 KB 🚨 |
| Dependency size | 0 B | 36.44 MB | 🚨 +36.44 MB 🚨 |
| Bundle Size Analyzer | node | node |
@storybook/csf-plugin
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 9 | 🚨 +9 🚨 |
| Self size | 0 B | 7 KB | 🚨 +7 KB 🚨 |
| Dependency size | 0 B | 1.26 MB | 🚨 +1.26 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
eslint-plugin-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 131 KB | 🚨 +131 KB 🚨 |
| Dependency size | 0 B | 2.81 MB | 🚨 +2.81 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-dom-shim
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 788 B | 🚨 +788 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-create-react-app
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 68 | 🚨 +68 🚨 |
| Self size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Dependency size | 0 B | 5.98 MB | 🚨 +5.98 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 170 | 🚨 +170 🚨 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 31.20 MB | 🚨 +31.20 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-server-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 10 | 🚨 +10 🚨 |
| Self size | 0 B | 7 KB | 🚨 +7 KB 🚨 |
| Dependency size | 0 B | 1.20 MB | 🚨 +1.20 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 29 KB | 🚨 +29 KB 🚨 |
| Dependency size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 16 KB | 🚨 +16 KB 🚨 |
| Dependency size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 57 | 🚨 +57 🚨 |
| Self size | 0 B | 717 KB | 🚨 +717 KB 🚨 |
| Dependency size | 0 B | 12.92 MB | 🚨 +12.92 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 8 KB | 🚨 +8 KB 🚨 |
| Dependency size | 0 B | 716 KB | 🚨 +716 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 45 KB | 🚨 +45 KB 🚨 |
| Dependency size | 0 B | 230 KB | 🚨 +230 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 55 KB | 🚨 +55 KB 🚨 |
| Dependency size | 0 B | 211 KB | 🚨 +211 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 41 KB | 🚨 +41 KB 🚨 |
| Dependency size | 0 B | 47 KB | 🚨 +47 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
yannbf
force-pushed
the
yann/improved-vitest-meta
branch
from
98576ce to
e54f0de
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
code/lib/react-dom-shim/package.json (1)
51-51: Invalid character in gitHead hash.The
gitHeadvalue ends with"1l"(one followed by lowercase L), but Git commit hashes must be hexadecimal (0-9, a-f only). The character"l"is invalid. This appears to be a typo—possibly"11"was mistyped as"1l".While this won't break builds, it represents incorrect metadata.
code/lib/cli-storybook/package.json (1)
67-67: Invalid character in gitHead hash.The
gitHeadvalue ends with1l, but lowercase 'l' is not a valid hexadecimal character. Git commit hashes should only contain 0-9 and a-f. This appears to be a typo and should be corrected to avoid potential publishing issues.code/lib/csf-plugin/package.json (1)
69-69: Invalid git commit hash in gitHead field.The gitHead value ends with "1l" (one followed by lowercase L), but git commit hashes are hexadecimal strings that can only contain characters 0-9 and a-f. The letter 'l' is not a valid hexadecimal character. This could break tooling that validates or parses this field during publishing or version tracking.
Please correct this to use a valid git commit hash.
🔎 How to fix
Replace the invalid hash with the correct git commit hash:
- "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae1l" + "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae10"Or verify and use the actual current commit hash:
#!/bin/bash # Get the current git commit hash git rev-parse HEADcode/presets/react-webpack/package.json (1)
69-69: Invalid Git SHA in gitHead field.The
gitHeadvalue contains an invalid character. Git commit SHAs must be exactly 40 hexadecimal characters (0-9, a-f), but this value is 41 characters long and ends with 'l' (lowercase L), which is not a valid hex digit. This could cause package publishing failures or break version tracking tools.🔎 Expected format
The gitHead should be a valid 40-character git commit SHA, for example:
- "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae1l" + "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae10"Or verify the correct commit SHA for this release:
#!/bin/bash # Get the current HEAD commit SHA git rev-parse HEADcode/lib/core-webpack/package.json (1)
52-52: Invalid git commit hash in gitHead field.The
gitHeadvalue ends with "1l", but git commit hashes are hexadecimal and can only contain characters 0-9 and a-f. The "l" character is invalid and appears to be a typo.🔎 Suggested fix
- "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae1l" + "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae11"Note: If this field is auto-generated during publishing, it will likely be corrected automatically. However, it should be fixed in the source to avoid potential tooling issues.
code/frameworks/react-vite/package.json (1)
76-76: Invalid character in gitHead field.The
gitHeadvalue ends with"1l"(one followed by lowercase L). Git commit hashes are hexadecimal and only contain characters 0-9 and a-f. The lowercase 'l' is not a valid hex digit. This should be corrected to a valid git commit hash.🔎 Verification script
#!/bin/bash # Description: Check if gitHead is a valid git commit hash # Extract the gitHead value git_head="a8e7fd8a655c69780bc20b9749d2699e45beae1l" # Check if it's a valid hex string (should only contain 0-9, a-f) if echo "$git_head" | grep -qE '^[0-9a-f]{40}$'; then echo "✓ Valid git commit hash" else echo "✗ Invalid git commit hash: contains non-hex characters" echo "Hash: $git_head" echo "Last 5 chars: ${git_head: -5}" fi # Try to verify if this commit exists in the repository if git cat-file -t "$git_head" 2>/dev/null; then echo "✓ Commit exists in repository" else echo "✗ Commit does not exist in repository" ficode/lib/eslint-plugin/package.json (1)
70-70: Invalid character in gitHead hash.The
gitHeadvalue ends with"l", which is not a valid hexadecimal character for a git commit hash. Git SHA-1 hashes only contain characters0-9anda-f.This appears to be a typo—possibly the trailing character should be
"1"instead of"l", or the hash may need regeneration.🔎 Verify and correct the git commit hash
If this is meant to reference a real commit, verify the correct hash with:
#!/bin/bash # Check recent commits to find the correct hash git log --oneline -20 --format="%H %s"The hash should be exactly 40 hexadecimal characters (0-9, a-f).
code/builders/builder-webpack5/package.json (1)
91-91: Fix invalid gitHead hash format.The gitHead value ends with an invalid character 'l' (lowercase L). Git commit hashes must contain only hexadecimal characters (0-9 and a-f). Verify and correct this hash to a valid 40-character commit hash.
code/frameworks/preact-vite/package.json (1)
65-65: Fix the gitHead field — it contains an invalid hexadecimal character.The
gitHeadvalue "a8e7fd8a655c69780bc20b9749d2699e45beae1l" is 41 characters long and ends with 'l', but git commit SHAs must be exactly 40 hexadecimal characters (0-9, a-f only). The trailing 'l' is not a valid hex digit and indicates a typo or corruption. Correct this before publishing.code/presets/create-react-app/package.json (1)
53-53: Fix the invalid gitHead value.The
gitHeadfield contains an invalid git SHA:"a8e7fd8a655c69780bc20b9749d2699e45beae1l". Git commit hashes are 40-character hexadecimal strings (0-9 and a-f), but this value has 41 characters and ends with"1l"(digit one followed by lowercase letter 'l'). The character 'l' is not valid hexadecimal.This field is populated by npm publish to point to the git HEAD SHA-1 at publish time. While gitHead is normally added automatically by npm and is useful to map packages back to a revision in the git repository, the invalid placeholder should be corrected or removed before publishing.
code/core/src/core-server/utils/manifests/render-components-manifest.ts (1)
712-712: Unused expression result.The
esc(c.error?.message || 'Unknown error')call result is not assigned or used. This appears to be dead code, possibly left over from a refactor.🔎 Proposed fix
- esc(c.error?.message || 'Unknown error'); return `
🧹 Nitpick comments (14)
storybook.code-workspace (1)
11-13: Consider more targeted file association for JSX files.Mapping all
*.jsfiles tojavascriptreactmay affect non-React JavaScript files (config files, build scripts, Node.js utilities). If the project contains JavaScript files without JSX, consider:
- Using
*.jsxextension for React component files, or- Accepting this trade-off if the vast majority of
.jsfiles contain JSXAlternative approach if JSX files can be renamed
If feasible, remove the broad association and use the
.jsxextension for React components:- "files.associations": { - "*.js": "javascriptreact", - },This provides more accurate language support for each file type.
code/builders/builder-vite/src/codegen-importfn-script.test.ts (1)
18-20: Missingspy: trueoption on mocks.Per coding guidelines,
vi.mock()should include thespy: trueoption for consistent mocking patterns. Thees-toolkit/functionmock correctly uses{ spy: true }, butwatchpackandstorybook/internal/node-loggerdo not.🔎 Proposed fix
-vi.mock('watchpack'); +vi.mock('watchpack', { spy: true }); vi.mock('es-toolkit/function', { spy: true }); -vi.mock('storybook/internal/node-logger'); +vi.mock('storybook/internal/node-logger', { spy: true });Based on coding guidelines for Vitest tests.
code/core/src/core-server/utils/manifests/render-components-manifest.ts (1)
862-864: Duplicate comment.The comment "// Prefer raw if provided" appears twice on consecutive lines.
🔎 Proposed fix
// Prefer raw if provided - // Prefer raw if provided if ('raw' in tsType && typeof tsType.raw === 'string' && tsType.raw.trim().length > 0) {code/core/src/core-server/utils/doTelemetry.ts (1)
19-22: Redundant optional chaining.Since
storyIndexGeneratorPromisenow resolves toStoryIndexGenerator(notStoryIndexGenerator | undefined), the optional chaining ongenerator?.getIndexAndStats()is unnecessary.🔎 Proposed fix
const generator = await storyIndexGeneratorPromise; let indexAndStats; try { - indexAndStats = await generator?.getIndexAndStats(); + indexAndStats = await generator.getIndexAndStats(); } catch (err) {code/core/src/core-server/utils/index-json.test.ts (1)
18-20: Missingspy: trueoption on some mocks.For consistency with the coding guidelines and the
es-toolkit/functionmock, addspy: trueto the other mocks.🔎 Proposed fix
-vi.mock('watchpack'); +vi.mock('watchpack', { spy: true }); vi.mock('es-toolkit/function', { spy: true }); -vi.mock('storybook/internal/node-logger'); +vi.mock('storybook/internal/node-logger', { spy: true });Based on coding guidelines for Vitest tests.
code/core/src/core-server/dev-server.ts (1)
172-173: Potentially redundant optional chaining.Since
storyIndexGeneratorPromiseresolves toStoryIndexGenerator(per the type at line 47), the optional chaining ongenerator?.getIndexAndStats()may be unnecessary. However, within a catch block for telemetry, defensive coding is reasonable.🔎 Proposed fix (if strict typing is desired)
const generator = await storyIndexGeneratorPromise; - const indexAndStats = await generator?.getIndexAndStats(); + const indexAndStats = await generator.getIndexAndStats();code/renderers/react/src/componentManifest/generator.ts (1)
53-85: Good per-story error isolation inextractStories.The try/catch around individual story extraction ensures one failing story doesn't prevent others from being processed. The error objects include both name and message for debugging.
Minor note: The
invariant(e instanceof Error)on line 78 will throw if a non-Error value is caught. While rare, this could mask the original error. Consider using a safer pattern:🔎 Suggested defensive handling
} catch (e) { - invariant(e instanceof Error); + const error = e instanceof Error ? e : new Error(String(e)); return { name: storyName, - error: { name: e.name, message: e.message }, + error: { name: error.name, message: error.message }, }; }code/core/src/core-server/utils/manifests/manifests.ts (1)
49-65: Consider setting Content-Type for 404 responses on the JSON route.The JSON route correctly sets
Content-Type: application/jsonfor success responses but omits it for 404 responses. For consistency with the HTML route (which sets Content-Type for 404), consider adding the header:🔎 Suggested improvement
} else { res.statusCode = 404; + res.setHeader('Content-Type', 'text/plain; charset=utf-8'); res.end(`Manifest "${req.params.name}" not found`); }code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
798-807: Consider defensive error handling for listener notifications.The listener notification loop at line 806 will propagate exceptions from any listener, potentially leaving other listeners unnotified. Consider wrapping each listener invocation in try-catch to ensure all listeners are notified even if one fails.
🔎 Proposed defensive wrapper
- this.invalidationListeners.forEach((listener) => listener()); + this.invalidationListeners.forEach((listener) => { + try { + listener(); + } catch (err) { + logger.error('Error in invalidation listener:', err); + } + });code/builders/builder-vite/src/plugins/code-generator-plugin.ts (1)
69-89: LGTM with minor observation: Consistent async handling.The switch-based approach cleanly separates the handling of different virtual modules. However, note that the
iframeIdcase at lines 84-88 usesreadFileSync(which is actually async in this context via the existingreadFileSyncimport from node:fs), but the import at line 1 shows it's from 'node:fs' which is synchronous. Consider whether async file reading would be more consistent.Optional: Use async file reading for consistency
+import { readFile } from 'node:fs/promises'; -import { readFileSync } from 'node:fs'; case iframeId: { - return readFileSync( + return await readFile( fileURLToPath(importMetaResolve('@storybook/builder-vite/input/iframe.html')), 'utf-8' ); }code/core/src/core-server/utils/watch-story-specifiers.test.ts (1)
23-35: Timer setup and teardown are appropriate for batched watcher behaviorUsing fake timers in setup, a shared
flushEventshelper, and restoring real timers plus closing the watcher in teardown gives deterministic coverage of the new batching logic without leaking timers or watchers across tests.Also applies to: 91-95
code/core/src/csf-tools/vitest-plugin/transformer.test.ts (1)
599-649: Component info extraction coverage is focused and usefulThe new tests validating
componentPathandcomponentNamefor named, default, and aliased imports give good confidence that the transformer computes the correct metadata for typical React component imports.code/addons/vitest/src/vitest-plugin/test-utils.ts (1)
34-52: testStory object API and meta extensions look consistentThe switch to a single options object and the addition of
componentPath/componentNameontask.metaalign with the updated transformer output and give downstream consumers access to the new metadata without changing runtime behavior for existing callers. If you want to keepmetaminimal, you could optionally only assign the component fields when they are defined, but it is not required.Also applies to: 77-83, 89-90
code/builders/builder-vite/src/virtual-file-names.ts (1)
7-15: Virtual file ID helpers align with Vite-style resolutionExposing
SB_VIRTUAL_FILE_IDSand using a simple leading-character slice to recover the original ID from the resolved form is consistent with\0-prefixed virtual ids and should work cleanly as long as callers only pass strings produced bygetResolvedVirtualModuleId.
yannbf
force-pushed
the
yann/ghost-stories-research
branch
from
badd420 to
5a0395e
Compare
2 participants
Closes #
What I did
This PR introduces two new properties in the Vitest task meta: componentPath and componentName.
They are quite useful for post-processing code e.g. to analyze a vitest test run and group test results, which could be coming from multiple story files but relating to the same component file:
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
Breaking Changes
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.