Skip to content

React-Docgen: Add tsconfig fallback chain and warning for monorepos#34353

Merged
valentinpalkovic merged 4 commits into
storybookjs:nextfrom
viditkbhatnagar:fix/tsconfig-fallback-warning-24585
Mar 31, 2026
Merged

React-Docgen: Add tsconfig fallback chain and warning for monorepos#34353
valentinpalkovic merged 4 commits into
storybookjs:nextfrom
viditkbhatnagar:fix/tsconfig-fallback-warning-24585

Conversation

@viditkbhatnagar
Copy link
Copy Markdown
Contributor

@viditkbhatnagar viditkbhatnagar commented Mar 26, 2026

What this PR does

Fixes #24585

In monorepo setups (e.g., Nx, Turborepo), the root directory often has
tsconfig.base.json instead of tsconfig.json. Storybook's tsconfig
resolution silently failed, causing react-docgen-typescript to produce
no controls with zero warnings.

Changes

  • utils.tsfindTsconfigPath() now tries tsconfig.json
    tsconfig.base.jsontsconfig.app.json in order, with a warning
    log when using a fallback
  • reactDocgenTypescript.ts — Warns when no tsconfig variant is
    found, telling the user TypeScript controls won't work
  • react-docgen.ts / react-docgen-loader.ts — Consistent
    fallback chain for path alias resolution in Vite and Webpack
  • utils.test.ts — 5 new tests covering all scenarios

How to test

  1. Create a monorepo with tsconfig.base.json in root (no tsconfig.json)
  2. Set up a Storybook with react-docgen-typescript
  3. Verify controls are now generated correctly
  4. Verify a warning is logged when falling back to tsconfig.base.json
  5. Remove all tsconfig variants and verify a clear warning is shown

Checklist

  • Bug fix
  • Tests added
  • No breaking changes

Summary by CodeRabbit

  • Bug Fixes & Improvements

    • Broadened TypeScript config discovery to try common alternative filenames for better multi-package/monorepo support.
    • Added a runtime warning when no TypeScript config is found (affects prop documentation behavior).
  • Tests

    • Added tests for config discovery priority and warning behavior.

When using react-docgen-typescript in monorepos where only
tsconfig.base.json or tsconfig.app.json exists in the root
directory, Storybook silently failed to generate controls.

- Add fallback chain: tsconfig.json → tsconfig.base.json → tsconfig.app.json
- Log warning when using a fallback tsconfig
- Log clear warning when no tsconfig variant is found at all
- Apply consistent fallback in both Vite and Webpack paths
- Add 5 tests covering all scenarios

Fixes storybookjs#24585

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 26, 2026

📝 Walkthrough

Walkthrough

Search for TypeScript config now tries tsconfig.json, then tsconfig.base.json, then tsconfig.app.json (bounded by projectRoot). Plugins/loaders use this expanded lookup; renderer logs a warning when no tsconfig is found. Tests added for priority and absence cases.

Changes

Cohort / File(s) Summary
Plugins & Loaders
code/frameworks/react-vite/src/plugins/react-docgen.ts, code/presets/react-webpack/src/loaders/react-docgen-loader.ts
Tsconfig discovery now checks ordered candidates tsconfig.json, tsconfig.base.json, tsconfig.app.json (search bounded by project root). Downstream TsconfigPaths usage unchanged.
Renderer: react-docgen integration
code/renderers/react/src/componentManifest/reactDocgenTypescript.ts
Import logger and emit logger.warn when no tsconfig is found before initializing react-docgen-typescript program.
Core utility
code/renderers/react/src/componentManifest/utils.ts
Replaced single-file upward search with ordered candidate search returning first match or undefined; preserves cached wrapper.
Tests
code/renderers/react/src/componentManifest/utils.test.ts
Added tests/mocks for findTsconfigPath: validates priority order (json → base → app), absence case, cache invalidation, and warning expectations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@valentinpalkovic valentinpalkovic changed the title fix(docs-tools): add tsconfig fallback chain and warning for monorepos React-Docgen: Add tsconfig fallback chain and warning for monorepos Mar 30, 2026
@valentinpalkovic valentinpalkovic moved this to Empathy Queue (prioritized) in Core Team Projects Mar 30, 2026
Comment thread code/renderers/react/src/componentManifest/utils.ts Outdated
@valentinpalkovic valentinpalkovic moved this from Empathy Queue (prioritized) to On Hold in Core Team Projects Mar 30, 2026
@valentinpalkovic valentinpalkovic self-assigned this Mar 30, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
code/renderers/react/src/componentManifest/utils.test.ts (1)

207-259: Consider wrapping logger with vi.mocked() for typed mock assertions.

For consistency with mocking patterns in the codebase (e.g., manifests.test.ts), wrap the entire logger object when asserting on its methods: expect(vi.mocked(logger).warn).not.toHaveBeenCalled() at lines 207, 242, and 259.

This provides type safety and aligns with the guideline: "Use vi.mocked() to type and access the mocked functions in Vitest tests."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@code/renderers/react/src/componentManifest/utils.test.ts` around lines 207 -
259, The tests call expect(logger.warn).not.toHaveBeenCalled() in several
places; update those assertions to use Vitest's typed mock helper by wrapping
the logger with vi.mocked, e.g., replace expect(logger.warn)... with
expect(vi.mocked(logger).warn).not.toHaveBeenCalled() in the tests that
reference logger (the ones around the findTsconfigPath tests), so all logger
method assertions use vi.mocked(logger) for proper typed mocking.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@code/renderers/react/src/componentManifest/utils.test.ts`:
- Around line 191-254: The tests currently set
vi.mocked(find.up).mockImplementation inline inside individual tests; move this
mock setup into the existing beforeEach so all mock behavior follows repo Vitest
rules: create a local mapping variable (e.g. findUpMap) in the test file, set a
default mapping in beforeEach and call
vi.mocked(find.up).mockImplementation((name) => findUpMap[name]); then in each
test set/override entries on findUpMap (or assign a new map) before invoking
findTsconfigPath so tests still return the desired filenames (tsconfig.json,
tsconfig.base.json, tsconfig.app.json or undefined) and preserve assertions such
as expect(logger.warn).not.toHaveBeenCalled().

---

Nitpick comments:
In `@code/renderers/react/src/componentManifest/utils.test.ts`:
- Around line 207-259: The tests call expect(logger.warn).not.toHaveBeenCalled()
in several places; update those assertions to use Vitest's typed mock helper by
wrapping the logger with vi.mocked, e.g., replace expect(logger.warn)... with
expect(vi.mocked(logger).warn).not.toHaveBeenCalled() in the tests that
reference logger (the ones around the findTsconfigPath tests), so all logger
method assertions use vi.mocked(logger) for proper typed mocking.
🪄 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: 4650ae5f-c8a1-4dfd-b46d-02abf259b79c

📥 Commits

Reviewing files that changed from the base of the PR and between 3a0725b and ee42d68.

📒 Files selected for processing (1)
  • code/renderers/react/src/componentManifest/utils.test.ts

Comment thread code/renderers/react/src/componentManifest/utils.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: Typescript auto-generated controls behaves weird

2 participants