Core: Fix react-docgen-typescript support in story creation
#33586
Conversation
📝 WalkthroughWalkthroughThis PR reorganizes React DocGen extraction utilities by migrating from a single extractor module to a multi-extractor architecture. It introduces support for both react-docgen and react-docgen-typescript tools, consolidates shared types and helpers into a utils module, and updates the preset system to route between extractors based on configuration. Changes
Sequence DiagramsequenceDiagram
participant Client as Preset Handler
participant Router as internal_getArgTypesData
participant Extractor1 as extractArgTypesFromDocgen
participant Extractor2 as extractArgTypesFromDocgenTypescript
participant Mapper1 as mapReactDocgenToArgType
participant Mapper2 as mapReactDocgenTypescriptToArgType
participant Utils as utils (mapCommonTypes)
Client->>Router: internal_getArgTypesData(options, presets)
activate Router
Router->>Router: Check preset reactDocgen value
alt reactDocgen === 'react-docgen-typescript'
Router->>Extractor2: extractArgTypesFromDocgenTypescript()
activate Extractor2
Extractor2->>Extractor2: Load react-docgen-typescript parser
Extractor2->>Extractor2: Parse component file
loop For each prop
Extractor2->>Mapper2: mapReactDocgenTypescriptToArgType(typeString)
activate Mapper2
Mapper2->>Utils: mapCommonTypes(typeName)
Utils-->>Mapper2: SBType | null
Mapper2-->>Extractor2: SBType
deactivate Mapper2
end
Extractor2-->>Router: ComponentArgTypesData | null
deactivate Extractor2
else reactDocgen !== false
Router->>Extractor1: extractArgTypesFromDocgen()
activate Extractor1
Extractor1->>Extractor1: Parse component file with react-docgen
loop For each prop
Extractor1->>Mapper1: mapReactDocgenToArgType(docgenType)
activate Mapper1
Mapper1->>Utils: mapCommonTypes(typeName)
Utils-->>Mapper1: SBType | null
Mapper1-->>Extractor1: SBType
deactivate Mapper1
end
Extractor1-->>Router: ComponentArgTypesData | null
deactivate Extractor1
end
Router-->>Client: ComponentArgTypesData | null
deactivate Router
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.test.ts`:
- Around line 188-191: The vi.mock call that stubs 'react-docgen-typescript' is
currently inside a test and must be moved to the top of the test file so it is
hoisted correctly; change the module mock invocation to
vi.mock('react-docgen-typescript', { spy: true, }, ) at file top (outside tests)
and remove the in-test vi.mock; then configure mock behavior (e.g. make
withCompilerOptions return mockParser) inside a beforeEach block by setting the
mock implementation/returnValue on the mocked withCompilerOptions function
(referencing withCompilerOptions and mockParser) so each test gets a fresh,
configured mock.
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts`:
- Around line 63-78: The union-handling logic in
extractReactTypescriptDocgenInfo that splits on ' | ' can break for nested or
generics (e.g., "((a: string) => void) | null" or "Map<string, A | B>"); update
the union parsing block (the if (trimmed.includes(' | ')) branch) to add a clear
inline comment documenting this limitation and examples, and mention a
recommended future improvement (parse with the TypeScript AST or a proper
tokenizer) so future maintainers understand why splitting was chosen and the
known edge cases.
🧹 Nitpick comments (5)
code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.ts (1)
137-162: Consider adding explicit return type annotation for clarity.The function's return type is inferred, but given it's part of the public API, an explicit return type would improve maintainability and documentation.
Suggested improvement
-export const extractArgTypesFromDocgen = ({ +export const extractArgTypesFromDocgen = ({ componentFilePath, componentExportName, -}: GetArgTypesDataOptions) => { +}: GetArgTypesDataOptions): ComponentArgTypesData | null => {code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.test.ts (1)
43-45: Track the@ts-expect-errorcomments for future cleanup.Multiple test assertions use
@ts-expect-errorwith "fix this later" comments. This suggests the return type structure fromgetComponentDocgencould be refined to avoid these type errors.Would you like me to open an issue to track cleaning up these type assertions, or help refine the return type of
getComponentDocgento eliminate the need for@ts-expect-error?Also applies to: 76-78, 109-115, 194-196, 294-296
code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.test.ts (1)
12-15: MovebeforeEachinside the describe blocks for better isolation.Per coding guidelines, mock behaviors should be in
beforeEachblocks, and related mocks should be grouped together. The current module-levelbeforeEachapplies to all tests, but the filesystem setup is only needed forextractArgTypesFromDocgenTypescripttests, not the pure function unit tests.Suggested improvement
-beforeEach(() => { - vi.spyOn(process, 'cwd').mockReturnValue('/app'); - vol.fromJSON(fsMocks, '/app'); -}); - describe('mapReactDocgenTypescriptToArgType', () => { test('maps primitive types correctly', () => {Move the
beforeEachinto theextractArgTypesFromDocgenTypescriptdescribe block where it's actually needed:describe('extractArgTypesFromDocgenTypescript', () => { + beforeEach(() => { + vi.spyOn(process, 'cwd').mockReturnValue('/app'); + vol.fromJSON(fsMocks, '/app'); + }); + test('extracts arg types from a simple component', async () => {code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts (2)
88-91: Potential false positive: numeric string detection.
!isNaN(Number(trimmed))returnstruefor empty strings and whitespace (sinceNumber('')is0andNumber(' ')is0). While the input is trimmed, iftrimmedwere empty, this would incorrectly return{ name: 'literal', value: 0 }.Given the earlier
trimmed = typeString.trim()and the various checks before this point, an empty string is unlikely to reach here, but adding a guard would be defensive:Suggested improvement
// Handle literal number values - if (!isNaN(Number(trimmed))) { + if (trimmed !== '' && !isNaN(Number(trimmed))) { return { name: 'literal', value: Number(trimmed) }; }
148-150: Component matching by filePath could produce unexpected results.The fallback
d.filePath?.includes(componentExportName)matches if the component name appears anywhere in the file path, which could match unintended components (e.g., searching for "Button" might match "/components/ButtonGroup/ButtonGroup.tsx").Consider using a more precise match:
Suggested improvement
const match = docgens.find( - (d) => d.displayName === componentExportName || d.filePath?.includes(componentExportName) + (d) => d.displayName === componentExportName );The
displayNamematch should be sufficient since react-docgen-typescript populates it for exported components. If filePath matching is truly needed, consider matching just the filename portion.
| vi.mock('react-docgen-typescript', () => ({ | ||
| withCompilerOptions: vi.fn().mockReturnValue(mockParser), | ||
| })); | ||
|
|
There was a problem hiding this comment.
Mock placement issue: vi.mock is called inside a test case.
Per coding guidelines, mocks should be placed at the top of the test file before any test cases, not inside individual tests. Additionally, vi.mock is hoisted by Vitest, so calling it inside a test doesn't work as expected - the mock applies to the entire module.
Suggested fix
Move the mock to the top of the file and configure behavior in beforeEach:
+vi.mock('react-docgen-typescript', { spy: true });
+
+// At module top, after imports
+
describe('extractArgTypesFromDocgenTypescript', () => {
+ beforeEach(() => {
+ vi.spyOn(process, 'cwd').mockReturnValue('/app');
+ vol.fromJSON(fsMocks, '/app');
+
+ // Configure mock behavior here
+ });
+
test('extracts arg types from a simple component', async () => {
- // ...
- vi.mock('react-docgen-typescript', () => ({
- withCompilerOptions: vi.fn().mockReturnValue(mockParser),
- }));As per coding guidelines, mocks should use vi.mock() with the spy: true option and mock implementations should be in beforeEach blocks.
🤖 Prompt for AI Agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.test.ts`
around lines 188 - 191, The vi.mock call that stubs 'react-docgen-typescript' is
currently inside a test and must be moved to the top of the test file so it is
hoisted correctly; change the module mock invocation to
vi.mock('react-docgen-typescript', { spy: true, }, ) at file top (outside tests)
and remove the in-test vi.mock; then configure mock behavior (e.g. make
withCompilerOptions return mockParser) inside a beforeEach block by setting the
mock implementation/returnValue on the mocked withCompilerOptions function
(referencing withCompilerOptions and mockParser) so each test gets a fresh,
configured mock.
| // Handle union types: Type1 | Type2 | ||
| if (trimmed.includes(' | ')) { | ||
| const parts = trimmed.split(' | ').map((p) => p.trim()); | ||
| // Check if it's a literal union (all parts are string literals) | ||
| const allLiterals = parts.every((p) => p.startsWith('"') || p.startsWith("'")); | ||
| if (allLiterals) { | ||
| return { | ||
| name: 'union', | ||
| value: parts.map((p) => ({ | ||
| name: 'literal' as const, | ||
| value: p.replace(/^["']|["']$/g, ''), | ||
| })), | ||
| }; | ||
| } | ||
| return { name: 'union', value: parts.map(mapReactDocgenTypescriptToArgType) }; | ||
| } |
There was a problem hiding this comment.
Union parsing with | split may fail for complex nested types.
Splitting on | works for simple unions but could incorrectly split types like ((a: string) => void) | null or generic types with unions inside angle brackets like Map<string, A | B>.
For the current use case with react-docgen-typescript output, this is likely acceptable since complex nested unions are typically represented differently. However, be aware this is a known limitation.
Consider adding a comment to document this limitation:
// Handle union types: Type1 | Type2
+ // Note: Simple split on ' | ' - may not correctly handle deeply nested unions
if (trimmed.includes(' | ')) {📝 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.
| // Handle union types: Type1 | Type2 | |
| if (trimmed.includes(' | ')) { | |
| const parts = trimmed.split(' | ').map((p) => p.trim()); | |
| // Check if it's a literal union (all parts are string literals) | |
| const allLiterals = parts.every((p) => p.startsWith('"') || p.startsWith("'")); | |
| if (allLiterals) { | |
| return { | |
| name: 'union', | |
| value: parts.map((p) => ({ | |
| name: 'literal' as const, | |
| value: p.replace(/^["']|["']$/g, ''), | |
| })), | |
| }; | |
| } | |
| return { name: 'union', value: parts.map(mapReactDocgenTypescriptToArgType) }; | |
| } | |
| // Handle union types: Type1 | Type2 | |
| // Note: Simple split on ' | ' - may not correctly handle deeply nested unions | |
| if (trimmed.includes(' | ')) { | |
| const parts = trimmed.split(' | ').map((p) => p.trim()); | |
| // Check if it's a literal union (all parts are string literals) | |
| const allLiterals = parts.every((p) => p.startsWith('"') || p.startsWith("'")); | |
| if (allLiterals) { | |
| return { | |
| name: 'union', | |
| value: parts.map((p) => ({ | |
| name: 'literal' as const, | |
| value: p.replace(/^["']|["']$/g, ''), | |
| })), | |
| }; | |
| } | |
| return { name: 'union', value: parts.map(mapReactDocgenTypescriptToArgType) }; | |
| } |
🤖 Prompt for AI Agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts`
around lines 63 - 78, The union-handling logic in
extractReactTypescriptDocgenInfo that splits on ' | ' can break for nested or
generics (e.g., "((a: string) => void) | null" or "Map<string, A | B>"); update
the union parsing block (the if (trimmed.includes(' | ')) branch) to add a clear
inline comment documenting this limitation and examples, and mention a
recommended future improvement (parse with the TypeScript AST or a proper
tokenizer) so future maintainers understand why splitting was chosen and the
known edge cases.
|
View your CI Pipeline Execution ↗ for commit bb7f266
☁️ Nx Cloud last updated this comment at |
yannbf
force-pushed
the
yann/story-creation-docgen-ts
branch
from
a1daad6 to
d2323bb
Compare
ndelangen
changed the title
| const parser = withCompilerOptions( | ||
| { esModuleInterop: true, jsx: 2 /* React */ }, | ||
| mergedOptions | ||
| ); |
There was a problem hiding this comment.
You might look into sharing this over multiple invocations of extractArgTypesFromDocgenTypescript since currently the typescript runtime Program gets re-instantiated every call.
There was a problem hiding this comment.
But likely, performance isn't the highest priority right now?
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 538 | 538 | 0 |
| Self size | 646 KB | 646 KB | 🎉 -130 B 🎉 |
| Dependency size | 59.22 MB | 59.28 MB | 🚨 +58 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 127 | 127 | 0 |
| Self size | 1.12 MB | 1.12 MB | 🎉 -22 B 🎉 |
| Dependency size | 21.82 MB | 21.87 MB | 🚨 +49 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 159 | 159 | 0 |
| Self size | 30 KB | 30 KB | 🎉 -10 B 🎉 |
| Dependency size | 23.00 MB | 23.17 MB | 🚨 +164 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 117 | 117 | 0 |
| Self size | 35 KB | 35 KB | 🎉 -44 B 🎉 |
| Dependency size | 19.62 MB | 19.66 MB | 🚨 +49 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 278 | 278 | 0 |
| Self size | 24 KB | 24 KB | 🚨 +2 B 🚨 |
| Dependency size | 44.13 MB | 44.19 MB | 🚨 +58 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 775 KB | 775 KB | 🚨 +139 B 🚨 |
| Dependency size | 67.38 MB | 67.46 MB | 🚨 +79 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 176 | 0 |
| Self size | 30 KB | 30 KB | 🎉 -4 B 🎉 |
| Dependency size | 65.95 MB | 66.03 MB | 🚨 +79 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 57 | 57 | 0 |
| Self size | 732 KB | 781 KB | 🚨 +49 KB 🚨 |
| Dependency size | 12.94 MB | 12.94 MB | 🚨 +409 B 🚨 |
| Bundle Size Analyzer | Link | Link |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.ts`:
- Around line 150-158: The loop in extractReactDocgenInfo.ts currently ignores
props that lack tsType; update the logic in the for-loop that iterates over
props (where tsType is derived from (propInfo as
ReactDocgenPropDescriptor).tsType) to fall back to (propInfo as
ReactDocgenPropDescriptor).type when tsType is undefined, and pass that fallback
into mapReactDocgenToArgType so props defined with PropTypes/type are preserved;
keep setting required via (propInfo as ReactDocgenPropDescriptor).required and
assign the result into mapped.props![propName]. Ensure mapReactDocgenToArgType
can accept the docgen "type" shape or normalize it before mapping.
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts`:
- Around line 145-155: The current matching logic in
extractReactTypescriptDocgenInfo.ts uses filePath.includes(componentExportName)
which can produce false positives (e.g., "Button" matching "IconButton"); update
the matching to only consider component identifiers like displayName,
actualName, or exportName from each docgen entry: change the predicate in
docgens.find(...) to (d) => d.displayName === componentExportName ||
d.actualName === componentExportName || d.exportName === componentExportName,
removing the filePath.includes check; if you must keep file path matching,
replace it with a strict basename match (compare the file's basename without
extension to componentExportName) rather than substring includes.
In `@code/renderers/react/src/preset.ts`:
- Around line 91-114: The code ignores explicit
GetArgTypesDataOptions.reactDocgen and .reactDocgenTypescriptOptions because it
only reads them from presets; update the logic in the function that uses
InternalGetArgTypesDataOptions so that you first collect typescriptOptions from
presets.apply('typescript', {}) (as now) but then merge the explicit options
from the incoming options object into that result with the incoming options
taking priority (i.e., override preset values with options.reactDocgen and
options.reactDocgenTypescriptOptions if they are provided), and ensure the
merged values are then used for the reactDocgen / reactDocgenTypescriptOptions
checks (preserve current fallback to defaults and the reactDocgen === false
early return).
🧹 Nitpick comments (2)
code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.test.ts (2)
1-13: Reset mocks/volume after each test to avoid leakage.The cwd spy and memfs volume can persist across suites if not restored.
♻️ Suggested cleanup
-import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; @@ beforeEach(() => { vi.spyOn(process, 'cwd').mockReturnValue('/app'); vol.fromJSON(fsMocks, '/app'); }); + afterEach(() => { + vi.restoreAllMocks(); + vol.reset(); + });
40-45: Replace@ts-expect-errorwith a typed success helper.You already assert success; a small helper can narrow the type and avoid suppressions across the suite.
💡 Example helper
const assertSuccess = (result: ReturnType<typeof getComponentDocgen>) => { if (!result || result.reactDocgen.type !== 'success') { throw new Error('Expected react-docgen success result'); } return result.reactDocgen.data; }; // usage const data = assertSuccess(result); expect(data.displayName).toBe('Button');
| for (const [propName, propInfo] of Object.entries(props)) { | ||
| const tsType = (propInfo as ReactDocgenPropDescriptor).tsType as ReactDocgenTsType | undefined; | ||
| if (!tsType) { | ||
| continue; | ||
| } | ||
| mapped.props![propName] = { | ||
| required: Boolean((propInfo as ReactDocgenPropDescriptor).required), | ||
| type: mapReactDocgenToArgType(tsType), | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.ts | head -180 | tail -40Repository: storybookjs/storybook
Length of output: 1534
🏁 Script executed:
rg "ReactDocgenPropDescriptor|ReactDocgenTsType" code/renderers/react --type ts -A 3 -B 1Repository: storybookjs/storybook
Length of output: 4975
🏁 Script executed:
rg "mapReactDocgenToArgType" code/renderers/react --type ts -A 10 -B 2Repository: storybookjs/storybook
Length of output: 7471
🏁 Script executed:
rg "react-docgen" code/renderers/react --type ts --type json -A 2 -B 1 | head -100Repository: storybookjs/storybook
Length of output: 9505
🏁 Script executed:
rg "type PropDescriptor" --type ts -A 10 code/renderers/reactRepository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.test.ts | head -100Repository: storybookjs/storybook
Length of output: 3555
🏁 Script executed:
rg "propInfo.*\.type|\.type\s*as" code/renderers/react --type ts -B 2 -A 2Repository: storybookjs/storybook
Length of output: 997
🏁 Script executed:
python3 << 'EOF'
import subprocess
import json
# Look for node_modules or check package.json to understand what fields are in PropDescriptor
result = subprocess.run(['find', 'code/renderers/react/node_modules', '-name', 'index.d.ts', '-path', '*react-docgen*'],
capture_output=True, text=True, timeout=10)
print("Found type files:")
print(result.stdout)
EOFRepository: storybookjs/storybook
Length of output: 151
🏁 Script executed:
rg "flowType|\.type" code/renderers/react/src/componentManifest/reactDocgen --type ts -B 1 -A 1 | head -50Repository: storybookjs/storybook
Length of output: 3116
🏁 Script executed:
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts | head -80Repository: storybookjs/storybook
Length of output: 3290
🏁 Script executed:
rg "PropDescriptor" code/renderers/react/src --type ts -B 2 -A 5Repository: storybookjs/storybook
Length of output: 2456
🏁 Script executed:
rg "tsType|flowType" code/renderers/react --type ts -B 2 -A 2Repository: storybookjs/storybook
Length of output: 24008
🏁 Script executed:
rg "propInfo\.type\b|propInfo\['type'\]" code/renderers/react --type ts -B 3 -A 3Repository: storybookjs/storybook
Length of output: 1390
🏁 Script executed:
rg "PropDescriptor|type.*flowType" --type ts -B 3 -A 3 code/renderers/react | head -80Repository: storybookjs/storybook
Length of output: 2345
🏁 Script executed:
rg "PropTypes|\.js" code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.test.ts -B 3 -A 3Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
rg "handler|resolver|tsType|flowType" code/renderers/react/src/componentManifest/reactDocgen.ts -B 2 -A 2 | head -100Repository: storybookjs/storybook
Length of output: 644
🏁 Script executed:
rg "propInfo\[" code/renderers/react --type ts -B 2 -A 2Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
python3 << 'EOF'
import json
import subprocess
# Try to find react-docgen package and check its types
result = subprocess.run(
['find', 'code/renderers/react', '-type', 'f', '-name', 'package.json'],
capture_output=True, text=True
)
for pkg_json in result.stdout.strip().split('\n')[:3]:
if pkg_json:
print(f"\n=== {pkg_json} ===")
result = subprocess.run(['head', '-30', pkg_json], capture_output=True, text=True)
print(result.stdout)
EOFRepository: storybookjs/storybook
Length of output: 982
🏁 Script executed:
rg "description|deprecation|flowType|tsType" code/renderers/react/src/componentManifest/reactDocgen --type ts -B 2 -A 1 | head -60Repository: storybookjs/storybook
Length of output: 5790
🏁 Script executed:
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.test.ts | tail -150Repository: storybookjs/storybook
Length of output: 4636
🌐 Web query:
react-docgen v8 PropDescriptor tsType type fields structure
💡 Result:
Short answer — in react-docgen v8 a prop's tsType is a TypeDescriptor, which is a tagged union. The possible shapes and their important fields are:
-
TSFunctionSignatureType (function signature)
- signature: { arguments: Array, return?: TypeDescriptor, this?: TypeDescriptor }.
- each FunctionArgumentType: { name: string; rest?: boolean; type?: TypeDescriptor }.[1]
-
ObjectSignatureType (object / interface)
- properties: Array<{ key: TypeDescriptor | string; value: TypeDescriptor; description?: string }>.
- constructor?: TypeDescriptor.[1]
-
ElementsType (array/tuple of element types)
- elements: Array; name: 'elements' (type kind).[1]
-
LiteralType
- raw, value (literal details) — represents literal types (e.g. "foo", 1). [1][2]
-
SimpleType
- name: string; raw?: string — basic named types (e.g. 'string', 'number', 'union', etc.).[1][2]
Notes:
- TypeDescriptor is defined as ElementsType | LiteralType | ObjectSignatureType | SimpleType | T (so a tsType may itself be a TS function-signature node).
- Use the getTSType / getTypeAnnotation utilities to see how these are produced. [1][2]
Sources: react-docgen v8 type definitions. [1] API/index (dist .d.ts) and [2] package main type exports.
🏁 Script executed:
rg "\.jsx?|PropTypes" code/renderers/react/src/componentManifest/reactDocgen --type ts -B 3 -A 3 | head -80Repository: storybookjs/storybook
Length of output: 1812
🏁 Script executed:
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.ts | head -80Repository: storybookjs/storybook
Length of output: 3231
🏁 Script executed:
rg "flowType|description|deprecated" code/renderers/react --type ts -B 2 -A 2 | head -60Repository: storybookjs/storybook
Length of output: 4544
🏁 Script executed:
python3 << 'EOF'
# Search for any files that mention what fields PropDescriptor has
import subprocess
# Check if there are any .d.ts files or type stubs that describe PropDescriptor
result = subprocess.run(
['find', 'code/renderers/react', '-type', 'f', '-name', '*.d.ts'],
capture_output=True, text=True, timeout=10
)
files = [f for f in result.stdout.strip().split('\n') if f and 'react-docgen' in f]
for f in files[:5]:
print(f"=== {f} ===")
result = subprocess.run(['head', '-100', f], capture_output=True, text=True)
print(result.stdout)
print()
EOFRepository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
rg "tsType|type.*fallback" code/renderers/react/src/componentManifest/reactDocgen -B 3 -A 3Repository: storybookjs/storybook
Length of output: 21891
🏁 Script executed:
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.ts | sed -n '19,76p'Repository: storybookjs/storybook
Length of output: 2420
🏁 Script executed:
python3 << 'EOF'
import subprocess
# Search for actual usage patterns of propInfo fields
result = subprocess.run(
['rg', 'propInfo\\s*\\.', 'code/renderers/react/src/componentManifest', '--type', 'ts', '-o'],
capture_output=True, text=True, timeout=10
)
# Get unique field accesses
fields = set()
for line in result.stdout.strip().split('\n'):
if line:
# Extract field name
parts = line.split('propInfo.')
if len(parts) > 1:
field = parts[1].split('?')[0].split('!')[0].split('[')[0].split(' ')[0].split('(')[0]
if field:
fields.add(field)
print("Fields accessed on propInfo:")
for f in sorted(fields):
print(f" - {f}")
EOFRepository: storybookjs/storybook
Length of output: 93
🏁 Script executed:
rg "propInfo\[|propInfo\." code/renderers/react/src/componentManifest --type ts -B 1 -A 1Repository: storybookjs/storybook
Length of output: 889
Add fallback to support props with only the type field.
The PropDescriptor has both tsType and type fields. The current code only checks tsType, causing props with only a type field to be dropped. This can happen with JavaScript/PropTypes components. Add a fallback to the type field:
🔧 Suggested fix
for (const [propName, propInfo] of Object.entries(props)) {
const tsType = (propInfo as ReactDocgenPropDescriptor).tsType as ReactDocgenTsType | undefined;
- if (!tsType) {
+ const fallbackType = (propInfo as ReactDocgenPropDescriptor).type as ReactDocgenTsType | undefined;
+ const docgenType = tsType ?? fallbackType;
+ if (!docgenType) {
continue;
}
mapped.props![propName] = {
required: Boolean((propInfo as ReactDocgenPropDescriptor).required),
- type: mapReactDocgenToArgType(tsType),
+ type: mapReactDocgenToArgType(docgenType),
};
}📝 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.
| for (const [propName, propInfo] of Object.entries(props)) { | |
| const tsType = (propInfo as ReactDocgenPropDescriptor).tsType as ReactDocgenTsType | undefined; | |
| if (!tsType) { | |
| continue; | |
| } | |
| mapped.props![propName] = { | |
| required: Boolean((propInfo as ReactDocgenPropDescriptor).required), | |
| type: mapReactDocgenToArgType(tsType), | |
| }; | |
| for (const [propName, propInfo] of Object.entries(props)) { | |
| const tsType = (propInfo as ReactDocgenPropDescriptor).tsType as ReactDocgenTsType | undefined; | |
| const fallbackType = (propInfo as ReactDocgenPropDescriptor).type as ReactDocgenTsType | undefined; | |
| const docgenType = tsType ?? fallbackType; | |
| if (!docgenType) { | |
| continue; | |
| } | |
| mapped.props![propName] = { | |
| required: Boolean((propInfo as ReactDocgenPropDescriptor).required), | |
| type: mapReactDocgenToArgType(docgenType), | |
| }; |
🤖 Prompt for AI Agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.ts`
around lines 150 - 158, The loop in extractReactDocgenInfo.ts currently ignores
props that lack tsType; update the logic in the for-loop that iterates over
props (where tsType is derived from (propInfo as
ReactDocgenPropDescriptor).tsType) to fall back to (propInfo as
ReactDocgenPropDescriptor).type when tsType is undefined, and pass that fallback
into mapReactDocgenToArgType so props defined with PropTypes/type are preserved;
keep setting required via (propInfo as ReactDocgenPropDescriptor).required and
assign the result into mapped.props![propName]. Ensure mapReactDocgenToArgType
can accept the docgen "type" shape or normalize it before mapping.
| // Find the matching component if a name is specified | ||
| let targetDocgen = docgens[0]; | ||
| if (componentExportName) { | ||
| const match = docgens.find( | ||
| (d) => d.displayName === componentExportName || d.filePath?.includes(componentExportName) | ||
| ); | ||
| if (match) { | ||
| targetDocgen = match; | ||
| } else { | ||
| // If specific component requested but not found, return null | ||
| return null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts | head -160Repository: storybookjs/storybook
Length of output: 6483
🏁 Script executed:
# Check the overall file structure
wc -l code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.tsRepository: storybookjs/storybook
Length of output: 159
🏁 Script executed:
# Search for usages of this function to understand how it's called
rg "extractReactTypescriptDocgenInfo|componentExportName" code/renderers/react/src/componentManifest/ -A 2 -B 2Repository: storybookjs/storybook
Length of output: 2958
🏁 Script executed:
# Look for test files related to this module
fd "extractReactTypescriptDocgenInfo" code/ --type fRepository: storybookjs/storybook
Length of output: 155
🏁 Script executed:
# Check if there are any components with similar naming patterns (Button, IconButton, etc.)
fd -e tsx -e ts "Button" code/renderers/react/src --max-one-result | head -20Repository: storybookjs/storybook
Length of output: 298
🏁 Script executed:
# Find test files related to reactDocgen
fd "test|spec" code/renderers/react/src/componentManifest/reactDocgen/ --type fRepository: storybookjs/storybook
Length of output: 150
🏁 Script executed:
# Search for tests in parent directory
fd "extractReactTypescriptDocgenInfo" code/ --type f -path "*test*" -o -path "*spec*"Repository: storybookjs/storybook
Length of output: 233
🏁 Script executed:
# Look for tests in the entire react renderer
find code/renderers/react -name "*.test.ts*" -o -name "*.spec.ts*" | grep -i "docgen"Repository: storybookjs/storybook
Length of output: 213
🏁 Script executed:
# Search for where this function is called
rg "extractArgTypesFromDocgenTypescript" code/renderers/react --type ts --type tsx -B 3 -A 3Repository: storybookjs/storybook
Length of output: 92
🏁 Script executed:
# Check react-docgen-typescript documentation behavior - look for examples in the codebase
rg "filePath" code/renderers/react/src/componentManifest/reactDocgen/ -B 2 -A 2Repository: storybookjs/storybook
Length of output: 3618
🏁 Script executed:
cat -n code/renderers/react/src/componentManifest/reactDocgen.test.ts | head -100Repository: storybookjs/storybook
Length of output: 3483
🏁 Script executed:
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.test.ts | head -150Repository: storybookjs/storybook
Length of output: 5423
🏁 Script executed:
# Look for where filePath comes from - check react-docgen-typescript output
rg "parser\.parse" code/renderers/react --type f -A 5 -B 5 | head -50Repository: storybookjs/storybook
Length of output: 90
🏁 Script executed:
# Check if there are any Button/IconButton components in the repo to understand real-world scenario
find code -name "Button.tsx" -o -name "IconButton.tsx" | head -20Repository: storybookjs/storybook
Length of output: 692
🏁 Script executed:
find code -name "IconButton.tsx"Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Look for how the function is actually called - check for real usages
rg "extractArgTypesFromDocgenTypescript" code --type file -B 5 -A 5Repository: storybookjs/storybook
Length of output: 93
🏁 Script executed:
# Check the other similar function that also uses filePath matching
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.ts | grep -A 20 "getComponentDocgen"Repository: storybookjs/storybook
Length of output: 1671
🏁 Script executed:
# Look specifically at the implementation details of how displayName and filePath are used
rg "displayName.*filePath|filePath.*displayName" code/renderers/react --type file -B 2 -A 2Repository: storybookjs/storybook
Length of output: 93
🏁 Script executed:
# Search for where extractArgTypesFromDocgenTypescript is actually called
rg "extractArgTypesFromDocgenTypescript" code/renderers/react -A 10 -B 5Repository: storybookjs/storybook
Length of output: 4542
🏁 Script executed:
# Look at componentManifest.ts to see how these functions are called
cat -n code/renderers/react/src/componentManifest/reactDocgen.ts | head -100Repository: storybookjs/storybook
Length of output: 3874
🏁 Script executed:
# Check what react-docgen-typescript actually returns for filePath
rg "react-docgen-typescript" code/renderers/react --max-count 5Repository: storybookjs/storybook
Length of output: 1504
🏁 Script executed:
# Let's examine the actual file paths in practice - check examples in the code
rg "filePath.*includes|includes.*filePath" code --type file -B 3 -A 3Repository: storybookjs/storybook
Length of output: 93
🏁 Script executed:
# Look at what react-docgen-typescript documentation says about filePath
# Let's check what the parser actually sets for filePath by looking at test outputs
cat -n code/renderers/react/src/componentManifest/reactDocgen.test.ts | grep -A 30 "filePath"Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Check how the similar matching logic works in the react-docgen version
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.ts | sed -n '93,125p'Repository: storybookjs/storybook
Length of output: 1371
🏁 Script executed:
# Search for any actual test cases of extractArgTypesFromDocgenTypescript
find code -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "extractArgTypesFromDocgenTypescript" 2>/dev/nullRepository: storybookjs/storybook
Length of output: 49
🏁 Script executed:
# Check what properties the docgen objects from react-docgen-typescript actually have
rg "ComponentDocgenResult|docgens\[" code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts -B 2 -A 5Repository: storybookjs/storybook
Length of output: 670
🏁 Script executed:
# Let's create a small test to see what react-docgen-typescript actually returns for filePath
# Check if there's any documentation or examples in package.json or node_modules
find code -name "package.json" | xargs grep -l "react-docgen-typescript" | head -1Repository: storybookjs/storybook
Length of output: 104
🏁 Script executed:
# Let's look at the actual interface expected from react-docgen-typescript
# Check what properties are documented or used
rg "react-docgen-typescript" code/renderers/react --max-count 3 -A 10 -B 2 | head -100Repository: storybookjs/storybook
Length of output: 8467
🏁 Script executed:
# Let's check if there are multiple components in the same file (which would trigger the matching issue)
rg "export.*Button|export.*Icon" code/renderers/react/src --max-count 10Repository: storybookjs/storybook
Length of output: 6313
🏁 Script executed:
# Create a Python script to understand the vulnerability more clearly
python3 << 'EOF'
# Demonstrate the substring matching issue
examples = [
("Button", "/path/to/Button.tsx"),
("Button", "/path/to/IconButton.tsx"), # FALSE POSITIVE
("Button", "/path/to/PrimaryButton.tsx"), # FALSE POSITIVE
("Button", "/path/to/button-icon.tsx"), # FALSE POSITIVE
("Icon", "/path/to/IconButton.tsx"), # Could match Icon when looking for IconButton
]
print("Substring matching issue with .includes():")
print("-" * 60)
for search, path in examples:
matches = search in path
print(f"'{search}' in '{path}': {matches}")
print("\n" + "="*60)
print("\nEndsWith fix (more strict):")
print("-" * 60)
extensions = [".tsx", ".ts", ".jsx", ".js"]
for search, path in examples:
matches = any(path.endswith(f"/{search}{ext}") for ext in extensions)
print(f"path.endsWith('/{search}.*'): {matches}")
EOFRepository: storybookjs/storybook
Length of output: 151
🏁 Script executed:
# Let's check how componentFilePath is used - it's a single file being parsed
cat -n code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts | sed -n '108,160p'Repository: storybookjs/storybook
Length of output: 2234
🏁 Script executed:
# Look for actual cases where multiple components might be in one file (the scenario that triggers the bug)
rg "export.*function|export.*const.*=.*\(\)" code/renderers/react/src/__test__/Button.tsx -B 2 -A 2Repository: storybookjs/storybook
Length of output: 47
Substring matching on filePath can incorrectly resolve the wrong component.
When multiple components exist in the same file, filePath.includes(componentExportName) will incorrectly match if one component name is a substring of another (e.g., "Button" matches "IconButton"). Since the parallel react-docgen implementation (in extractReactDocgenInfo.ts) relies only on displayName, actualName, and exportName—never on filePath—consider doing the same here. If filePath matching is needed, use a stricter approach (e.g., basename with extension).
🔧 Safer matching (example)
- const match = docgens.find(
- (d) => d.displayName === componentExportName || d.filePath?.includes(componentExportName)
- );
+ const match = docgens.find(
+ (d) =>
+ d.displayName === componentExportName ||
+ d.filePath?.endsWith(`/${componentExportName}.tsx`) ||
+ d.filePath?.endsWith(`/${componentExportName}.ts`) ||
+ d.filePath?.endsWith(`/${componentExportName}.jsx`) ||
+ d.filePath?.endsWith(`/${componentExportName}.js`)
+ );📝 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.
| // Find the matching component if a name is specified | |
| let targetDocgen = docgens[0]; | |
| if (componentExportName) { | |
| const match = docgens.find( | |
| (d) => d.displayName === componentExportName || d.filePath?.includes(componentExportName) | |
| ); | |
| if (match) { | |
| targetDocgen = match; | |
| } else { | |
| // If specific component requested but not found, return null | |
| return null; | |
| // Find the matching component if a name is specified | |
| let targetDocgen = docgens[0]; | |
| if (componentExportName) { | |
| const match = docgens.find( | |
| (d) => | |
| d.displayName === componentExportName || | |
| d.filePath?.endsWith(`/${componentExportName}.tsx`) || | |
| d.filePath?.endsWith(`/${componentExportName}.ts`) || | |
| d.filePath?.endsWith(`/${componentExportName}.jsx`) || | |
| d.filePath?.endsWith(`/${componentExportName}.js`) | |
| ); | |
| if (match) { | |
| targetDocgen = match; | |
| } else { | |
| // If specific component requested but not found, return null | |
| return null; |
🤖 Prompt for AI Agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts`
around lines 145 - 155, The current matching logic in
extractReactTypescriptDocgenInfo.ts uses filePath.includes(componentExportName)
which can produce false positives (e.g., "Button" matching "IconButton"); update
the matching to only consider component identifiers like displayName,
actualName, or exportName from each docgen entry: change the predicate in
docgens.find(...) to (d) => d.displayName === componentExportName ||
d.actualName === componentExportName || d.exportName === componentExportName,
removing the filePath.includes check; if you must keep file path matching,
replace it with a strict basename match (compare the file's basename without
extension to componentExportName) rather than substring includes.
| const { componentFilePath, componentExportName, presets } = (options ?? | ||
| {}) as InternalGetArgTypesDataOptions; | ||
|
|
||
| if (!componentFilePath) { | ||
| return null; | ||
| } | ||
|
|
||
| const argTypesData = extractArgTypesFromDocgen({ | ||
| componentFilePath: path.isAbsolute(componentFilePath) | ||
| ? componentFilePath | ||
| : join(getProjectRoot(), componentFilePath), | ||
| componentExportName, | ||
| }); | ||
| // Get typescript options from presets to determine which docgen to use | ||
| let typescriptOptions: TypescriptOptionsWithDocgen = {}; | ||
| if (presets) { | ||
| try { | ||
| const appliedOptions = await presets.apply('typescript', {}); | ||
| typescriptOptions = appliedOptions as TypescriptOptionsWithDocgen; | ||
| } catch { | ||
| // If typescript preset is not available, use defaults | ||
| } | ||
| } | ||
|
|
||
| const { reactDocgen = 'react-docgen', reactDocgenTypescriptOptions } = typescriptOptions; | ||
|
|
||
| // If docgen is disabled, return null | ||
| if (reactDocgen === false) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -nP '\binternal_getArgTypesData\s*\(' -C 3Repository: storybookjs/storybook
Length of output: 477
🏁 Script executed:
cd code/renderers/react/src && grep -n "internal_getArgTypesData" preset.ts | head -20Repository: storybookjs/storybook
Length of output: 115
🏁 Script executed:
fd -e ts -e tsx | xargs rg "internal_getArgTypesData" --no-headingRepository: storybookjs/storybook
Length of output: 621
🏁 Script executed:
sed -n '1,115p' code/core/src/core-server/utils/get-new-story-file.ts | grep -A 10 "internal_getArgTypesData"Repository: storybookjs/storybook
Length of output: 504
🏁 Script executed:
sed -n '1,50p' code/addons/vitest/src/vitest-plugin/index.ts | grep -B 5 -A 10 "internal_getArgTypesData"Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
cat code/addons/vitest/src/vitest-plugin/index.ts | grep -B 10 -A 10 "internal_getArgTypesData"Repository: storybookjs/storybook
Length of output: 615
🏁 Script executed:
cat code/core/src/core-server/utils/get-new-story-file.ts | sed -n '1,150p' | tail -50Repository: storybookjs/storybook
Length of output: 1618
🏁 Script executed:
cat code/renderers/react/src/preset.ts | sed -n '75,120p'Repository: storybookjs/storybook
Length of output: 1383
🏁 Script executed:
rg "InternalGetArgTypesDataOptions" -A 10Repository: storybookjs/storybook
Length of output: 1785
🏁 Script executed:
rg "interface GetArgTypesDataOptions|type GetArgTypesDataOptions" -A 15Repository: storybookjs/storybook
Length of output: 5311
🏁 Script executed:
cat code/core/src/core-server/utils/get-new-story-file.ts | sed -n '1,100p'Repository: storybookjs/storybook
Length of output: 4074
🏁 Script executed:
rg "type Options = " -A 30 code/lib/types/srcRepository: storybookjs/storybook
Length of output: 123
🏁 Script executed:
cat code/core/src/core-server/utils/get-new-story-file.ts | sed -n '55,85p'Repository: storybookjs/storybook
Length of output: 1154
🏁 Script executed:
rg "reactDocgen|reactDocgenTypescriptOptions" code/core/src/core-server/utils/get-new-story-file.tsRepository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
rg "reactDocgen|reactDocgenTypescriptOptions" code/addons/vitest/src/vitest-plugin/index.tsRepository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
cat code/renderers/react/src/preset.ts | sed -n '87,160p'Repository: storybookjs/storybook
Length of output: 1900
🏁 Script executed:
rg "internal_getArgTypesData" --type ts --type tsx -B 2 -A 5 | head -80Repository: storybookjs/storybook
Length of output: 92
🏁 Script executed:
cat code/renderers/react/src/preset.ts | sed -n '60,90p'Repository: storybookjs/storybook
Length of output: 1087
🏁 Script executed:
rg "type Options" code/core/src -A 20 | head -60Repository: storybookjs/storybook
Length of output: 4770
🏁 Script executed:
cat code/addons/vitest/src/vitest-plugin/index.ts | sed -n '1,80p'Repository: storybookjs/storybook
Length of output: 2587
🏁 Script executed:
rg "reactDocgen:" -B 3 -A 3 code/renderers/react/srcRepository: storybookjs/storybook
Length of output: 3197
🏁 Script executed:
rg "reactDocgenTypescriptOptions:" -B 3 -A 3 code/renderers/react/srcRepository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
cat code/renderers/react/src/preset.ts | sed -n '1,85p'Repository: storybookjs/storybook
Length of output: 3028
🏁 Script executed:
cat code/renderers/react/src/componentManifest/reactDocgen/utils.ts | sed -n '1,30p'Repository: storybookjs/storybook
Length of output: 1057
🏁 Script executed:
rg "GetArgTypesDataOptions" -B 2 -A 8Repository: storybookjs/storybook
Length of output: 8654
🏁 Script executed:
rg "internal_getArgTypesData.*reactDocgen|reactDocgenTypescriptOptions" -B 5 -A 5Repository: storybookjs/storybook
Length of output: 23943
🏁 Script executed:
rg "presets.apply\('internal_getArgTypesData" -B 5 -A 10Repository: storybookjs/storybook
Length of output: 2602
Explicit docgen options passed to internal_getArgTypesData are ignored; only preset-provided options are used.
The function accepts reactDocgen and reactDocgenTypescriptOptions via GetArgTypesDataOptions, but never reads them. It only extracts these settings from presets.apply('typescript', {}). When presets aren't available (as in the vitest plugin call site), no fallback to passed options occurs—defaults are silently used instead.
Merge explicit options with preset results, with explicit options taking priority:
🔧 Proposed fix
- const appliedOptions = await presets.apply('typescript', {});
+ const appliedOptions = await presets.apply('typescript', {}, options);
typescriptOptions = appliedOptions as TypescriptOptionsWithDocgen;
@@
- const { reactDocgen = 'react-docgen', reactDocgenTypescriptOptions } = typescriptOptions;
+ const reactDocgen =
+ options?.reactDocgen ?? typescriptOptions.reactDocgen ?? 'react-docgen';
+ const reactDocgenTypescriptOptions =
+ options?.reactDocgenTypescriptOptions ?? typescriptOptions.reactDocgenTypescriptOptions;🤖 Prompt for AI Agents
In `@code/renderers/react/src/preset.ts` around lines 91 - 114, The code ignores
explicit GetArgTypesDataOptions.reactDocgen and .reactDocgenTypescriptOptions
because it only reads them from presets; update the logic in the function that
uses InternalGetArgTypesDataOptions so that you first collect typescriptOptions
from presets.apply('typescript', {}) (as now) but then merge the explicit
options from the incoming options object into that result with the incoming
options taking priority (i.e., override preset values with options.reactDocgen
and options.reactDocgenTypescriptOptions if they are provided), and ensure the
merged values are then used for the reactDocgen / reactDocgenTypescriptOptions
checks (preserve current fallback to defaults and the reactDocgen === false
early return).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts`:
- Around line 145-157: The current matching for selecting targetDocgen uses a
substring check (d.filePath?.includes(componentExportName)) which can yield
false positives; update the docgens.find logic (used to set targetDocgen) to
first check d.displayName === componentExportName and then, when matching by
file path, extract the filename without extension (use path.basename and
path.extname) and compare that equality to componentExportName; add an import
for path if missing and keep the existing fallback behavior (return null when no
match).
♻️ Duplicate comments (1)
code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts (1)
53-61: Array generic parsing may fail for nested types.Similar to the union parsing limitation (already noted in past reviews), the
Array<Type>parsing using simple string slicing will fail for types likeArray<A | B>orArray<Record<string, number>>. The slice at line 59 would incorrectly extractA | Bbut would then fail to parse it correctly due to the union handling.For the current use case with react-docgen-typescript output, this may be acceptable since complex nested types might be represented differently. Consider adding a comment documenting this limitation alongside the union parsing comment.
🧹 Nitpick comments (2)
code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.test.ts (1)
43-44: Consider using type assertion or narrowing instead of@ts-expect-error.Multiple test assertions use
@ts-expect-error fix this laterto accessreactDocgen.data. Since you're already checkingresult?.reactDocgen.type === 'success'implicitly through the assertion, consider adding a type guard or using a type assertion to make this type-safe:const data = result?.reactDocgen as { type: 'success'; data: { displayName: string } }; expect(data.data.displayName).toBe('Button');This is a minor code quality concern and can be addressed later as the comment indicates.
code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts (1)
134-137: Consider adding a comment explaining the jsx compiler option value.The magic number
2for jsx option could be more self-documenting:const parser = withCompilerOptions( - { esModuleInterop: true, jsx: 2 /* React */ }, + { esModuleInterop: true, jsx: 2 }, // jsx: 2 = ts.JsxEmit.React mergedOptions );The inline comment
/* React */is helpful but could be expanded to referencets.JsxEmit.Reactfor developers unfamiliar with the TypeScript compiler API.
| // Find the matching component if a name is specified | ||
| let targetDocgen = docgens[0]; | ||
| if (componentExportName) { | ||
| const match = docgens.find( | ||
| (d) => d.displayName === componentExportName || d.filePath?.includes(componentExportName) | ||
| ); | ||
| if (match) { | ||
| targetDocgen = match; | ||
| } else { | ||
| // If specific component requested but not found, return null | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Weak component matching via filePath.includes() may cause false positives.
The matching logic at line 149 uses d.filePath?.includes(componentExportName), which is a substring match. This could incorrectly match components when the export name is a substring of another file path:
componentExportName = "Button"would matchfilePath = "/app/ButtonGroup.tsx"componentExportName = "Icon"would matchfilePath = "/app/IconButton.tsx"
Consider using a more precise match, such as checking if the filename (without extension) equals the component name:
🔧 Suggested improvement
if (componentExportName) {
const match = docgens.find(
- (d) => d.displayName === componentExportName || d.filePath?.includes(componentExportName)
+ (d) => d.displayName === componentExportName
);Or if filePath matching is needed, use a more precise check:
const match = docgens.find((d) => {
if (d.displayName === componentExportName) return true;
if (d.filePath) {
const fileName = path.basename(d.filePath, path.extname(d.filePath));
return fileName === componentExportName;
}
return false;
});🤖 Prompt for AI Agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts`
around lines 145 - 157, The current matching for selecting targetDocgen uses a
substring check (d.filePath?.includes(componentExportName)) which can yield
false positives; update the docgens.find logic (used to set targetDocgen) to
first check d.displayName === componentExportName and then, when matching by
file path, extract the filename without extension (use path.basename and
path.extname) and compare that equality to componentExportName; add an import
for path if missing and keep the existing fallback behavior (return null when no
match).
There was a problem hiding this comment.
I have not been able to look carefully through every LOC here.
Particularly, the extractReactDocgenInfo.ts & extractReactTypescriptDocgenInfo.ts are too much to digest the implementation of. I'm not sure what we can do to get higher degree of confidence on this right now.
I can see there are unit tests, so I trust that coverage is good, and this is well tested and QA-ed.
But I see the if-else and the getting options via preset.apply and that look looks correct to me.
What I did
This reintroduces support for
react-docgen-typescriptand its options defined inmain.ts, which were supported before and therefore this fixes that issue.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
react-docgenin main.tsDocumentation
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-33586-sha-bb7f266c. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-33586-sha-bb7f266c sandboxor in an existing project withnpx storybook@0.0.0-pr-33586-sha-bb7f266c upgrade.More information
0.0.0-pr-33586-sha-bb7f266cyann/story-creation-docgen-tsbb7f266c1768839549)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=33586Summary by CodeRabbit
Release Notes
Refactor
New Features
✏️ Tip: You can customize this high-level summary in your review settings.