Repository navigation
Conversation
Follow referenced tsconfigs when the root config yields no source files, so react-docgen-typescript can extract props in modern Vite layouts. Co-authored-by: Cursor <cursoragent@cursor.com>
ndelangen
added
bug
ci:normal
ndelangen
added
the
patch:yes
📝 WalkthroughWalkthroughReact docgen TypeScript parsing now follows Vite-style referenced tsconfigs when the root config has no files. A new test suite validates prop extraction, defaults, serialization, component matching, DOM prop filtering, and referenced project parsing. ChangesReact docgen TypeScript support
Estimated code review effort: 3 (Moderate) | ~20 minutes ✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
code/renderers/react/src/componentManifest/reactDocgenTypescript.ts (1)
185-223: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReference-order assumption is untested; harden and cover it.
parseTsconfigForDocgenstops at the first referenced config with non-emptyfileNames, which only works because the default Vite template happens to list the app config before the node config; this is unverified elsewhere.
code/renderers/react/src/componentManifest/reactDocgenTypescript.ts#L185-L223: prefer the reference with the mostfileNames(or merge all non-empty references) instead of breaking on the first match, so ordering/extra references don't hide the real app config.code/renderers/react/src/componentManifest/reactDocgenTypescript.test.ts#L226-L274: add a case with references reversed (or an extra reference resolving to non-emptyfileNameslisted first) to lock in correct behavior regardless of order.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/renderers/react/src/componentManifest/reactDocgenTypescript.ts` around lines 185 - 223, The reference traversal in parseTsconfigForDocgen must not depend on declaration order: evaluate all resolvable referenced configs with non-empty fileNames and retain the one with the greatest file count (or merge them), rather than stopping at the first match. Add a test in reactDocgenTypescript.test.ts covering reversed or extra non-empty references and asserting the correct source-bearing config is selected.code/renderers/react/src/componentManifest/reactDocgenTypescript.test.ts (1)
20-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the
process.cwdspy insidecreateTempProject.Every test repeats
vi.spyOn(process, 'cwd').mockReturnValue(dir)inline right after callingcreateTempProject. Folding this into the helper removes the duplication and keeps mock setup out of individual test bodies, per the Vitest mocking guidelines for this repo.♻️ Proposed refactor
function createTempProject(files: Record<string, string>) { const dir = mkdtempSync(path.join(tmpdir(), 'sb-rdt-test-')); tempDirs.push(dir); for (const [name, content] of Object.entries(files)) { const filePath = path.join(dir, name); mkdirSync(path.dirname(filePath), { recursive: true }); writeFileSync(filePath, content, 'utf-8'); } + vi.spyOn(process, 'cwd').mockReturnValue(dir); return dir; }As per coding guidelines,
**/*.{test,spec}.{ts,tsx}should "Implement mock behaviors in beforeEach blocks" and "Avoid inline mock implementations within test cases in Vitest tests."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/renderers/react/src/componentManifest/reactDocgenTypescript.test.ts` around lines 20 - 29, Update createTempProject to install the process.cwd spy with the newly created directory before returning it, then remove the repeated inline vi.spyOn(process, 'cwd').mockReturnValue(dir) calls from individual tests. Keep the existing temporary-directory creation and file-writing behavior unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/renderers/react/src/componentManifest/reactDocgenTypescript.test.ts`:
- Around line 226-230: Remove the “see `#34414`” issue-number reference from the
comment in the test handling Vite-style project references, while preserving the
maintenance-relevant explanation about following referenced tsconfig files to
avoid an empty program.
---
Nitpick comments:
In `@code/renderers/react/src/componentManifest/reactDocgenTypescript.test.ts`:
- Around line 20-29: Update createTempProject to install the process.cwd spy
with the newly created directory before returning it, then remove the repeated
inline vi.spyOn(process, 'cwd').mockReturnValue(dir) calls from individual
tests. Keep the existing temporary-directory creation and file-writing behavior
unchanged.
In `@code/renderers/react/src/componentManifest/reactDocgenTypescript.ts`:
- Around line 185-223: The reference traversal in parseTsconfigForDocgen must
not depend on declaration order: evaluate all resolvable referenced configs with
non-empty fileNames and retain the one with the greatest file count (or merge
them), rather than stopping at the first match. Add a test in
reactDocgenTypescript.test.ts covering reversed or extra non-empty references
and asserting the correct source-bearing config is selected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7e732cb-03d7-4e81-b889-8f6bebc477ac
📒 Files selected for processing (2)
code/renderers/react/src/componentManifest/reactDocgenTypescript.test.tscode/renderers/react/src/componentManifest/reactDocgenTypescript.ts
| test('handles Vite-style project references tsconfig (files: [], references: [...])', async () => { | ||
| // Modern Vite projects use a root tsconfig that only lists project references; the real | ||
| // include/compilerOptions live in tsconfig.app.json. Without following those references, RDT | ||
| // builds an empty program and returns no component docs (see #34414). | ||
| const dir = createTempProject({ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Drop the issue-number reference from the comment.
(see #34414) is a ticket-code provenance reference embedded in a code comment.
✏️ Proposed fix
- // Modern Vite projects use a root tsconfig that only lists project references; the real
- // include/compilerOptions live in tsconfig.app.json. Without following those references, RDT
- // builds an empty program and returns no component docs (see `#34414`).
+ // Modern Vite projects use a root tsconfig that only lists project references; the real
+ // include/compilerOptions live in tsconfig.app.json. Without following those references, RDT
+ // builds an empty program and returns no component docs.As per coding guidelines, comments should explain maintenance-relevant rationale, "not investigation transcripts, ticket or acceptance-criteria codes, provenance claims, or cross-file line references."
📝 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.
| test('handles Vite-style project references tsconfig (files: [], references: [...])', async () => { | |
| // Modern Vite projects use a root tsconfig that only lists project references; the real | |
| // include/compilerOptions live in tsconfig.app.json. Without following those references, RDT | |
| // builds an empty program and returns no component docs (see #34414). | |
| const dir = createTempProject({ | |
| test('handles Vite-style project references tsconfig (files: [], references: [...])', async () => { | |
| // Modern Vite projects use a root tsconfig that only lists project references; the real | |
| // include/compilerOptions live in tsconfig.app.json. Without following those references, RDT | |
| // builds an empty program and returns no component docs. | |
| const dir = createTempProject({ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@code/renderers/react/src/componentManifest/reactDocgenTypescript.test.ts`
around lines 226 - 230, Remove the “see `#34414`” issue-number reference from the
comment in the test handling Vite-style project references, while preserving the
maintenance-relevant explanation about following referenced tsconfig files to
avoid an empty program.
Source: Coding guidelines
Scope note (vs #34415)Hey @kasperpeulen — flagging an intentional difference from your PR, since you’ll probably notice this one is much smaller. Your #34415 goes further: shared file-aware tsconfig selection in This PR only fixes the reported failure in the component-manifest Why we kept it narrow:
Happy to revisit the broader shared-helper approach from #34415 if you think we’re leaving a real gap — especially if you’ve seen cases the narrower fix doesn’t cover. Wanted the tradeoff called out here rather than buried. |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 186 | 186 | 0 |
| Self size | 79 KB | 79 KB | 🚨 +1 B 🚨 |
| Dependency size | 35.73 MB | 36.13 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 21.39 MB | 21.30 MB | 🎉 -81 KB 🎉 |
| Dependency size | 36.75 MB | 36.75 MB | 🚨 +197 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 160 KB | 160 KB | 0 B |
| Dependency size | 30.13 MB | 30.52 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 13 KB | 13 KB | 0 B |
| Dependency size | 31.08 MB | 31.47 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 531 | 531 | 0 |
| Self size | 641 KB | 640 KB | 🎉 -1 KB 🎉 |
| Dependency size | 62.41 MB | 62.81 MB | 🚨 +405 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 94 | 93 | 🎉 -1 🎉 |
| Self size | 1.42 MB | 1.37 MB | 🎉 -48 KB 🎉 |
| Dependency size | 23.91 MB | 23.85 MB | 🎉 -61 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 125 | 122 | 🎉 -3 🎉 |
| Self size | 29 KB | 29 KB | 0 B |
| Dependency size | 25.90 MB | 24.91 MB | 🎉 -981 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 83 | 83 | 0 |
| Self size | 32 KB | 32 KB | 🚨 +8 B 🚨 |
| Dependency size | 21.22 MB | 21.62 MB | 🚨 +399 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 272 | 272 | 0 |
| Self size | 23 KB | 23 KB | 0 B |
| Dependency size | 48.19 MB | 48.59 MB | 🚨 +405 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 15 KB | 15 KB | 🎉 -12 B 🎉 |
| Dependency size | 37.00 MB | 37.39 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 20 | 0 |
| Self size | 54 KB | 54 KB | 🎉 -10 B 🎉 |
| Dependency size | 26.65 MB | 27.04 MB | 🚨 +386 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 21 | 21 | 0 |
| Self size | 56 KB | 56 KB | 0 B |
| Dependency size | 26.71 MB | 27.09 MB | 🚨 +386 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/tanstack-react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 84 | 84 | 0 |
| Self size | 113 KB | 113 KB | 🚨 +12 B 🚨 |
| Dependency size | 21.25 MB | 21.65 MB | 🚨 +399 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 103 | 102 | 🎉 -1 🎉 |
| Self size | 34 KB | 34 KB | 🎉 -252 B 🎉 |
| Dependency size | 43.11 MB | 43.04 MB | 🎉 -73 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 205 | 205 | 0 |
| Self size | 827 KB | 827 KB | 🚨 +52 B 🚨 |
| Dependency size | 91.60 MB | 91.52 MB | 🎉 -81 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 32 KB | 32 KB | 🚨 +36 B 🚨 |
| Dependency size | 90.08 MB | 90.00 MB | 🎉 -81 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 74 | 74 | 0 |
| Self size | 1.09 MB | 1.09 MB | 🎉 -13 B 🎉 |
| Dependency size | 58.14 MB | 58.06 MB | 🎉 -81 KB 🎉 |
| Bundle Size Analyzer | node | node |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 161 | 161 | 0 |
| Self size | 19 KB | 19 KB | 🎉 -23 B 🎉 |
| Dependency size | 32.70 MB | 33.09 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 59 | 59 | 0 |
| Self size | 1.48 MB | 1.49 MB | 🚨 +13 KB 🚨 |
| Dependency size | 12.30 MB | 12.30 MB | 🚨 +137 B 🚨 |
| Bundle Size Analyzer | Link | Link |
Closes #34414
What I did
Modern Vite React TypeScript apps often have a root
tsconfig.jsonthat does not list source files. It only points at other configs:{ "files": [], "references": [ { "path": "./tsconfig.app.json" }, { "path": "./tsconfig.node.json" } ] }The real compiler options and
includelive intsconfig.app.json.The bug: Storybook’s component-manifest path for
react-docgen-typescriptonly read the root config. That produced an empty TypeScript program, so prop docs came back empty and the manifest debugger showed an error.The fix: When the root config has no source files but has project references, follow those references and use the first config that actually includes sources (usually
tsconfig.app.json).This is a small, Storybook-side change in the React component-manifest RDT parser. It supersedes the stale PRs #34415 and #34416.
Implementation notes
parseTsconfigForDocgeninsidecode/renderers/react/src/componentManifest/reactDocgenTypescript.ts.common/tsconfig helper or change the Vite/webpackreact-docgenpath-alias plugins.reactDocgen: "react-docgen-typescript"go through@joshwooding/vite-plugin-react-docgen-typescript@0.7.0, which already follows project references, so that path was left alone.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Unit test command and result
cd code/renderers/react yarn vitest run --config vitest.config.ts src/componentManifest/reactDocgenTypescript.test.tsOne of those tests recreates the Vite
tsconfig.json+tsconfig.app.jsonlayout from #34414.Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Status: already verified on canary
0.0.0-pr-35586-sha-3cb2db1c.We published that canary, then reproduced the issue outside this repo:
files: []+referenceslayout).0.0.0-pr-35586-sha-3cb2db1c.typescript.reactDocgento'react-docgen-typescript'.What we saw:
example-buttonin/manifests/components.jsonreactDocgenTypescriptprops:primary,backgroundColor,size,label,onClickIf you want to repeat the check:
Create a fresh Vite React-TS app and install the canary:
Or from this repo:
In
.storybook/main.ts, set:Confirm the Vite-style tsconfig layout (
tsconfig.jsonwithfiles: []and references totsconfig.app.json/tsconfig.node.json).Start Storybook and open
http://localhost:6006/manifests/components.html(or/manifests/components.json).Expected: Example/Button (and similar components) show prop types from
react-docgen-typescript, with no “did not return any component docs” error.Open a docs entry (e.g. Example/Button) and confirm Controls / argTypes still look reasonable.
Also worth a quick look: projects with a single flat
tsconfig.json(no references) should behave the same as before.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.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake 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-35586-sha-3cb2db1c. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-35586-sha-3cb2db1c sandboxor in an existing project withnpx storybook@0.0.0-pr-35586-sha-3cb2db1c upgrade.More information
0.0.0-pr-35586-sha-3cb2db1cnorbert/fix-rdt-vite-project-references3cb2db1c1784903818)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=35586Summary by CodeRabbit
Bug Fixes
Tests