React: Extract jsdoc tags in component manifest #32748
Conversation
Co-authored-by: Jeppe Reinhold <[email protected]>
Co-authored-by: Jeppe Reinhold <[email protected]>
Co-authored-by: Jeppe Reinhold <[email protected]>
Co-authored-by: Jeppe Reinhold <[email protected]>
…r/code-snippets # Conflicts: # code/core/src/core-server/dev-server.ts
📝 WalkthroughWalkthroughThe pull request adds a component manifest feature to Storybook, enabling generation and serving of component metadata, documentation, and code snippets. It includes new type definitions, build-time manifest generation, a dev-server endpoint for serving manifests, and React-specific manifest generation with DocGen integration, JSDoc parsing, and code snippet extraction from CSF stories. Changes
Sequence DiagramsequenceDiagram
actor User
participant BuildStatic as build-static.ts
participant Index as StoryIndexGenerator
participant Generator as componentManifestGenerator
participant DevServer as dev-server.ts
participant DevGenerator as componentManifestGenerator
participant Disk as Disk/FS
User->>BuildStatic: Run build with experimental_componentsManifest=true
BuildStatic->>Index: Initialize StoryIndexGenerator
Index-->>BuildStatic: Index ready
BuildStatic->>Generator: Invoke with story index
Generator->>Disk: Read story & component files
Disk-->>Generator: File contents
Generator->>Generator: Parse CSF, extract metadata<br/>Run React DocGen, generate snippets
Generator-->>BuildStatic: Component manifests object
BuildStatic->>Disk: Write manifests/components.json
User->>DevServer: GET /manifests/components.json
alt experimental_componentsManifest disabled
DevServer->>Index: Initialize StoryIndexGenerator
Index-->>DevServer: Index ready
DevServer->>DevGenerator: Invoke with story index
DevGenerator->>Disk: Read files, parse, run DocGen
Disk-->>DevGenerator: File contents
DevGenerator-->>DevServer: Manifests
DevServer->>User: Return manifests JSON
else experimental_componentsManifest enabled
DevServer->>User: Return 400 error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The changes span multiple interconnected modules with dense logic for CSF parsing, code snippet generation, React DocGen integration, and JSDoc parsing. While individual modules are focused, their interdependencies and the variety of transformation logic across Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 51cdfc7
☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 532 | 532 | 0 |
| Self size | 950 KB | 950 KB | 🎉 -188 B 🎉 |
| Dependency size | 58.57 MB | 61.94 MB | 🚨 +3.37 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 124 | 124 | 0 |
| Self size | 4.10 MB | 4.10 MB | 0 B |
| Dependency size | 21.63 MB | 25.01 MB | 🚨 +3.37 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 157 | 157 | 0 |
| Self size | 31 KB | 31 KB | 0 B |
| Dependency size | 23.02 MB | 26.40 MB | 🚨 +3.37 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 114 | 114 | 0 |
| Self size | 37 KB | 37 KB | 0 B |
| Dependency size | 19.58 MB | 22.95 MB | 🚨 +3.37 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 272 | 272 | 0 |
| Self size | 25 KB | 25 KB | 🚨 +18 B 🚨 |
| Dependency size | 43.53 MB | 46.91 MB | 🚨 +3.37 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 2 | 0 |
| Self size | 894 KB | 4.27 MB | 🚨 +3.37 MB 🚨 |
| Dependency size | 28 KB | 28 KB | 🎉 -6 B 🎉 |
| Bundle Size Analyzer | Link | Link |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
code/renderers/react/src/component-manifest/utils.ts (1)
6-6: Remove redundant default parameter.The
acc = {}default parameter is unnecessary sincereducealready provides{}as the initial value on line 11.Apply this diff:
- return items.reduce<Partial<Record<K, T[]>>>((acc = {}, item, index) => { + return items.reduce<Partial<Record<K, T[]>>>((acc, item, index) => {code/renderers/react/src/component-manifest/react-docgen.ts (3)
46-71: Remove unnecessary async and reduce per-call overheadFunction is marked
asyncbut contains noawait. Either removeasyncor ensure all call sitesawaitit. Also consider caching thematchPathresult at module scope to avoid repeated filesystem scans on every call.-export async function parseWithReactDocgen({ code, filename }: { code: string; filename: string }) { +export function parseWithReactDocgen({ code, filename }: { code: string; filename: string }) { const tsconfigPath = find.up('tsconfig.json', { cwd: process.cwd(), last: getProjectRoot() }); const tsconfig = TsconfigPaths.loadConfig(tsconfigPath);Additionally, memoize
tsconfig/matchPathand reuse them across invocations.
52-59: Lower log level and log once
logger.info('Using tsconfig paths for react-docgen')will emit on each call. Change to debug and guard with a one-time flag.- if (tsconfig.resultType === 'success') { - logger.info('Using tsconfig paths for react-docgen'); + if (tsconfig.resultType === 'success') { + // log once at debug level to avoid noise + logger.debug?.('Using tsconfig paths for react-docgen');
61-71: Swallowed docgen errors hinder diagnosticsCatching all errors and returning
[]hides useful context. Consider logging at debug level with a concise message.- } catch (e) { - return []; + } catch (e) { + logger.debug?.(`react-docgen parse failed for ${filename}: ${e instanceof Error ? e.message : String(e)}`); + return []; }code/renderers/react/src/component-manifest/generateCodeSnippet.ts (4)
108-121: Generalize 'args' handling to the actual parameter nameInlining and spreads only trigger for identifiers named 'args'. If the story uses a different param (e.g.,
props), transforms are skipped. Extract the first parameter identifier name and use it in checks.- if (storyFn?.isArrowFunctionExpression() || storyFn?.isFunctionExpression()) { + if (storyFn?.isArrowFunctionExpression() || storyFn?.isFunctionExpression()) { const fn = storyFn.node; + const firstParamIdent = + (t.isArrowFunctionExpression(fn) || t.isFunctionExpression(fn)) && + fn.params.length > 0 && + t.isIdentifier(fn.params[0]) + ? fn.params[0].name + : 'args'; @@ - const firstSpreadIndex = attrs.findIndex( - (a) => t.isJSXSpreadAttribute(a) && t.isIdentifier(a.argument) && a.argument.name === 'args' - ); + const firstSpreadIndex = attrs.findIndex( + (a) => t.isJSXSpreadAttribute(a) && t.isIdentifier(a.argument) && a.argument.name === firstParamIdent + ); @@ - const isArgsSpread = - t.isJSXSpreadAttribute(a) && t.isIdentifier(a.argument) && a.argument.name === 'args'; + const isArgsSpread = + t.isJSXSpreadAttribute(a) && t.isIdentifier(a.argument) && a.argument.name === firstParamIdent;And pass the param name through to
getArgsMemberKeycalls:- const key = getArgsMemberKey(a.value.expression); + const key = getArgsMemberKey(a.value.expression, firstParamIdent); @@ - const key = getArgsMemberKey(c.expression); + const key = getArgsMemberKey(c.expression, firstParamIdent);Also applies to: 136-171, 382-405, 444-456
336-367: Update helper to detect member usage for any param nameMake
getArgsMemberKeyparam-name aware (defaults to 'args' for backwards compatibility).-function getArgsMemberKey(expr: t.Node): string | null { - if (t.isMemberExpression(expr) && t.isIdentifier(expr.object) && expr.object.name === 'args') { +function getArgsMemberKey(expr: t.Node, paramName = 'args'): string | null { + if (t.isMemberExpression(expr) && t.isIdentifier(expr.object) && expr.object.name === paramName) { @@ - if ( - t.isOptionalMemberExpression?.(expr) && - t.isIdentifier(expr.object) && - expr.object.name === 'args' - ) { + if (t.isOptionalMemberExpression?.(expr) && t.isIdentifier(expr.object) && expr.object.name === paramName) {
111-209: Handle arrow functions with block bodies returning JSXCurrently only arrows with direct JSX bodies are transformed. Consider supporting
() => { return <Comp {...args} /> }by locating the returned JSX element and applying the same inlining/transform passes.I can propose a targeted patch to find
ReturnStatementwith JSX and reuse the same transformation pipeline.
571-610: Template.bind resolver misses FunctionDeclaration
resolveBindIdentifierInitonly inspects variable declarators (including exported). Common CSF also usesfunction Template(...) { ... }. Extend resolution to detectFunctionDeclaration Template(...)and return an equivalent expression node (or keep as-is).I can adapt this to support
FunctionDeclaration(wrapping into aFunctionExpression) if you want a patch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
code/core/src/core-server/build-static.ts(3 hunks)code/core/src/core-server/dev-server.ts(2 hunks)code/core/src/csf-tools/CsfFile.ts(1 hunks)code/core/src/preview-api/modules/preview-web/render/mount-utils.ts(1 hunks)code/core/src/types/modules/core-common.ts(4 hunks)code/core/src/types/modules/indexer.ts(1 hunks)code/renderers/react/__mocks__/fs/promises.cjs(1 hunks)code/renderers/react/package.json(1 hunks)code/renderers/react/src/component-manifest/docgen-handlers/actualNameHandler.ts(1 hunks)code/renderers/react/src/component-manifest/docgen-resolver.ts(1 hunks)code/renderers/react/src/component-manifest/generateCodeSnippet.test.tsx(1 hunks)code/renderers/react/src/component-manifest/generateCodeSnippet.ts(1 hunks)code/renderers/react/src/component-manifest/generator.test.ts(1 hunks)code/renderers/react/src/component-manifest/generator.ts(1 hunks)code/renderers/react/src/component-manifest/jsdoc-tags.test.ts(1 hunks)code/renderers/react/src/component-manifest/jsdoc-tags.ts(1 hunks)code/renderers/react/src/component-manifest/react-docgen.ts(1 hunks)code/renderers/react/src/component-manifest/utils.ts(1 hunks)code/renderers/react/src/preset.ts(1 hunks)
🧰 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/preset.tscode/core/src/types/modules/indexer.tscode/core/src/core-server/dev-server.tscode/renderers/react/src/component-manifest/generator.test.tscode/renderers/react/package.jsoncode/renderers/react/src/component-manifest/utils.tscode/core/src/core-server/build-static.tscode/core/src/preview-api/modules/preview-web/render/mount-utils.tscode/renderers/react/src/component-manifest/generateCodeSnippet.tscode/core/src/csf-tools/CsfFile.tscode/renderers/react/src/component-manifest/react-docgen.tscode/renderers/react/src/component-manifest/jsdoc-tags.test.tscode/renderers/react/src/component-manifest/docgen-resolver.tscode/renderers/react/src/component-manifest/jsdoc-tags.tscode/renderers/react/src/component-manifest/docgen-handlers/actualNameHandler.tscode/renderers/react/src/component-manifest/generateCodeSnippet.test.tsxcode/core/src/types/modules/core-common.tscode/renderers/react/src/component-manifest/generator.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/preset.tscode/core/src/types/modules/indexer.tscode/core/src/core-server/dev-server.tscode/renderers/react/src/component-manifest/generator.test.tscode/renderers/react/src/component-manifest/utils.tscode/core/src/core-server/build-static.tscode/core/src/preview-api/modules/preview-web/render/mount-utils.tscode/renderers/react/src/component-manifest/generateCodeSnippet.tscode/core/src/csf-tools/CsfFile.tscode/renderers/react/src/component-manifest/react-docgen.tscode/renderers/react/src/component-manifest/jsdoc-tags.test.tscode/renderers/react/src/component-manifest/docgen-resolver.tscode/renderers/react/src/component-manifest/jsdoc-tags.tscode/renderers/react/src/component-manifest/docgen-handlers/actualNameHandler.tscode/renderers/react/src/component-manifest/generateCodeSnippet.test.tsxcode/core/src/types/modules/core-common.tscode/renderers/react/src/component-manifest/generator.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/preset.tscode/core/src/types/modules/indexer.tscode/core/src/core-server/dev-server.tscode/renderers/react/src/component-manifest/generator.test.tscode/renderers/react/src/component-manifest/utils.tscode/core/src/core-server/build-static.tscode/core/src/preview-api/modules/preview-web/render/mount-utils.tscode/renderers/react/src/component-manifest/generateCodeSnippet.tscode/core/src/csf-tools/CsfFile.tscode/renderers/react/src/component-manifest/react-docgen.tscode/renderers/react/src/component-manifest/jsdoc-tags.test.tscode/renderers/react/src/component-manifest/docgen-resolver.tscode/renderers/react/src/component-manifest/jsdoc-tags.tscode/renderers/react/src/component-manifest/docgen-handlers/actualNameHandler.tscode/renderers/react/src/component-manifest/generateCodeSnippet.test.tsxcode/core/src/types/modules/core-common.tscode/renderers/react/src/component-manifest/generator.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/preset.tscode/core/src/types/modules/indexer.tscode/core/src/core-server/dev-server.tscode/renderers/react/src/component-manifest/generator.test.tscode/renderers/react/src/component-manifest/utils.tscode/core/src/core-server/build-static.tscode/core/src/preview-api/modules/preview-web/render/mount-utils.tscode/renderers/react/src/component-manifest/generateCodeSnippet.tscode/core/src/csf-tools/CsfFile.tscode/renderers/react/src/component-manifest/react-docgen.tscode/renderers/react/src/component-manifest/jsdoc-tags.test.tscode/renderers/react/src/component-manifest/docgen-resolver.tscode/renderers/react/src/component-manifest/jsdoc-tags.tscode/renderers/react/src/component-manifest/docgen-handlers/actualNameHandler.tscode/renderers/react/src/component-manifest/generateCodeSnippet.test.tsxcode/core/src/types/modules/core-common.tscode/renderers/react/src/component-manifest/generator.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/component-manifest/generator.test.tscode/renderers/react/src/component-manifest/jsdoc-tags.test.tscode/renderers/react/src/component-manifest/generateCodeSnippet.test.tsx
**/*.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/component-manifest/generator.test.tscode/renderers/react/src/component-manifest/jsdoc-tags.test.tscode/renderers/react/src/component-manifest/generateCodeSnippet.test.tsx
**/*.@(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/component-manifest/generator.test.tscode/renderers/react/src/component-manifest/jsdoc-tags.test.tscode/renderers/react/src/component-manifest/generateCodeSnippet.test.tsx
🧬 Code graph analysis (9)
code/core/src/core-server/dev-server.ts (3)
code/core/src/core-server/presets/common-preset.ts (1)
features(197-210)code/renderers/react/src/preset.ts (1)
componentManifestGenerator(11-11)code/core/src/types/modules/core-common.ts (1)
ComponentManifestGenerator(354-356)
code/renderers/react/src/component-manifest/generator.test.ts (1)
code/renderers/react/src/component-manifest/generator.ts (1)
componentManifestGenerator(15-67)
code/core/src/core-server/build-static.ts (3)
code/core/src/core-server/presets/common-preset.ts (1)
features(197-210)code/renderers/react/src/preset.ts (1)
componentManifestGenerator(11-11)code/core/src/types/modules/core-common.ts (1)
ComponentManifestGenerator(354-356)
code/renderers/react/src/component-manifest/react-docgen.ts (2)
code/core/src/common/utils/paths.ts (1)
getProjectRoot(11-57)code/renderers/react/src/component-manifest/docgen-resolver.ts (3)
defaultLookupModule(37-75)RESOLVE_EXTENSIONS(22-35)ReactDocgenResolveError(5-12)
code/renderers/react/src/component-manifest/jsdoc-tags.test.ts (1)
code/renderers/react/src/component-manifest/jsdoc-tags.ts (1)
extractJSDocTags(5-17)
code/renderers/react/src/component-manifest/jsdoc-tags.ts (1)
code/renderers/react/src/component-manifest/utils.ts (1)
groupBy(2-12)
code/renderers/react/src/component-manifest/generateCodeSnippet.test.tsx (2)
code/core/src/csf-tools/CsfFile.ts (1)
loadCsf(1021-1025)code/renderers/react/src/component-manifest/generateCodeSnippet.ts (1)
getCodeSnippet(18-241)
code/core/src/types/modules/core-common.ts (1)
code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
StoryIndexGenerator(103-878)
code/renderers/react/src/component-manifest/generator.ts (4)
code/renderers/react/src/component-manifest/utils.ts (1)
groupBy(2-12)code/renderers/react/src/component-manifest/react-docgen.ts (2)
parseWithReactDocgen(46-71)getMatchingDocgen(31-44)code/renderers/react/src/component-manifest/jsdoc-tags.ts (2)
extractJSDocTags(5-17)removeTags(19-24)code/core/src/types/modules/core-common.ts (1)
ComponentManifestGenerator(354-356)
⏰ 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 (17)
code/renderers/react/__mocks__/fs/promises.cjs (1)
1-3: LGTM!The mock correctly re-exports
fs.promisesfrom memfs for testing filesystem interactions in the component manifest generation flow.code/renderers/react/src/component-manifest/docgen-handlers/actualNameHandler.ts (1)
17-54: LGTM!The handler correctly extracts the
actualNamefrom various component declaration patterns (class/function declarations, arrow functions, forwardRef) and sets thedefinedInFilepath. The traversal logic properly handles variable declarators and assignment expressions, with appropriate early returns for efficiency.code/core/src/preview-api/modules/preview-web/render/mount-utils.ts (1)
3-7: LGTM!The function signature changes broaden the API surface by accepting any function with any arguments, removing the dependency on specific internal Storybook types. The runtime behavior remains unchanged—the functions still parse function signatures via
toString()to extract destructured parameters.code/renderers/react/src/preset.ts (1)
11-11: LGTM!The re-export of
componentManifestGeneratorcleanly integrates the new manifest generation functionality into the React preset system.code/core/src/csf-tools/CsfFile.ts (1)
298-298: LGTM!The type narrowing from
t.Expression | undefinedtot.ObjectExpression | undefinedcorrectly reflects the actual usage of_metaNodethroughout the codebase. All assignments to this field confirm the value is anObjectExpressionbefore assignment (lines 479-496, 651-674, 809).code/core/src/types/modules/indexer.ts (1)
73-73: No action needed:componentPathtypes align.Pathis defined as an alias forstring, socomponentPath?: stringinStoryIndexEntryis consistent withBaseIndexEntry.code/renderers/react/src/component-manifest/docgen-resolver.ts (1)
14-18: Duplicate code is in sync. RESOLVE_EXTENSIONS and defaultLookupModule implementations match in both files; no further action needed.code/renderers/react/package.json (1)
69-69: Approve dependency versionThe
comment-parserpackage version ^1.4.1 matches the latest release and has no known security vulnerabilities.code/renderers/react/src/component-manifest/jsdoc-tags.test.ts (1)
1-34: LGTM!The test coverage for
extractJSDocTagsis well-structured. The tests validate:code/renderers/react/src/component-manifest/generator.test.ts (1)
1-265: LGTM!The test provides comprehensive coverage of the component manifest generator:
- Properly mocks the filesystem using memfs
- Seeds realistic test data for Button and Header components with multiple stories
- Validates the complete manifest output including descriptions, JSDoc tags, imports, and code snippets
- Uses inline snapshots for exact output verification
code/core/src/types/modules/core-common.ts (3)
348-356: LGTM!The new type definitions are well-structured:
ComponentManifestinterface with clear fields (id, name, examples)ComponentManifestGeneratortype that takes a StoryIndexGenerator and returns a Promise of component manifests
371-371: LGTM!The
componentManifestGeneratorfield is properly added toStorybookConfigRawwith the correct type.
470-470: LGTM!The
experimental_componentsManifestfeature flag is correctly added to the features configuration.code/renderers/react/src/component-manifest/generateCodeSnippet.test.tsx (1)
1-489: LGTM!Excellent comprehensive test coverage for code snippet generation:
- Covers multiple CSF versions (CSF1, CSF2, CSF3, CSF4)
- Tests various prop types (boolean, string, number, object, array, JSX)
- Tests complex scenarios (nested elements, multiple spreads, custom renders, invalid attributes)
- Tests deep nesting and args inlining
- Uses inline snapshots for precise output validation
code/renderers/react/src/component-manifest/generator.ts (1)
15-67: Approve the overall generator structure.The component manifest generator implementation is well-structured:
- Groups stories by component ID and selects a representative entry per component
- Reads both story and component files
- Integrates CSF parsing, react-docgen, and JSDoc extraction
- Generates code snippets for each story example
- Returns a properly typed manifest
code/renderers/react/src/component-manifest/jsdoc-tags.ts (1)
19-24: Tag removal is fineSimple and safe filter for lines starting with '@'. LGTM.
code/renderers/react/src/component-manifest/react-docgen.ts (1)
47-55: loadConfig usage is correct
loadConfig accepts a direct tsconfig.json path, so no changes are needed.
| const features = await presets.apply('features'); | ||
|
|
||
| if (features?.experimental_componentsManifest) { | ||
| const componentManifestGenerator: ComponentManifestGenerator = await presets.apply( | ||
| 'componentManifestGenerator' | ||
| ); | ||
| const indexGenerator = await initializedStoryIndexGenerator; | ||
| if (componentManifestGenerator && indexGenerator) { | ||
| const manifests = await componentManifestGenerator(indexGenerator); | ||
| await mkdir(join(options.outputDir, 'manifests'), { recursive: true }); | ||
| await writeFile( | ||
| join(options.outputDir, 'manifests', 'components.json'), | ||
| JSON.stringify(manifests) | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove shadowing variable and use existing features.
Line 168 creates a shadow variable features that was already defined and fetched on line 101. This is redundant and can lead to confusion.
Apply this diff to use the existing features variable:
- const features = await presets.apply('features');
-
- if (features?.experimental_componentsManifest) {
+ if (features?.experimental_componentsManifest) {📝 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.
| const features = await presets.apply('features'); | |
| if (features?.experimental_componentsManifest) { | |
| const componentManifestGenerator: ComponentManifestGenerator = await presets.apply( | |
| 'componentManifestGenerator' | |
| ); | |
| const indexGenerator = await initializedStoryIndexGenerator; | |
| if (componentManifestGenerator && indexGenerator) { | |
| const manifests = await componentManifestGenerator(indexGenerator); | |
| await mkdir(join(options.outputDir, 'manifests'), { recursive: true }); | |
| await writeFile( | |
| join(options.outputDir, 'manifests', 'components.json'), | |
| JSON.stringify(manifests) | |
| ); | |
| } | |
| } | |
| if (features?.experimental_componentsManifest) { | |
| const componentManifestGenerator: ComponentManifestGenerator = await presets.apply( | |
| 'componentManifestGenerator' | |
| ); | |
| const indexGenerator = await initializedStoryIndexGenerator; | |
| if (componentManifestGenerator && indexGenerator) { | |
| const manifests = await componentManifestGenerator(indexGenerator); | |
| await mkdir(join(options.outputDir, 'manifests'), { recursive: true }); | |
| await writeFile( | |
| join(options.outputDir, 'manifests', 'components.json'), | |
| JSON.stringify(manifests) | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In code/core/src/core-server/build-static.ts around lines 168 to 183 there is a
redundant re-declaration of const features = await presets.apply('features')
that shadows the features variable already fetched earlier (around line 101);
remove this second declaration and reuse the existing features variable in the
if-check (i.e., change the code to use the previously obtained features value),
ensuring the subsequent logic that reads
features?.experimental_componentsManifest remains unchanged and that TypeScript
types still align with the original features definition.
| app.use('/manifests/components.json', async (req, res) => { | ||
| try { | ||
| const features = await options.presets.apply('features'); | ||
| if (!features?.experimental_componentsManifest) { | ||
| const componentManifestGenerator: ComponentManifestGenerator = await options.presets.apply( | ||
| 'componentManifestGenerator' | ||
| ); | ||
| const indexGenerator = await initializedStoryIndexGenerator; | ||
| if (componentManifestGenerator && indexGenerator) { | ||
| const manifest = await componentManifestGenerator(indexGenerator); | ||
| res.setHeader('Content-Type', 'application/json'); | ||
| res.end(JSON.stringify(manifest)); | ||
| return; | ||
| } | ||
| } | ||
| res.statusCode = 400; | ||
| res.end('No component manifest generator configured.'); | ||
| return; | ||
| } catch (e) { | ||
| logger.error(e instanceof Error ? e : String(e)); | ||
| res.statusCode = 500; | ||
| res.end(e instanceof Error ? e.toString() : String(e)); | ||
| return; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Fix inverted feature flag logic.
Line 142 checks !features?.experimental_componentsManifest (NOT enabled), but the code inside this block generates and serves the manifest. This logic appears inverted - manifests should be generated when the flag IS enabled, not when it's disabled.
Apply this diff to fix the logic:
const features = await options.presets.apply('features');
- if (!features?.experimental_componentsManifest) {
+ if (features?.experimental_componentsManifest) {
const componentManifestGenerator: ComponentManifestGenerator = await options.presets.apply(
'componentManifestGenerator'
);📝 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.
| app.use('/manifests/components.json', async (req, res) => { | |
| try { | |
| const features = await options.presets.apply('features'); | |
| if (!features?.experimental_componentsManifest) { | |
| const componentManifestGenerator: ComponentManifestGenerator = await options.presets.apply( | |
| 'componentManifestGenerator' | |
| ); | |
| const indexGenerator = await initializedStoryIndexGenerator; | |
| if (componentManifestGenerator && indexGenerator) { | |
| const manifest = await componentManifestGenerator(indexGenerator); | |
| res.setHeader('Content-Type', 'application/json'); | |
| res.end(JSON.stringify(manifest)); | |
| return; | |
| } | |
| } | |
| res.statusCode = 400; | |
| res.end('No component manifest generator configured.'); | |
| return; | |
| } catch (e) { | |
| logger.error(e instanceof Error ? e : String(e)); | |
| res.statusCode = 500; | |
| res.end(e instanceof Error ? e.toString() : String(e)); | |
| return; | |
| } | |
| }); | |
| app.use('/manifests/components.json', async (req, res) => { | |
| try { | |
| const features = await options.presets.apply('features'); | |
| if (features?.experimental_componentsManifest) { | |
| const componentManifestGenerator: ComponentManifestGenerator = await options.presets.apply( | |
| 'componentManifestGenerator' | |
| ); | |
| const indexGenerator = await initializedStoryIndexGenerator; | |
| if (componentManifestGenerator && indexGenerator) { | |
| const manifest = await componentManifestGenerator(indexGenerator); | |
| res.setHeader('Content-Type', 'application/json'); | |
| res.end(JSON.stringify(manifest)); | |
| return; | |
| } | |
| } | |
| res.statusCode = 400; | |
| res.end('No component manifest generator configured.'); | |
| return; | |
| } catch (e) { | |
| logger.error(e instanceof Error ? e : String(e)); | |
| res.statusCode = 500; | |
| res.end(e instanceof Error ? e.toString() : String(e)); | |
| return; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In code/core/src/core-server/dev-server.ts around lines 139 to 163, the feature
flag check is inverted: the block that generates and serves the component
manifest runs when !features?.experimental_componentsManifest (i.e. when the
flag is false or missing). Change the condition to check for the flag being
enabled (features?.experimental_componentsManifest) so the manifest is generated
only when the experimental_componentsManifest feature is true; otherwise keep
returning the 400 "No component manifest generator configured." response. Ensure
the negation is removed and the surrounding flow/returns remain unchanged.
| export const RESOLVE_EXTENSIONS = [ | ||
| '.js', | ||
| '.cts', // These were originally not in the code, I added them | ||
| '.mts', // These were originally not in the code, I added them | ||
| '.ctsx', // These were originally not in the code, I added them | ||
| '.mtsx', // These were originally not in the code, I added them | ||
| '.ts', | ||
| '.tsx', | ||
| '.mjs', | ||
| '.cjs', | ||
| '.mts', | ||
| '.cts', | ||
| '.jsx', | ||
| ]; |
There was a problem hiding this comment.
Duplicate extension entries detected.
Lines 25-27 and 32-33 both include .cts and .mts extensions, resulting in duplicates within the RESOLVE_EXTENSIONS array. This could lead to unnecessary resolution attempts.
Apply this diff to remove the duplicates:
export const RESOLVE_EXTENSIONS = [
'.js',
'.cts', // These were originally not in the code, I added them
'.mts', // These were originally not in the code, I added them
'.ctsx', // These were originally not in the code, I added them
'.mtsx', // These were originally not in the code, I added them
'.ts',
'.tsx',
'.mjs',
'.cjs',
- '.mts',
- '.cts',
'.jsx',
];📝 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 RESOLVE_EXTENSIONS = [ | |
| '.js', | |
| '.cts', // These were originally not in the code, I added them | |
| '.mts', // These were originally not in the code, I added them | |
| '.ctsx', // These were originally not in the code, I added them | |
| '.mtsx', // These were originally not in the code, I added them | |
| '.ts', | |
| '.tsx', | |
| '.mjs', | |
| '.cjs', | |
| '.mts', | |
| '.cts', | |
| '.jsx', | |
| ]; | |
| export const RESOLVE_EXTENSIONS = [ | |
| '.js', | |
| '.cts', // These were originally not in the code, I added them | |
| '.mts', // These were originally not in the code, I added them | |
| '.ctsx', // These were originally not in the code, I added them | |
| '.mtsx', // These were originally not in the code, I added them | |
| '.ts', | |
| '.tsx', | |
| '.mjs', | |
| '.cjs', | |
| '.jsx', | |
| ]; |
🤖 Prompt for AI Agents
In code/renderers/react/src/component-manifest/docgen-resolver.ts around lines
22 to 35, the RESOLVE_EXTENSIONS array contains duplicate entries for '.cts' and
'.mts' which should be removed; update the array to include each extension only
once (remove the repeated '.cts' and '.mts' entries) while preserving the
intended ordering of unique extensions.
| const keyOf = (p: t.ObjectProperty): string | null => | ||
| t.isIdentifier(p.key) ? p.key.name : t.isStringLiteral(p.key) ? p.key.value : null; | ||
|
|
||
| const isValidJsxAttrName = (n: string) => /^[A-Za-z_][A-Za-z0-9_.:-]*$/.test(n); |
There was a problem hiding this comment.
Fix invalid JSX attribute construction ('.' allowed; ':' not handled)
isValidJsxAttrName permits '.', and toAttr always builds a JSXIdentifier. This can generate invalid AST (e.g., foo.bar is not a valid attribute identifier). Also, names with : should produce JSXNamespacedName.
-const isValidJsxAttrName = (n: string) => /^[A-Za-z_][A-Za-z0-9_.:-]*$/.test(n);
+// Valid for JSXIdentifier (hyphens allowed), but exclude '.'; handle ':' via JSXNamespacedName
+const isValidJsxAttrName = (n: string) => /^[A-Za-z_][A-Za-z0-9_-]*(:[A-Za-z_][A-Za-z0-9_-]*)?$/.test(n);
-const toAttr = (key: string, value: t.Node): t.JSXAttribute | null => {
+const toAttrName = (key: string): t.JSXIdentifier | t.JSXNamespacedName | null => {
+ if (!isValidJsxAttrName(key)) return null;
+ if (key.includes(':')) {
+ const [ns, local] = key.split(':', 2);
+ return t.jsxNamespacedName(t.jsxIdentifier(ns), t.jsxIdentifier(local));
+ }
+ return t.jsxIdentifier(key);
+};
+
+const toAttr = (key: string, value: t.Node): t.JSXAttribute | null => {
if (t.isBooleanLiteral(value)) {
- return value.value ? t.jsxAttribute(t.jsxIdentifier(key), null) : null;
+ if (!value.value) return null;
+ const name = toAttrName(key);
+ return name ? t.jsxAttribute(name, null) : null;
}
if (t.isStringLiteral(value)) {
- return t.jsxAttribute(t.jsxIdentifier(key), t.stringLiteral(value.value));
+ const name = toAttrName(key);
+ return name ? t.jsxAttribute(name, t.stringLiteral(value.value)) : null;
}
if (t.isNullLiteral(value)) {
return null;
}
if (t.isIdentifier(value) && value.name === 'undefined') {
return null;
}
if (t.isExpression(value)) {
- return t.jsxAttribute(t.jsxIdentifier(key), t.jsxExpressionContainer(value));
+ const name = toAttrName(key);
+ return name ? t.jsxAttribute(name, t.jsxExpressionContainer(value)) : null;
}
return null; // non-expression nodes are not valid as attribute values
};Also applies to: 292-313
| const docgens = await parseWithReactDocgen({ | ||
| code: componentFile, | ||
| filename: path.join(process.cwd(), entry.importPath), | ||
| }); | ||
| const docgen = getMatchingDocgen(docgens, componentName); |
There was a problem hiding this comment.
Fix filename parameter mismatch in react-docgen.
Line 38 passes entry.importPath as the filename parameter to parseWithReactDocgen, but the code parameter (line 37) contains componentFile which was read from entry.componentPath (line 31). The filename should match the file whose code is being parsed.
Apply this diff:
const docgens = await parseWithReactDocgen({
code: componentFile,
- filename: path.join(process.cwd(), entry.importPath),
+ filename: path.join(process.cwd(), entry.componentPath!),
});📝 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.
| const docgens = await parseWithReactDocgen({ | |
| code: componentFile, | |
| filename: path.join(process.cwd(), entry.importPath), | |
| }); | |
| const docgen = getMatchingDocgen(docgens, componentName); | |
| const docgens = await parseWithReactDocgen({ | |
| code: componentFile, | |
| filename: path.join(process.cwd(), entry.componentPath!), | |
| }); | |
| const docgen = getMatchingDocgen(docgens, componentName); |
🤖 Prompt for AI Agents
In code/renderers/react/src/component-manifest/generator.ts around lines 36 to
40, the filename passed to parseWithReactDocgen incorrectly uses
entry.importPath while the code string was read from entry.componentPath; update
the filename argument to match the source of componentFile (e.g., use
path.join(process.cwd(), entry.componentPath) or otherwise pass
entry.componentPath) so the filename aligns with the parsed code.
| export function extractJSDocTags(description: string) { | ||
| const lines = description.split('\n'); | ||
| const jsDoc = ['/**', ...lines.map((line) => ` * ${line}`), ' */'].join('\n'); | ||
|
|
||
| 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}`), | ||
| ]) | ||
| ); |
There was a problem hiding this comment.
Harden return type and guard undefined tags
groupBy returns Partial and tags?.map(...) yields string[] | undefined. Also parsed[0] may be absent. Add an explicit return type and default to empty arrays to make the API stable.
-export function extractJSDocTags(description: string) {
+export function extractJSDocTags(description: string): Record<string, string[]> {
const lines = description.split('\n');
const jsDoc = ['/**', ...lines.map((line) => ` * ${line}`), ' */'].join('\n');
- const parsed = parse(jsDoc);
+ const parsed = parse(jsDoc);
+ const block = parsed[0] ?? ({ tags: [] } as { tags: Array<{ tag: string; type?: string; name?: string; description?: string }> });
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}`),
- ])
+ Object.entries(groupBy(block.tags, (it) => it.tag)).map(([key, arr]) => [
+ key,
+ (arr ?? []).map((tag) => {
+ const pieces = [];
+ if (tag.type) pieces.push(`{${tag.type}}`);
+ if (tag.name) pieces.push(tag.name);
+ if (tag.description) pieces.push(tag.description);
+ return pieces.join(' ').trim();
+ }),
+ ])
);
}📝 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 function extractJSDocTags(description: string) { | |
| const lines = description.split('\n'); | |
| const jsDoc = ['/**', ...lines.map((line) => ` * ${line}`), ' */'].join('\n'); | |
| 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 extractJSDocTags(description: string): Record<string, string[]> { | |
| const lines = description.split('\n'); | |
| const jsDoc = ['/**', ...lines.map((line) => ` * ${line}`), ' */'].join('\n'); | |
| const parsed = parse(jsDoc); | |
| const block = parsed[0] ?? ({ | |
| tags: [], | |
| } as { | |
| tags: Array<{ | |
| tag: string; | |
| type?: string; | |
| name?: string; | |
| description?: string; | |
| }>; | |
| }); | |
| return Object.fromEntries( | |
| Object.entries(groupBy(block.tags, (it) => it.tag)).map(([key, arr]) => [ | |
| key, | |
| (arr ?? []).map((tag) => { | |
| const pieces = []; | |
| if (tag.type) pieces.push(`{${tag.type}}`); | |
| if (tag.name) pieces.push(tag.name); | |
| if (tag.description) pieces.push(tag.description); | |
| return pieces.join(' ').trim(); | |
| }), | |
| ]) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In code/renderers/react/src/component-manifest/jsdoc-tags.ts around lines 5 to
16, the function currently returns possibly undefined arrays because groupBy
returns a Partial and parsed[0] may be absent; change the function signature to
an explicit return type like Record<string, string[]>, guard parsed[0] by
checking parsed.length and default to an empty tags array (e.g. const tags =
parsed[0]?.tags ?? []), pass that tags array to groupBy, and when building the
result ensure every key maps to a definite string[] (use || [] or map to [] when
tags is undefined) so the function never returns undefined values.
| if (result.includes(`${sep}react-native${sep}index.js`)) { | ||
| const replaced = result.replace( | ||
| `${sep}react-native${sep}index.js`, | ||
| `${sep}react-native-web${sep}dist${sep}index.js` | ||
| ); | ||
| if (existsSync(replaced)) { | ||
| if (RESOLVE_EXTENSIONS.find((ext) => result.endsWith(ext))) { | ||
| return replaced; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Check extension on the replaced path (RN→RNW)
You’re validating extensions against result even when returning replaced. Validate replaced instead for correctness.
- if (existsSync(replaced)) {
- if (RESOLVE_EXTENSIONS.find((ext) => result.endsWith(ext))) {
- return replaced;
- }
- }
+ if (existsSync(replaced) && RESOLVE_EXTENSIONS.some((ext) => replaced.endsWith(ext))) {
+ return replaced;
+ }🤖 Prompt for AI Agents
In code/renderers/react/src/component-manifest/react-docgen.ts around lines 86
to 96, the code checks file extensions against the original `result` path even
after constructing a `replaced` path for react-native → react-native-web; update
the extension validation to test `replaced` instead of `result` (i.e., call
RESOLVE_EXTENSIONS.find(ext => replaced.endsWith(ext))) so you only return
`replaced` when it both exists and has a valid extension.
Closes #
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
/manifests/components.jsondevelopment server endpoint that serves generated component manifest data