React-Vite: Add reactDocgenOptions for built-in react-docgen plugin#34454
Open
andrewsrigom wants to merge 3 commits intostorybookjs:nextfrom
Open
React-Vite: Add reactDocgenOptions for built-in react-docgen plugin#34454andrewsrigom wants to merge 3 commits intostorybookjs:nextfrom
andrewsrigom wants to merge 3 commits intostorybookjs:nextfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughPasses user-provided Changes
Sequence DiagramsequenceDiagram
participant User as User Config
participant Preset as viteFinal Preset
participant Plugin as react-docgen Plugin
participant Vite as Vite Config
User->>Preset: provide `reactDocgen` / `reactDocgenOptions`
Preset->>Preset: presets.apply('typescript') -> extract reactDocgen, reactDocgenOptions
Preset->>Preset: merge default `include` with reactDocgenOptions (skip if include provided)
Preset->>Plugin: initialize `react-docgen` plugin with merged options
Plugin->>Vite: insert plugin into `config.plugins`
Vite->>Vite: use plugin options during build
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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. 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/frameworks/react-vite/src/preset.ts (1)
14-18: Consider improving type safety for the preset options.Using
<any>here loses type information for the destructured properties. While this works, it could mask issues if the preset shape changes.♻️ Optional: Define an inline type for better type safety
- const { - reactDocgen: reactDocgenOption, - reactDocgenOptions, - reactDocgenTypescriptOptions, - } = await presets.apply<any>('typescript', {}); + const { + reactDocgen: reactDocgenOption, + reactDocgenOptions, + reactDocgenTypescriptOptions, + } = await presets.apply<{ + reactDocgen?: 'react-docgen-typescript' | 'react-docgen' | false; + reactDocgenOptions?: Parameters<typeof import('./plugins/react-docgen.ts').reactDocgen>[0]; + reactDocgenTypescriptOptions?: Parameters<typeof import('@joshwooding/vite-plugin-react-docgen-typescript').default>[0]; + }>('typescript', {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/frameworks/react-vite/src/preset.ts` around lines 14 - 18, The destructuring uses await presets.apply<any>('typescript', {}) which loses type safety; define a small inline type or interface (e.g., TypeScriptPresetOptions with reactDocgen, reactDocgenOptions, reactDocgenTypescriptOptions) and pass it as the generic to presets.apply instead of any so the returned shape is typed and the destructured variables (reactDocgenOption, reactDocgenOptions, reactDocgenTypescriptOptions) have proper types.code/frameworks/react-vite/src/preset.test.ts (1)
7-9: Consider usingspy: trueoption for consistency with project mocking conventions.As per coding guidelines,
vi.mock()should use thespy: trueoption for file mocks. While the current approach works, adding the option maintains consistency with the repository's testing conventions.♻️ Add spy option to vi.mock
-vi.mock('./plugins/react-docgen.ts', () => ({ +vi.mock('./plugins/react-docgen.ts', { spy: true }, () => ({ reactDocgen: mocks.reactDocgen, }));As per coding guidelines: "Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/frameworks/react-vite/src/preset.test.ts` around lines 7 - 9, The test mock for './plugins/react-docgen.ts' in preset.test.ts should follow project conventions by adding the spy: true option to the vi.mock call; update the existing vi.mock('./plugins/react-docgen.ts', () => ({ reactDocgen: mocks.reactDocgen })) invocation to include the spy: true option so the mock is registered as a spy while keeping the same factory that returns reactDocgen from mocks.
🤖 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/preset.test.ts`:
- Around line 7-9: The test mock for './plugins/react-docgen.ts' in
preset.test.ts should follow project conventions by adding the spy: true option
to the vi.mock call; update the existing vi.mock('./plugins/react-docgen.ts', ()
=> ({ reactDocgen: mocks.reactDocgen })) invocation to include the spy: true
option so the mock is registered as a spy while keeping the same factory that
returns reactDocgen from mocks.
In `@code/frameworks/react-vite/src/preset.ts`:
- Around line 14-18: The destructuring uses await
presets.apply<any>('typescript', {}) which loses type safety; define a small
inline type or interface (e.g., TypeScriptPresetOptions with reactDocgen,
reactDocgenOptions, reactDocgenTypescriptOptions) and pass it as the generic to
presets.apply instead of any so the returned shape is typed and the destructured
variables (reactDocgenOption, reactDocgenOptions, reactDocgenTypescriptOptions)
have proper types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6ba580b8-2e37-48b2-8efc-a8fc33760240
📒 Files selected for processing (6)
code/frameworks/react-vite/src/preset.test.tscode/frameworks/react-vite/src/preset.tscode/frameworks/react-vite/src/types.tsdocs/_snippets/main-config-typescript-react-docgen-options.mddocs/api/main-config/main-config-typescript.mdxdocs/configure/integration/typescript.mdx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/frameworks/react-vite/src/preset.test.ts (1)
21-34: Optional: extract duplicatedpresets.applyscaffold into a helper.Both tests repeat the same
presets.applywiring. A tiny factory/helper would reduce noise and make future cases easier to add.♻️ Suggested refactor
+const makeOptions = (reactDocgenOptions: Record<string, unknown>) => + ({ + presets: { + apply: async (name: string) => + name === 'typescript' + ? { reactDocgen: 'react-docgen', reactDocgenOptions } + : undefined, + }, + }) as any; ... - const config = await viteFinal({ plugins: [existingPlugin] }, { - presets: { - apply: async (name: string) => { - if (name === 'typescript') { - return { - reactDocgen: 'react-docgen', - reactDocgenOptions, - }; - } - - return undefined; - }, - }, - } as any); + const config = await viteFinal({ plugins: [existingPlugin] }, makeOptions(reactDocgenOptions)); ... - await viteFinal({}, { - presets: { - apply: async (name: string) => { - if (name === 'typescript') { - return { - reactDocgen: 'react-docgen', - reactDocgenOptions, - }; - } - - return undefined; - }, - }, - } as any); + await viteFinal({}, makeOptions(reactDocgenOptions));Also applies to: 49-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/frameworks/react-vite/src/preset.test.ts` around lines 21 - 34, Tests duplicate the same presets.apply wiring when calling viteFinal; extract that repeated scaffold into a small helper (e.g., createTypescriptPreset or buildPresetApply) and use it in both test cases to reduce noise. Update the two test blocks that call viteFinal so they pass the helper result instead of inline apply: ensure the helper returns an object with apply: async (name: string) => { if (name === 'typescript') return { reactDocgen: 'react-docgen', reactDocgenOptions }; return undefined; } and reference this helper in the viteFinal calls to replace the duplicated code.
🤖 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/preset.test.ts`:
- Around line 21-34: Tests duplicate the same presets.apply wiring when calling
viteFinal; extract that repeated scaffold into a small helper (e.g.,
createTypescriptPreset or buildPresetApply) and use it in both test cases to
reduce noise. Update the two test blocks that call viteFinal so they pass the
helper result instead of inline apply: ensure the helper returns an object with
apply: async (name: string) => { if (name === 'typescript') return {
reactDocgen: 'react-docgen', reactDocgenOptions }; return undefined; } and
reference this helper in the viteFinal calls to replace the duplicated code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77d495cf-f11a-4fc8-919d-aa41b3b69056
📒 Files selected for processing (2)
code/frameworks/react-vite/src/preset.test.tscode/frameworks/react-vite/src/preset.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/frameworks/react-vite/src/preset.ts
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.
Closes #22565
What I did
typescript.reactDocgenOptionsto@storybook/react-vitereact-docgenpluginexcludeand overriddenincludeChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Manual testing not run locally.
Suggested verification steps:
yarn starttypescript.reactDocgento'react-docgen'reactDocgenOptions: { exclude: [/Button\\.tsx$/] }to.storybook/main.tsButtonreactDocgenOptionsand confirm the generated props/argTypes returnDocumentation
MIGRATION.MD
Summary by CodeRabbit
New Features
Documentation
Tests