Fix react-docgen-typescript with Vite-style project references tsconfig#34416
Open
kasperpeulen wants to merge 1 commit into
Open
Fix react-docgen-typescript with Vite-style project references tsconfig#34416kasperpeulen wants to merge 1 commit into
kasperpeulen wants to merge 1 commit into
Conversation
Modern Vite projects use a tsconfig.json with only project references (`files: [], references: [...]`), where the actual compiler options and file include patterns live in tsconfig.app.json. The manifest's RDT parser was finding this root tsconfig but getting zero source files from it, causing component prop extraction to silently fail. When the parsed config yields no file names and has project references, we now follow the references to find the first config that includes source files. Closes #34414
Contributor
|
Contributor
📝 WalkthroughWalkthroughAdds a comprehensive test suite for React Docgen Typescript integration and enhances TypeScript config resolution to support Vite-style project references. Tests validate parsing of component documentation, JSON serializability, and multi-export matching using temporary TypeScript projects. Implementation adds fallback logic to load referenced tsconfig files when the primary config yields no fileNames. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
code/renderers/react/src/componentManifest/reactDocgenTypescript.ts (1)
208-233: Good implementation for handling Vite-style project references.The logic correctly follows TypeScript project references when the root tsconfig yields no file names. The handling of both directory references (
./tsconfig.app) and direct JSON file references (./tsconfig.app.json) aligns with TypeScript's behavior.One consideration: if all referenced configs also yield zero file names, the original empty
parsedis used silently. This is likely fine since it degrades gracefully to the existing behavior, but adding a debug log could help troubleshoot complex setups.💡 Optional: Add debug logging when following references
// Vite-style project references: tsconfig.json has { files: [], references: [...] } // which yields 0 file names. Follow the references to find a config with source files. if (parsed.fileNames.length === 0 && Array.isArray(config.references)) { + logger.debug('Root tsconfig has no files, following project references...'); for (const ref of config.references as { path: string }[]) { const refPath = resolve(dirname(configPath), ref.path); // A reference path can point to a directory (meaning <dir>/tsconfig.json) // or directly to a config file. const refConfigPath = refPath.endsWith('.json') ? refPath : resolve(refPath, 'tsconfig.json'); const refResult = typescript.readConfigFile(refConfigPath, typescript.sys.readFile); if (refResult.error || !refResult.config) { continue; } const refParsed = typescript.parseJsonConfigFileContent( refResult.config, typescript.sys, dirname(refConfigPath) ); if (refParsed.fileNames.length > 0) { + logger.debug(`Using referenced tsconfig: ${refConfigPath}`); parsed = refParsed; break; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/renderers/react/src/componentManifest/reactDocgenTypescript.ts` around lines 208 - 233, Add a debug log when following TypeScript project references and when no referenced config yields fileNames so investigators can trace why parsed.fileNames remained empty: inside the block that checks parsed.fileNames === 0 && Array.isArray(config.references) (and when iterating refs using refPath/refConfigPath), log the ref being attempted and the result of typescript.readConfigFile and typescript.parseJsonConfigFileContent (use the same logger used elsewhere or a temporary debug logger) and, after the loop, log a warning if parsed.fileNames is still 0 to indicate fallback to the original parsed; refer to symbols parsed, config.references, configPath, refConfigPath, typescript.readConfigFile and typescript.parseJsonConfigFileContent to locate where to add these logs.code/renderers/react/src/componentManifest/reactDocgenTypescript.test.ts (1)
16-24: Consider cleaning up temp directories after tests.The
createTempProjecthelper creates temp directories but they're never cleaned up. While the OS will eventually clean them, explicit cleanup prevents accumulation during long test runs or CI.♻️ Proposed cleanup enhancement
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; -import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs'; +const tempDirs: string[] = []; + // Create a temporary directory with a tsconfig and component files // to exercise RDT against real files on disk (memfs can't be used with RDT). 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'); } return dir; } afterEach(() => { invalidateParser(); invalidateCache(); vi.restoreAllMocks(); + // Clean up temp directories + for (const dir of tempDirs) { + rmSync(dir, { recursive: true, force: true }); + } + tempDirs.length = 0; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/renderers/react/src/componentManifest/reactDocgenTypescript.test.ts` around lines 16 - 24, The helper createTempProject currently creates temp directories but never removes them; modify it to return both the temp directory path and a cleanup function (or push created dirs into a shared array) so tests can explicitly delete the directory after running; implement cleanup by removing the directory recursively (e.g., using fs.rmSync or rimraf) and call that cleanup from the test teardown (afterEach/afterAll) to avoid accumulating temp files. Ensure references to createTempProject in tests are updated to call the cleanup function or rely on the shared-tracking cleanup in teardown.
🤖 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/renderers/react/src/componentManifest/reactDocgenTypescript.test.ts`:
- Around line 16-24: The helper createTempProject currently creates temp
directories but never removes them; modify it to return both the temp directory
path and a cleanup function (or push created dirs into a shared array) so tests
can explicitly delete the directory after running; implement cleanup by removing
the directory recursively (e.g., using fs.rmSync or rimraf) and call that
cleanup from the test teardown (afterEach/afterAll) to avoid accumulating temp
files. Ensure references to createTempProject in tests are updated to call the
cleanup function or rely on the shared-tracking cleanup in teardown.
In `@code/renderers/react/src/componentManifest/reactDocgenTypescript.ts`:
- Around line 208-233: Add a debug log when following TypeScript project
references and when no referenced config yields fileNames so investigators can
trace why parsed.fileNames remained empty: inside the block that checks
parsed.fileNames === 0 && Array.isArray(config.references) (and when iterating
refs using refPath/refConfigPath), log the ref being attempted and the result of
typescript.readConfigFile and typescript.parseJsonConfigFileContent (use the
same logger used elsewhere or a temporary debug logger) and, after the loop, log
a warning if parsed.fileNames is still 0 to indicate fallback to the original
parsed; refer to symbols parsed, config.references, configPath, refConfigPath,
typescript.readConfigFile and typescript.parseJsonConfigFileContent to locate
where to add these logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5073267-5ce0-489e-98f3-01b73500eefe
📒 Files selected for processing (2)
code/renderers/react/src/componentManifest/reactDocgenTypescript.test.tscode/renderers/react/src/componentManifest/reactDocgenTypescript.ts
| dirname(refConfigPath) | ||
| ); | ||
| if (refParsed.fileNames.length > 0) { | ||
| parsed = refParsed; |
Contributor
There was a problem hiding this comment.
This would only include the first reference isn't it ?
What if the first referenced tsconfig doesn't include app components ?
like:
{
"files": [],
"references": [
{ "path": "./tsconfig.node.json" },
{ "path": "./tsconfig.app.json" }
]
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #34414
tsconfig.jsonwith only project references (files: [], references: [...]), where the actual compiler options and file include patterns live intsconfig.app.jsonTest plan
parseWithReactDocgenTypescriptcovering basic props, union types, default values, JSON serialization round-trip, multi-component matching, prop filteringtsconfig.jsonwithfiles: []andreferencespointing totsconfig.app.json)Summary by CodeRabbit
Tests
Improvements
referencesinstead of direct file lists.