React-Docgen: Try .tsx fallback when resolving .js ESM imports in docgen resolvers

[mixelburg]

What

When resolving ESM-style .js imports that point to TypeScript files, the docgen resolver tried .ts as a fallback but never tried .tsx. This means React components using .tsx extension with ESM-style imports fail MCP manifest generation with "No component file found".

Fixes #34387

Changes

Updated the .js/.mjs/.cjs fallback logic in all three copies of the docgen resolver pattern to try .ts first, then fall back to .tsx:

• code/frameworks/react-vite/src/plugins/docgen-resolver.ts
• code/presets/react-webpack/src/loaders/docgen-resolver.ts
• code/renderers/react/src/componentManifest/reactDocgen/docgenResolver.ts

Before:

case '.js': newFilename = `${filename.slice(0, -2)}ts`; // Chip.js → Chip.ts (fails if Chip.tsx)

After:

case '.js': {
  try { return resolve.sync(`${base}ts`, ...); } // try Chip.ts first
  catch { newFilename = `${base}tsx`; }          // fall back to Chip.tsx
  break;
}

Summary by CodeRabbit

• Bug Fixes
  • Improved module resolution fallback mechanisms across multiple framework packages to provide more robust handling of CommonJS imports in edge cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request updates .cjs module resolution logic across three docgen resolver files. Instead of unconditionally rewriting .cjs to .ts, the resolvers now attempt .ts resolution first via resolve.sync, then fall back to .tsx if that fails, improving fallback handling for CommonJS module imports.

Changes

Cohort / File(s)	Summary
Docgen Resolver Updates code/frameworks/react-vite/src/plugins/docgen-resolver.ts, code/presets/react-webpack/src/loaders/docgen-resolver.ts, code/renderers/react/src/componentManifest/reactDocgen/docgenResolver.ts	Updated .cjs module resolution to attempt .ts file resolution first via resolve.sync with extensions: [], then fall back to .tsx if unsuccessful, replacing prior unconditional rewriting behavior. Changed control flow to use scoped blocks with try/catch for sequential resolution attempts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• storybookjs/storybook#32905 — Related changes to React docgen module-resolution logic and .cjs → .ts/.tsx fallback handling across multiple resolver implementations.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

code/renderers/react/src/componentManifest/reactDocgen/docgenResolver.ts (1)

41-50: Consider centralizing this fallback logic or at least adding a sync note here.

The same .ts-then-TSX selection now lives in this renderer resolver plus the Vite and webpack copies. Since this fix already had to touch all three places, a shared helper—or a local “keep in sync” note in this file—would reduce the odds of the next resolver tweak drifting.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@code/renderers/react/src/componentManifest/reactDocgen/docgenResolver.ts`
around lines 41 - 50, The .cjs branch currently duplicates the ".ts then .tsx"
fallback (see the case '.cjs' block using base, resolve.sync and newFilename)
which is repeated across other resolvers; extract that selection logic into a
small shared helper (e.g., resolveTsThenTsx(filename, resolveOptions)) and call
it from docgenResolver.ts (replace the try/catch and newFilename assignment with
that helper), or if extracting a helper is out-of-scope, add a clear sync
note/TODO above the case '.cjs' block documenting the exact behavior and where
the Vite and webpack copies live so future edits stay consistent. Ensure the
helper returns the resolved filename or throws the same error shape as the
original resolve.sync usage and preserve the existing variable names
(base/newFilename) so surrounding code requires minimal changes.

code/frameworks/react-vite/src/plugins/docgen-resolver.ts (1)

58-67: Please add regression coverage for this fallback branch.

This fixes a reported resolver edge case, but the diff does not show a behavior test for ./Chip.js -> ./Chip.tsx and the corresponding .mjs / .cjs cases. A small resolver-level test would make this much harder to regress. Happy to sketch that fixture if useful.

Based on learnings: "Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Test real behavior, not just syntax patterns".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@code/frameworks/react-vite/src/plugins/docgen-resolver.ts` around lines 58 -
67, Add a regression test that exercises the fallback branch in
docgen-resolver.ts where a `.cjs` (and similarly `.mjs`) import for `./Chip.js`
should resolve to `./Chip.tsx`; create a resolver-level test (matching the
project test patterns e.g. *.test.(ts|tsx|js|jsx)) that sets up fixtures: a
module importing `./Chip.js` and filesystem entries `Chip.ts` (absent) and
`Chip.tsx` (present), then assert the resolver used by the code in
docgen-resolver.ts returns the `Chip.tsx` path; include both `.cjs` and `.mjs`
cases to cover the try/catch fallback logic in the switch block handling
`.cjs`/`.mjs` so the `${base}tsx` branch is exercised and cannot regress.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@code/frameworks/react-vite/src/plugins/docgen-resolver.ts`:
- Around line 58-67: Add a regression test that exercises the fallback branch in
docgen-resolver.ts where a `.cjs` (and similarly `.mjs`) import for `./Chip.js`
should resolve to `./Chip.tsx`; create a resolver-level test (matching the
project test patterns e.g. *.test.(ts|tsx|js|jsx)) that sets up fixtures: a
module importing `./Chip.js` and filesystem entries `Chip.ts` (absent) and
`Chip.tsx` (present), then assert the resolver used by the code in
docgen-resolver.ts returns the `Chip.tsx` path; include both `.cjs` and `.mjs`
cases to cover the try/catch fallback logic in the switch block handling
`.cjs`/`.mjs` so the `${base}tsx` branch is exercised and cannot regress.

In `@code/renderers/react/src/componentManifest/reactDocgen/docgenResolver.ts`:
- Around line 41-50: The .cjs branch currently duplicates the ".ts then .tsx"
fallback (see the case '.cjs' block using base, resolve.sync and newFilename)
which is repeated across other resolvers; extract that selection logic into a
small shared helper (e.g., resolveTsThenTsx(filename, resolveOptions)) and call
it from docgenResolver.ts (replace the try/catch and newFilename assignment with
that helper), or if extracting a helper is out-of-scope, add a clear sync
note/TODO above the case '.cjs' block documenting the exact behavior and where
the Vite and webpack copies live so future edits stay consistent. Ensure the
helper returns the resolved filename or throws the same error shape as the
original resolve.sync usage and preserve the existing variable names
(base/newFilename) so surrounding code requires minimal changes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 83951f85-cb03-4634-86c4-8a11618888c3

📥 Commits

Reviewing files that changed from the base of the PR and between 0efef13 and 835d653.

📒 Files selected for processing (3)

• code/frameworks/react-vite/src/plugins/docgen-resolver.ts
• code/presets/react-webpack/src/loaders/docgen-resolver.ts
• code/renderers/react/src/componentManifest/reactDocgen/docgenResolver.ts

[valentinpalkovic]

LGTM!