React: Experimental code examples #32813
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughMakes CSF enrichment asynchronous, adds an experimentalCodeExamples feature and experimental_enrichCsf preset hook, implements React renderer code-snippet generation/injection into CSF, restructures JSDoc parsing to {description, tags}, and wires enrichment through Vite/Webpack/Rollup, presets, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Build as Build System (Vite/Webpack/Rollup)
participant Presets as Preset Layer
participant Core as core.enrichCsf
participant Renderer as react.enrichCsf
participant Codegen as Code Generator
participant CSF as CSF AST
Build->>Presets: load presets (may expose experimental_enrichCsf)
Presets-->>Build: provide enrichCsf hook (if present)
Build->>Core: await enrichCsf(csf, csfSource, { enrichCsf })
Core->>Renderer: invoke optional renderer enrichCsf hook (async)
alt experimentalCodeExamples enabled
Renderer->>Codegen: generate formatted snippet per story (async)
Codegen-->>Renderer: formatted snippet
Renderer->>CSF: inject snippet into story.input.parameters.docs.source.code
else feature disabled or no hook
Renderer->>CSF: return CSF unchanged
end
Core-->>Build: return enriched CSF
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
code/renderers/react/src/enrichCsf.ts (1)
22-33: Track the TODO for user config support.The TODO on Line 29 indicates that prettier formatting should respect user configuration. Consider creating an issue to track this enhancement.
Do you want me to open a new issue to track the user config support for code formatting?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
code/.storybook/main.ts(1 hunks)code/addons/docs/src/preset.ts(3 hunks)code/builders/builder-vite/src/plugins/csf-plugin.ts(1 hunks)code/core/src/common/presets.ts(2 hunks)code/core/src/common/utils/formatter.ts(1 hunks)code/core/src/csf-tools/enrichCsf.ts(2 hunks)code/core/src/types/modules/core-common.ts(4 hunks)code/lib/csf-plugin/src/rollup-based-plugin.ts(1 hunks)code/lib/csf-plugin/src/webpack-loader.ts(1 hunks)code/renderers/react/src/componentManifest/generator.test.ts(2 hunks)code/renderers/react/src/componentManifest/generator.ts(2 hunks)code/renderers/react/src/componentManifest/jsdocTags.test.ts(2 hunks)code/renderers/react/src/componentManifest/jsdocTags.ts(1 hunks)code/renderers/react/src/docs/jsxDecorator.tsx(2 hunks)code/renderers/react/src/enrichCsf.test.ts(1 hunks)code/renderers/react/src/enrichCsf.ts(1 hunks)code/renderers/react/src/entry-preview-docs.ts(1 hunks)code/renderers/react/src/preset.ts(1 hunks)code/renderers/react/vitest.config.ts(1 hunks)code/renderers/react/vitest.setup.ts(1 hunks)code/vitest-setup.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/core/src/common/utils/formatter.tscode/renderers/react/src/preset.tscode/renderers/react/vitest.setup.tscode/renderers/react/vitest.config.tscode/renderers/react/src/componentManifest/jsdocTags.tscode/vitest-setup.tscode/addons/docs/src/preset.tscode/renderers/react/src/enrichCsf.test.tscode/core/src/csf-tools/enrichCsf.tscode/renderers/react/src/entry-preview-docs.tscode/lib/csf-plugin/src/webpack-loader.tscode/renderers/react/src/componentManifest/generator.test.tscode/lib/csf-plugin/src/rollup-based-plugin.tscode/renderers/react/src/enrichCsf.tscode/renderers/react/src/componentManifest/jsdocTags.test.tscode/builders/builder-vite/src/plugins/csf-plugin.tscode/core/src/common/presets.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/types/modules/core-common.tscode/renderers/react/src/docs/jsxDecorator.tsx
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/core/src/common/utils/formatter.tscode/renderers/react/src/preset.tscode/renderers/react/vitest.setup.tscode/renderers/react/vitest.config.tscode/renderers/react/src/componentManifest/jsdocTags.tscode/vitest-setup.tscode/addons/docs/src/preset.tscode/renderers/react/src/enrichCsf.test.tscode/core/src/csf-tools/enrichCsf.tscode/renderers/react/src/entry-preview-docs.tscode/lib/csf-plugin/src/webpack-loader.tscode/renderers/react/src/componentManifest/generator.test.tscode/lib/csf-plugin/src/rollup-based-plugin.tscode/renderers/react/src/enrichCsf.tscode/renderers/react/src/componentManifest/jsdocTags.test.tscode/builders/builder-vite/src/plugins/csf-plugin.tscode/core/src/common/presets.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/types/modules/core-common.tscode/renderers/react/src/docs/jsxDecorator.tsx
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/core/src/common/utils/formatter.tscode/renderers/react/src/preset.tscode/renderers/react/vitest.setup.tscode/renderers/react/vitest.config.tscode/renderers/react/src/componentManifest/jsdocTags.tscode/vitest-setup.tscode/addons/docs/src/preset.tscode/renderers/react/src/enrichCsf.test.tscode/core/src/csf-tools/enrichCsf.tscode/renderers/react/src/entry-preview-docs.tscode/lib/csf-plugin/src/webpack-loader.tscode/renderers/react/src/componentManifest/generator.test.tscode/lib/csf-plugin/src/rollup-based-plugin.tscode/renderers/react/src/enrichCsf.tscode/renderers/react/src/componentManifest/jsdocTags.test.tscode/builders/builder-vite/src/plugins/csf-plugin.tscode/core/src/common/presets.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/types/modules/core-common.tscode/renderers/react/src/docs/jsxDecorator.tsx
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/common/utils/formatter.tscode/renderers/react/src/preset.tscode/renderers/react/vitest.setup.tscode/renderers/react/vitest.config.tscode/renderers/react/src/componentManifest/jsdocTags.tscode/vitest-setup.tscode/addons/docs/src/preset.tscode/renderers/react/src/enrichCsf.test.tscode/core/src/csf-tools/enrichCsf.tscode/renderers/react/src/entry-preview-docs.tscode/lib/csf-plugin/src/webpack-loader.tscode/renderers/react/src/componentManifest/generator.test.tscode/lib/csf-plugin/src/rollup-based-plugin.tscode/renderers/react/src/enrichCsf.tscode/renderers/react/src/componentManifest/jsdocTags.test.tscode/builders/builder-vite/src/plugins/csf-plugin.tscode/core/src/common/presets.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/types/modules/core-common.tscode/renderers/react/src/docs/jsxDecorator.tsx
code/.storybook/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place internal UI Storybook configuration in
code/.storybook/and maintain it there
Files:
code/.storybook/main.ts
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/renderers/react/src/enrichCsf.test.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/jsdocTags.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/renderers/react/src/enrichCsf.test.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/jsdocTags.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/renderers/react/src/enrichCsf.test.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/jsdocTags.test.ts
🧠 Learnings (12)
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Applied to files:
code/renderers/react/vitest.setup.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Mock external dependencies in tests using `vi.mock()` (e.g., filesystem, loggers)
Applied to files:
code/renderers/react/vitest.setup.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to access and implement mock behaviors
Applied to files:
code/renderers/react/vitest.setup.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in beforeEach blocks
Applied to files:
code/renderers/react/vitest.setup.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to type and access mocked functions
Applied to files:
code/renderers/react/vitest.setup.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should be placed in beforeEach blocks
Applied to files:
code/renderers/react/vitest.setup.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with vi.mocked()
Applied to files:
code/renderers/react/vitest.setup.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of beforeEach blocks
Applied to files:
code/renderers/react/vitest.setup.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without vi.mocked()
Applied to files:
code/renderers/react/vitest.setup.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:04.287Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Applies to code/vitest.workspace.ts : Keep and use the Vitest workspace configuration at code/vitest.workspace.ts
Applied to files:
code/renderers/react/vitest.config.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
PR: storybookjs/storybook#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/react/src/entry-preview-docs.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/renderers/react/src/docs/jsxDecorator.tsx
🧬 Code graph analysis (14)
code/renderers/react/src/componentManifest/jsdocTags.ts (1)
code/renderers/react/src/componentManifest/utils.ts (1)
groupBy(2-12)
code/vitest-setup.ts (1)
code/renderers/web-components/template/components/Html.js (1)
globalThis(5-5)
code/addons/docs/src/preset.ts (3)
code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(10-115)code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)
code/renderers/react/src/enrichCsf.test.ts (3)
code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(10-115)code/core/src/common/presets.ts (2)
getPresets(317-327)InterPresetOptions(23-28)code/core/src/csf-tools/CsfFile.ts (1)
loadCsf(1032-1036)
code/core/src/csf-tools/enrichCsf.ts (3)
code/core/src/types/modules/core-common.ts (1)
CsfEnricher(365-365)code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(10-115)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)
code/renderers/react/src/entry-preview-docs.ts (2)
code/core/src/types/modules/core-common.ts (1)
StorybookConfigRaw(367-526)code/renderers/react/src/docs/jsxDecorator.tsx (1)
jsxDecorator(238-269)
code/lib/csf-plugin/src/webpack-loader.ts (3)
code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(10-115)code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)
code/lib/csf-plugin/src/rollup-based-plugin.ts (3)
code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(10-115)code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)
code/renderers/react/src/enrichCsf.ts (5)
code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(277-1017)code/core/src/common/utils/formatter.ts (1)
getPrettier(1-6)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(23-247)
code/renderers/react/src/componentManifest/jsdocTags.test.ts (1)
code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)
code/builders/builder-vite/src/plugins/csf-plugin.ts (3)
code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(10-115)code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)
code/renderers/react/src/componentManifest/generator.ts (1)
code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)
code/core/src/types/modules/core-common.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(277-1017)
code/renderers/react/src/docs/jsxDecorator.tsx (1)
code/core/src/types/modules/core-common.ts (1)
StorybookConfigRaw(367-526)
⏰ 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). (1)
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (34)
code/core/src/common/utils/formatter.ts (3)
1-6: Export ofgetPrettieris well-suited for public API.The function implements a robust pattern for handling optional dependencies: it dynamically imports prettier and returns a safe no-op fallback (
resolveConfig: async () => null,format: (content) => content) if the import fails. This allows consumers to handle formatting failures gracefully without breaking the build.
13-31: Error handling strategy is sound for a formatter utility.The broad catch-all at line 28–30 silently returns the original content on any error. This "fail gracefully" approach is appropriate for formatting utilities where a formatting failure should not break the pipeline. The function also correctly delegates to
formatWithEditorConfigas a fallback when prettier config is missing or empty.
22-25:as anytype assertions are acceptable given prettier's dynamic return type.The type assertions when destructuring the prettier module return value are appropriate, since the dynamic import result varies depending on whether prettier is available or whether the fallback object is returned. While not ideal from a type-safety perspective, this is a pragmatic trade-off for handling optional dependencies.
Also applies to: 42-45
code/renderers/react/src/componentManifest/jsdocTags.ts (1)
5-21: Verify error handling for malformed JSDoc comments.The function doesn't handle cases where
parsed[0]might be undefined (e.g., if the comment-parser fails or returns an empty array). This could lead to runtime errors when processing malformed JSDoc comments.Consider adding defensive checks:
export function extractJSDocInfo(jsdocComment: string) { const lines = jsdocComment.split('\n'); const jsDoc = ['/**', ...lines.map((line) => ` * ${line}`), ' */'].join('\n'); const parsed = parse(jsDoc); + + if (!parsed[0]) { + return { description: '', tags: {} }; + } return { description: parsed[0].description, tags: Object.fromEntries(code/core/src/common/presets.ts (1)
23-28: LGTM on exporting InterPresetOptions.Making this type public improves type safety for code that needs to reference preset options.
code/renderers/react/src/componentManifest/jsdocTags.test.ts (1)
5-40: LGTM on test updates.The tests correctly reflect the renamed function and new return structure. Inline snapshots accurately capture the expected output format.
code/renderers/react/src/componentManifest/generator.ts (3)
12-12: LGTM on updated import.The import correctly reflects the renamed function.
86-86: Good addition of jsDocTags to the manifest.Exposing the structured JSDoc tags enriches the component manifest and enables downstream consumers to access granular documentation metadata.
75-82: Potential undefined error when trimming description.On Line 82,
manifestDescription?.trim()could fail ifmanifestDescriptionisundefinedwhen neithertags.describe,tags.desc, nordescriptionexist. The optional chaining on Line 77 doesn't preventundefinedfrom being passed totrim().Apply this fix to safely handle undefined:
- const manifestDescription = (tags?.describe?.[0] || tags?.desc?.[0]) ?? description; + const manifestDescription = tags?.describe?.[0] || tags?.desc?.[0] || description; return { id, name: componentName, - description: manifestDescription?.trim(), + description: manifestDescription?.trim() ?? undefined,Likely an incorrect or invalid review comment.
code/.storybook/main.ts (1)
141-141: LGTM on enabling experimental feature.Enabling
experimentalCodeExamplesallows testing the new code examples functionality in the internal Storybook.code/vitest-setup.ts (1)
42-42: LGTM on global FEATURES initialization.Initializing
globalThis.FEATURESearly in the test setup ensures the global object is available for feature flag checks throughout the test suite.code/lib/csf-plugin/src/webpack-loader.ts (1)
23-23: LGTM on awaiting enrichCsf.Correctly awaits the now-async
enrichCsffunction, ensuring enrichment completes before formatting. The existing try-catch block handles any enrichment errors appropriately.code/builders/builder-vite/src/plugins/csf-plugin.ts (1)
15-21: LGTM on integrating enrichCsf into Vite plugin.The code correctly retrieves the optional
experimental_enrichCsfenricher from presets and passes it to the CSF plugin, enabling async enrichment when configured.code/renderers/react/vitest.setup.ts (1)
1-6: LGTM!The setup file correctly restores mocks after each test, and the comment helpfully explains why this must be in
afterEachrather thanbeforeEach(to preservespy: truemocks).code/lib/csf-plugin/src/rollup-based-plugin.ts (1)
26-26: LGTM!The addition of
awaitcorrectly handles the async enrichCsf call, aligning with the broader changes to make CSF enrichment asynchronous throughout the pipeline.code/renderers/react/src/docs/jsxDecorator.tsx (1)
7-8: LGTM!The global
FEATUREStype declaration correctly mirrors the configuration surface fromStorybookConfigRaw['features'], ensuring consistent typing for feature flags across runtime and tests.Also applies to: 235-236
code/renderers/react/src/preset.ts (1)
13-13: LGTM!The export alias follows the established pattern for experimental features (consistent with
experimental_componentManifestGeneratoron Line 11) and properly exposes the enrichment capability.code/addons/docs/src/preset.ts (1)
6-6: LGTM!The integration of
enrichCsfinto the webpack configuration is correctly implemented, deriving it from the presets viaexperimental_enrichCsfand passing it to the CSF plugin when present.Also applies to: 45-45, 106-111
code/renderers/react/src/componentManifest/generator.test.ts (1)
7-7: LGTM!The refactoring from wildcard import to named import, combined with the addition of
spy: trueto the mock, aligns with the coding guidelines for Vitest mocking patterns.Based on learnings.
Also applies to: 13-13, 98-98
code/renderers/react/vitest.config.ts (1)
8-10: LGTM!The
setupFilesconfiguration correctly references the new setup file, ensuring mocks are properly restored after each test.code/renderers/react/src/entry-preview-docs.ts (1)
7-12: Now let me search the codebase for how FEATURES is initialized in the preview context and how entry-preview-docs.ts is loaded:Now let me search the codebase directly for FEATURES initialization and usage:Based on my verification, I have sufficient information to generate the rewritten review comment. Let me analyze what I've found:
- vitest-setup.ts initializes
globalThis.FEATURES = {}at line 42 - this is test setup only- entry-preview-docs.ts is a preview entry file (loaded in Storybook's preview iframe)
- Web searches show
StorybookConfigRaw['features']is a real configuration option in Storybook's main.ts- The code uses
FEATURES.experimentalCodeExampleswith a ternary operator that doesn't guard against undefinedThe core issue is that the review comment identifies a real concern: the code conditionally accesses a property on FEATURES without verifying it exists first. The current approach works if FEATURES is always injected by Storybook's build system before this module loads, but there's no explicit evidence that this happens for preview entry files. The vitest initialization found is insufficient because it's test-only.
However, since this is part of Storybook's internal rendering system and FEATURES is declared with
declare var, it indicates this variable is meant to be injected by the build system. The risk is primarily around ensuring FEATURES is always available at preview runtime.
The conditional logic at lines 9-12 safely accesses
FEATURES.experimentalCodeExamplesbecause it's evaluated within a simple ternary operator. WhileFEATURESis declared but not initialized in this file, it's intended to be injected by Storybook's build system before this preview entry executes. The initialization invitest-setup.ts(line 42:globalThis.FEATURES = {}) confirms the injection pattern for test environments. The type declaration withNonNullable<StorybookConfigRaw['features']>ensures type safety. This code is working as designed within Storybook's preview initialization framework and requires no changes.code/renderers/react/src/enrichCsf.ts (3)
10-14: LGTM! Clean feature flag guard.The early return pattern when the feature is disabled is appropriate and efficient.
35-45: Verify type assertion safety.Line 41 casts
declarator.get('init')toNodePath<t.Expression>before checkingisExpression()on Line 43. While the subsequent check provides a guard, the assertion assumes the type before verification.Consider whether the type guard check on Line 43 is sufficient, or if additional validation is needed before the cast.
15-114: Well-structured enrichment implementation.The async enrichment logic with parallel processing via
Promise.allis efficient. The AST manipulation correctly injects code snippets into story parameters while preserving existing values through spread operators, as documented in the inline comments.code/renderers/react/src/enrichCsf.test.ts (3)
13-22: LGTM! Mocking follows coding guidelines.The mocks use
spy: trueand implementations are inbeforeEach, aligning with the project's testing guidelines.Based on coding guidelines.
24-78: Comprehensive test coverage with inline snapshots.The test validates the enriched CSF output including the injected code snippets in story parameters. The inline snapshot approach clearly documents the expected transformation.
80-110: Good coverage of disabled feature state.The test verifies that enrichment is skipped when
experimentalCodeExamplesis disabled. The@ts-expect-erroron Line 81 is acceptable for testing mock property access.code/core/src/types/modules/core-common.ts (4)
110-111: Improved type inference for presets.The generic overload enables better type inference when calling
presets.apply()with a specific StorybookConfigRaw key, improving the developer experience.
365-365: Well-defined enricher type.The
CsfEnrichertype signature clearly defines the contract for CSF enrichment hooks, accepting both the modified and source CSF files and returning a Promise.
381-381: LGTM! Experimental enricher configuration option.The
experimental_enrichCsfoption is properly typed using theCsfEnrichertype and follows the naming convention for experimental features.
481-488: Feature flag properly documented as experimental.The JSDoc clearly marks this as experimental and provides a useful description. The default of
falseis appropriate for an experimental feature.code/core/src/csf-tools/enrichCsf.ts (3)
2-2: LGTM! Import added for new enricher type.The
CsfEnrichertype import is necessary for the EnrichCsfOptions extension.
9-9: Clean extension of EnrichCsfOptions.Adding the optional
enrichCsfhook enables customization of the enrichment pipeline without breaking existing usage.
144-146: Well-integrated async enrichment hook.The conversion to
asyncand the placement of the enricher hook between meta and story enrichment allows for asynchronous transformations while maintaining the existing enrichment flow.
|
View your CI Pipeline Execution ↗ for commit 8422167
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code/core/src/types/modules/core-common.ts(5 hunks)code/renderers/react/src/componentManifest/generator.ts(2 hunks)code/renderers/react/src/docs/jsxDecorator.tsx(1 hunks)code/renderers/react/src/enrichCsf.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/types/modules/core-common.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/docs/jsxDecorator.tsxcode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/enrichCsf.test.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/docs/jsxDecorator.tsxcode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/enrichCsf.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/docs/jsxDecorator.tsxcode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/enrichCsf.test.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/docs/jsxDecorator.tsxcode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/enrichCsf.test.ts
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/renderers/react/src/enrichCsf.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/renderers/react/src/enrichCsf.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/renderers/react/src/enrichCsf.test.ts
🧠 Learnings (1)
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/renderers/react/src/docs/jsxDecorator.tsx
🧬 Code graph analysis (2)
code/renderers/react/src/componentManifest/generator.ts (1)
code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)
code/renderers/react/src/enrichCsf.test.ts (3)
code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(10-115)code/core/src/common/presets.ts (2)
getPresets(317-327)InterPresetOptions(23-28)code/core/src/csf-tools/CsfFile.ts (1)
loadCsf(1042-1046)
⏰ 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). (1)
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (8)
code/renderers/react/src/enrichCsf.test.ts (3)
1-16: Mock setup follows guidelines.The mocks are correctly placed at the top of the file, and
node:moduleuses the requiredspy: trueoption. The virtual 'my-preset' module mock is appropriately structured for this test scenario.
18-20: beforeEach implementation follows best practices.Mock behavior is correctly implemented in the beforeEach block using
vi.mocked()for type-safe access.
22-76: Test coverage for enrichment is solid.The test properly validates that
enrichCsfinjects code parameters into story exports when the feature flag is enabled. Async operations are correctly awaited, and the inline snapshot provides clear expectations.code/renderers/react/src/componentManifest/generator.ts (5)
12-12: LGTM: Import updated to match refactored JSDoc extraction.The import change from
extractJSDocTagstoextractJSDocInfoaligns with the refactoring that now returns bothdescriptionandtags, providing more complete JSDoc information.
79-79: Good practice: Trimming whitespace from description.Using optional chaining with
trim()appropriately handles the case wheremanifestDescriptionis undefined, while ensuring any description value is cleaned of extraneous whitespace.
76-85: LGTM: Manifest structure enhanced with JSDoc tags.The returned manifest now includes
jsDocTags, providing access to all extracted JSDoc tags. The use of optional chaining forsummaryandimportsafely handles missing tag values, and thesatisfies ReactComponentManifestconstraint ensures type safety.
61-61: Correct handling: Early return includes empty jsDocTags.When the component file is unavailable, returning
jsDocTags: {}is appropriate since JSDoc extraction hasn't occurred yet. This maintains structural consistency with the main return path while still providing component examples.
72-74: The review comment flags a theoretical concern that does not apply to the actual codebase.The code at lines 72-74 is correct. While
extractJSDocInfodoes format tag values to include type and name information (e.g.,{Type} name description), the empirical evidence shows that@describeand@desctags are not used anywhere in the componentManifest codebase. The optional chaining at line 74 safely handles the absence of these tags by returningundefined, which then falls back to thedescriptionvalue. The logic is sound and defensive—no changes needed.Likely an incorrect or invalid review comment.
| vi.spyOn((await import('my-preset')).default, 'features', 'get').mockImplementation(() => ({ | ||
| experimentalCodeExamples: false, | ||
| })); |
There was a problem hiding this comment.
Refactor: Mock implementation within test case violates guidelines.
Lines 80-82 modify the mock behavior inside the test case, which violates the guideline: "Avoid inline mock implementations within test cases" and "Implement mock behaviors in beforeEach blocks." Additionally, there's no cleanup/restore after this test, which could affect test isolation.
Consider one of these approaches:
Option 1 (Recommended): Separate describe blocks with beforeEach
-test('should not enrich when experimentalCodeExamples is disabled', async () => {
- // @ts-expect-error module does not exist
- vi.spyOn((await import('my-preset')).default, 'features', 'get').mockImplementation(() => ({
- experimentalCodeExamples: false,
- }));
+describe('when experimentalCodeExamples is disabled', () => {
+ beforeEach(async () => {
+ // @ts-expect-error module does not exist
+ vi.spyOn((await import('my-preset')).default, 'features', 'get').mockImplementation(() => ({
+ experimentalCodeExamples: false,
+ }));
+ });
+
+ test('should not enrich csf', async () => {
const presets = await getPresets(['my-preset'], { isCritical: true } as InterPresetOptions);
const enrichCsf = await presets.apply('experimental_enrichCsf');
+ // ... rest of test
+ });
+});Option 2: Add mock cleanup
test('should not enrich when experimentalCodeExamples is disabled', async () => {
// @ts-expect-error module does not exist
- vi.spyOn((await import('my-preset')).default, 'features', 'get').mockImplementation(() => ({
+ const spy = vi.spyOn((await import('my-preset')).default, 'features', 'get').mockImplementation(() => ({
experimentalCodeExamples: false,
}));
+
+ try {
const presets = await getPresets(['my-preset'], { isCritical: true } as InterPresetOptions);
// ... rest of test assertions
+ } finally {
+ spy.mockRestore();
+ }
});Based on learnings (if any previous feedback suggested this pattern).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In code/renderers/react/src/enrichCsf.test.ts around lines 80 to 82, the test
sets a spy mock implementation inline which violates the guideline to avoid
inline mocks and lacks cleanup; move the vi.spyOn(...).mockImplementation(...)
into a beforeEach within an appropriate describe block (or create a new describe
block for this behavior) and pair it with an afterEach that restores the spy
(e.g., vi.restoreAllMocks or spy.mockRestore) to ensure test isolation and
proper cleanup.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/core/src/core-server/presets/favicon.test.ts (1)
32-46: Consider adding the optionalargsparameter for full interface compliance.The mock
applysignature currently has two parameters but thePresetsinterface generic overload includes a third optional parameter:apply<T>(extension: string, config?: T, args?: unknown): Promise<T>. While the current tests don't require it, adding the third parameter would make the mock fully compliant with the interface.Apply this diff to add the optional parameter:
const createOptions = (locations: string[]): Parameters<typeof m.favicon>[1] => ({ configDir: '', presets: { - apply: async (extension: string, config: any) => { + apply: async (extension: string, config: any, args?: unknown) => { switch (extension) { case 'staticDirs': { return locations.map((location) => ({ from: location, to: '/' })); } default: { return config as any; } } }, } as Presets, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/core/src/core-server/presets/favicon.test.ts(2 hunks)code/core/src/csf-tools/enrichCsf.test.ts(22 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/core/src/core-server/presets/favicon.test.tscode/core/src/csf-tools/enrichCsf.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/core/src/core-server/presets/favicon.test.tscode/core/src/csf-tools/enrichCsf.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/core/src/core-server/presets/favicon.test.tscode/core/src/csf-tools/enrichCsf.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/core/src/core-server/presets/favicon.test.tscode/core/src/csf-tools/enrichCsf.test.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/core/src/core-server/presets/favicon.test.tscode/core/src/csf-tools/enrichCsf.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/core/src/core-server/presets/favicon.test.tscode/core/src/csf-tools/enrichCsf.test.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-server/presets/favicon.test.tscode/core/src/csf-tools/enrichCsf.test.ts
🧬 Code graph analysis (2)
code/core/src/core-server/presets/favicon.test.ts (1)
code/core/src/types/modules/core-common.ts (1)
Presets(66-114)
code/core/src/csf-tools/enrichCsf.test.ts (2)
code/core/src/csf-tools/enrichCsf.ts (1)
EnrichCsfOptions(6-10)code/core/src/csf-tools/CsfFile.ts (1)
formatCsf(1048-1058)
⏰ 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). (1)
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/core/src/csf-tools/enrichCsf.test.ts (2)
14-25: LGTM! Async helper correctly implemented.The
enrichhelper has been properly converted to async, correctly awaiting theenrichCsfcall while maintaining the synchronousformatCsfreturn.
29-952: LGTM! Test cases correctly updated for async flow.All test cases have been consistently updated to be async and properly await the
enrichhelper. The pattern is correct throughout, and the unchanged snapshots confirm that the enrichment behavior is preserved.code/core/src/core-server/presets/favicon.test.ts (1)
7-7: LGTM!The type-only import is a good practice and correctly imports the
Presetsinterface needed for type casting the mock object.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
code/renderers/react/src/componentManifest/jsdocTags.ts (1)
13-19: Simplify the complex tag transformation.The nested
Object.entries(groupBy(...)).map(...)wrapped inObject.fromEntriesis difficult to read and maintain. Consider extracting this logic or adding intermediate variables.Apply this diff to improve readability:
+ const groupedTags = groupBy(parsed[0].tags || [], (it) => it.tag); + const formattedTags = Object.entries(groupedTags).map(([key, tags]) => { + const tagStrings = tags.map((tag) => { + const typePrefix = tag.type ? `{${tag.type}} ` : ''; + const name = tag.name || ''; + const description = tag.description || ''; + return `${typePrefix}${name} ${description}`.trim(); + }); + return [key, tagStrings]; + }); + return { description: parsed[0].description, - tags: Object.fromEntries( - Object.entries(groupBy(parsed[0].tags, (it) => it.tag)).map(([key, tags]) => [ - key, - tags?.map((tag) => (tag.type ? `{${tag.type}} ` : '') + `${tag.name} ${tag.description}`) ?? - [], - ]) - ), + tags: Object.fromEntries(formattedTags), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/renderers/react/src/componentManifest/jsdocTags.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/componentManifest/jsdocTags.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/componentManifest/jsdocTags.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/componentManifest/jsdocTags.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/jsdocTags.ts
🧬 Code graph analysis (1)
code/renderers/react/src/componentManifest/jsdocTags.ts (1)
code/renderers/react/src/componentManifest/utils.ts (1)
groupBy(2-12)
⏰ 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). (1)
- GitHub Check: Core Unit Tests, windows-latest
| const parsed = parse(jsDoc); | ||
|
|
||
| return Object.fromEntries( | ||
| Object.entries(groupBy(parsed[0].tags, (it) => it.tag)).map(([key, tags]) => [ | ||
| key, | ||
| tags?.map((tag) => (tag.type ? `{${tag.type}} ` : '') + `${tag.name} ${tag.description}`) ?? | ||
| [], | ||
| ]) | ||
| ); | ||
| } | ||
|
|
||
| export function removeTags(jsdocComment: string) { | ||
| return jsdocComment | ||
| .split('\n') | ||
| .filter((line) => !line.trim().startsWith('@')) | ||
| .join('\n'); | ||
| return { | ||
| description: parsed[0].description, |
There was a problem hiding this comment.
Add defensive checks for parse results.
The code assumes parse() always returns a non-empty array with a valid first element. If parsing fails or the input is malformed, accessing parsed[0].description and parsed[0].tags will throw a runtime error.
Apply this diff to add defensive checks:
const parsed = parse(jsDoc);
+
+ if (!parsed || parsed.length === 0) {
+ return { description: '', tags: {} };
+ }
return {
description: parsed[0].description,🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/jsdocTags.ts around lines 9 to 12,
the code assumes parse(jsDoc) always returns a non-empty array and directly
accesses parsed[0].description and parsed[0].tags; add defensive checks so
parsing failures or empty results don't throw: after calling parse(jsDoc) verify
parsed is an array and has at least one element, set a const first =
Array.isArray(parsed) && parsed.length > 0 ? parsed[0] : {} and then read
description and tags from first using sensible defaults (e.g., description =
first.description || '' and tags = Array.isArray(first.tags) ? first.tags : []);
optionally wrap parse in try/catch to return safe defaults if parse throws.
| tags: Object.fromEntries( | ||
| Object.entries(groupBy(parsed[0].tags, (it) => it.tag)).map(([key, tags]) => [ | ||
| key, | ||
| tags?.map((tag) => (tag.type ? `{${tag.type}} ` : '') + `${tag.name} ${tag.description}`) ?? | ||
| [], | ||
| ]) | ||
| ), |
There was a problem hiding this comment.
Guard against undefined tag properties.
The string concatenation on line 16 may produce "undefined" text if tag.name or tag.description is undefined. This could result in malformed output strings like "{string} undefined undefined".
Apply this diff to safely handle undefined values:
Object.entries(groupBy(parsed[0].tags, (it) => it.tag)).map(([key, tags]) => [
key,
- tags?.map((tag) => (tag.type ? `{${tag.type}} ` : '') + `${tag.name} ${tag.description}`) ??
+ tags?.map((tag) => {
+ const typePrefix = tag.type ? `{${tag.type}} ` : '';
+ const name = tag.name || '';
+ const description = tag.description || '';
+ return `${typePrefix}${name} ${description}`.trim();
+ }) ??
[],
])🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/jsdocTags.ts around lines 13 to
19, the map builds strings using tag.type, tag.name and tag.description which
can be undefined and produce literal "undefined" in output; update the mapping
to coerce each value to an empty string when undefined (e.g. use tag.type ?
`{${tag.type}} ` : '' and `${tag.name ?? ''}` and `${tag.description ?? ''}`),
then trim the resulting string to remove extra spaces so malformed strings like
"{string} undefined undefined" cannot occur.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
code/core/src/types/modules/core-common.ts (3)
110-112: Consider clarifying the comment.The comment "The second and third parameter are not needed" could be more explicit about the purpose of this overload.
Apply this diff to improve clarity:
- /** The second and third parameter are not needed. And make type inference easier. */ + /** Convenience overload for retrieving preset values without providing config or args. Improves type inference. */ apply<T extends keyof StorybookConfigRaw>(extension: T): Promise<StorybookConfigRaw[T]>;
367-367: Clarify parameter roles in theCsfEnrichertype.The two parameters
csfandcsfSourceare both of typeCsfFile, but their distinct roles aren't clear from the names alone. Consider either renaming them (e.g.,targetandsource, orenrichedandoriginal) or adding JSDoc to explain their purpose.Example with JSDoc:
+/** + * Function that enriches a CSF (Component Story Format) file with additional metadata. + * @param csf - The CSF file to be enriched (will be modified) + * @param csfSource - The original CSF file source (read-only) + */ export type CsfEnricher = (csf: CsfFile, csfSource: CsfFile) => Promise<void>;
383-383: Add JSDoc documentation forexperimental_enrichCsf.The new
experimental_enrichCsffield lacks documentation explaining its purpose and usage. Since this is a public API inStorybookConfigRaw, it should have JSDoc similar to other fields in this interface.Example JSDoc:
componentManifestGenerator?: ComponentManifestGenerator; + /** + * Experimental hook for enriching CSF (Component Story Format) files during processing. + * This enables asynchronous transformation of CSF files before they are loaded. + * + * @experimental This feature is in early development and may change significantly in future releases. + */ experimental_enrichCsf?: CsfEnricher;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/addons/docs/src/preset.ts(3 hunks)code/core/src/types/modules/core-common.ts(5 hunks)code/renderers/react/src/preset.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- code/renderers/react/src/preset.ts
- code/addons/docs/src/preset.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/core/src/types/modules/core-common.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/core/src/types/modules/core-common.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/core/src/types/modules/core-common.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/types/modules/core-common.ts
🧬 Code graph analysis (1)
code/core/src/types/modules/core-common.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(281-1027)
⏰ 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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/core/src/types/modules/core-common.ts (1)
4-4: LGTM: Import properly supports new type definition.The type-only import of
CsfFileis correctly structured and necessary for theCsfEnrichertype definition below.
| /** | ||
| * Enables the new code example generation for React components. You can see those examples when | ||
| * clicking on the "Show code" button in the Storybook UI. | ||
| * | ||
| * We refactored the code examples by reading the actual source file. This should make the code | ||
| * examples a lot faster, more readable and more accurate. They are not dynamic though, it won't | ||
| * change if you change when using the control panel. | ||
| * | ||
| * @default false | ||
| * @experimental This feature is in early development and may change significantly in future releases. | ||
| */ | ||
| experimentalCodeExamples?: boolean; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix grammatical error in JSDoc and consider addressing past review feedback.
The JSDoc has a grammatical issue in the sentence "it won't change if you change when using the control panel."
Apply this diff to fix the grammar:
* We refactored the code examples by reading the actual source file. This should make the code
* examples a lot faster, more readable and more accurate. They are not dynamic though, it won't
- * change if you change when using the control panel.
+ * change when you modify controls in the control panel.Additionally, the past review comment mentions "better support for render functions." If this is a key improvement, consider adding it explicitly to the JSDoc:
* Enables the new code example generation for React components. You can see those examples when
* clicking on the "Show code" button in the Storybook UI.
*
* We refactored the code examples by reading the actual source file. This should make the code
- * examples a lot faster, more readable and more accurate. They are not dynamic though, it won't
- * change when you modify controls in the control panel.
+ * examples a lot faster, more readable and more accurate, with improved support for render
+ * functions. They are not dynamic though, meaning they won't change when you modify controls
+ * in the control panel.🤖 Prompt for AI Agents
In code/core/src/types/modules/core-common.ts around lines 484 to 495, the JSDoc
contains a grammatical error ("it won't change if you change when using the
control panel"); update the sentence to a clear form such as "The examples are
static and will not update when you change controls in the control panel." Also,
if desired, add a brief note about improved support for render functions (e.g.,
"May include better support for render functions") to the JSDoc to address the
past review feedback, keeping the @default and @experimental tags unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test-storybooks/portable-stories-kitchen-sink/react/jest.setup.ts (1)
5-6: LGTM! Consider adding TypeScript declaration for type safety.The defensive initialization using nullish coalescing is correct and ensures the FEATURES namespace exists before project annotations are applied.
For better TypeScript type safety, consider adding a global type declaration. You can add this at the top of the file:
+declare global { + // eslint-disable-next-line no-var + var FEATURES: Record<string, unknown>; +} + import "@testing-library/jest-dom"; import { setProjectAnnotations } from "@storybook/react-vite"; import sbAnnotations from "./.storybook/preview";Optionally, add a brief comment explaining the purpose:
+// Initialize global FEATURES namespace for experimental feature flags globalThis.FEATURES ??= {};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test-storybooks/portable-stories-kitchen-sink/react/jest.setup.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
test-storybooks/portable-stories-kitchen-sink/react/jest.setup.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
test-storybooks/portable-stories-kitchen-sink/react/jest.setup.ts
test-storybooks/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain test Storybook configurations under
test-storybooks/for E2E and visual testing scenarios
Files:
test-storybooks/portable-stories-kitchen-sink/react/jest.setup.ts
🧬 Code graph analysis (1)
test-storybooks/portable-stories-kitchen-sink/react/jest.setup.ts (1)
code/renderers/web-components/template/components/Html.js (1)
globalThis(5-5)
⏰ 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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test-storybooks/portable-stories-kitchen-sink/react/globals.setup.ts (1)
1-1: LGTM! Consider adding a brief comment.The initialization is correct and follows the established pattern used elsewhere in the codebase (e.g.,
code/vitest-setup.ts).Optionally, add a brief comment explaining the purpose:
+// Initialize global feature flags object for test environment globalThis.FEATURES = {};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test-storybooks/portable-stories-kitchen-sink/react/globals.setup.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/jest.config.js(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/jest.setup.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test-storybooks/portable-stories-kitchen-sink/react/jest.setup.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
test-storybooks/portable-stories-kitchen-sink/react/globals.setup.tstest-storybooks/portable-stories-kitchen-sink/react/jest.config.js
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
test-storybooks/portable-stories-kitchen-sink/react/globals.setup.tstest-storybooks/portable-stories-kitchen-sink/react/jest.config.js
test-storybooks/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain test Storybook configurations under
test-storybooks/for E2E and visual testing scenarios
Files:
test-storybooks/portable-stories-kitchen-sink/react/globals.setup.tstest-storybooks/portable-stories-kitchen-sink/react/jest.config.js
🧠 Learnings (2)
📚 Learning: 2025-09-17T08:11:04.287Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx} : Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Applied to files:
test-storybooks/portable-stories-kitchen-sink/react/jest.config.js
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together
Applied to files:
test-storybooks/portable-stories-kitchen-sink/react/jest.config.js
🧬 Code graph analysis (1)
test-storybooks/portable-stories-kitchen-sink/react/globals.setup.ts (1)
code/renderers/web-components/template/components/Html.js (1)
globalThis(5-5)
⏰ 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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
test-storybooks/portable-stories-kitchen-sink/react/jest.config.js (1)
3-3: LGTM! Proper placement for global initialization.The
setupFilesconfiguration correctly loads the global setup beforesetupFilesAfterEnv, ensuring that the globalFEATURESobject is available before any test setup runs.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/renderers/react/src/entry-preview-docs.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/entry-preview-docs.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/entry-preview-docs.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/entry-preview-docs.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/entry-preview-docs.ts
🧬 Code graph analysis (1)
code/renderers/react/src/entry-preview-docs.ts (2)
code/renderers/web-components/template/components/Html.js (1)
globalThis(5-5)code/renderers/react/src/docs/jsxDecorator.tsx (1)
jsxDecorator(234-265)
⏰ 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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
| export const decorators: DecoratorFunction<ReactRenderer>[] = | ||
| 'FEATURES' in globalThis && globalThis?.FEATURES?.experimentalCodeExamples ? [] : [jsxDecorator]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify the necessity of the 'in' operator check.
The condition combines the in operator with optional chaining. While this is defensively sound, the 'FEATURES' in globalThis check may be redundant since optional chaining already handles undefined/null cases.
Consider simplifying to:
-export const decorators: DecoratorFunction<ReactRenderer>[] =
- 'FEATURES' in globalThis && globalThis?.FEATURES?.experimentalCodeExamples ? [] : [jsxDecorator];
+export const decorators: DecoratorFunction<ReactRenderer>[] =
+ globalThis.FEATURES?.experimentalCodeExamples ? [] : [jsxDecorator];Additionally, consider using explicit boolean comparison if experimentalCodeExamples should strictly be a boolean flag:
globalThis.FEATURES?.experimentalCodeExamples === trueRun the following script to check if there are TypeScript errors and whether the 'in' check pattern is used consistently elsewhere:
🏁 Script executed:
#!/bin/bash
# Check for similar feature flag patterns in the codebase
rg -n "FEATURES.*in.*globalThis|globalThis.*FEATURES" --type=ts --type=js -C2
# Check for TypeScript definitions of globalThis.FEATURES
rg -n "globalThis.*FEATURES|FEATURES.*globalThis" --type=ts -g "*.d.ts" -C2Length of output: 5148
Remove unnecessary 'FEATURES' in globalThis check to match codebase patterns.
The 'FEATURES' in globalThis check is inconsistent with the rest of the codebase. Every other feature flag in the application uses optional chaining without the 'in' operator:
code/core/src/measure/preview.ts:7:globalThis.FEATURES?.measurecode/core/src/outline/preview.ts:7:globalThis.FEATURES?.outlinecode/core/src/actions/manager.tsx:10:globalThis?.FEATURES?.actions- And 8+ additional instances throughout the codebase
Since globalThis.FEATURES is initialized in vitest-setup.ts before code execution, the 'in' check is redundant. Simplify to match the established pattern:
-export const decorators: DecoratorFunction<ReactRenderer>[] =
- 'FEATURES' in globalThis && globalThis?.FEATURES?.experimentalCodeExamples ? [] : [jsxDecorator];
+export const decorators: DecoratorFunction<ReactRenderer>[] =
+ globalThis.FEATURES?.experimentalCodeExamples ? [] : [jsxDecorator];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const decorators: DecoratorFunction<ReactRenderer>[] = | |
| 'FEATURES' in globalThis && globalThis?.FEATURES?.experimentalCodeExamples ? [] : [jsxDecorator]; | |
| export const decorators: DecoratorFunction<ReactRenderer>[] = | |
| globalThis.FEATURES?.experimentalCodeExamples ? [] : [jsxDecorator]; |
🤖 Prompt for AI Agents
In code/renderers/react/src/entry-preview-docs.ts around lines 6 to 7, remove
the redundant "'FEATURES' in globalThis" check and replace it with the same
optional-chaining pattern used across the codebase (e.g.,
globalThis?.FEATURES?.experimentalCodeExamples) so the decorators assignment
becomes conditional solely on optional chaining; ensure the expression remains
logically equivalent (use globalThis?.FEATURES?.experimentalCodeExamples) and
update formatting to match surrounding files.
kasperpeulen
merged commit 26d8df6
into
10.1-with-canary-release
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 pull request has been released as version
0.0.0-pr-32813-sha-13a2dbef. Try it out in a new sandbox by runningnpx [email protected] sandboxor in an existing project withnpx [email protected] upgrade.More information
0.0.0-pr-32813-sha-13a2dbefkasper/source-code13a2dbef1761316102)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 publish.yml --field pr=32813Summary by CodeRabbit
New Features
Improvements
Tests
Public API