Core: Fix import statement for react-docgen-typescript
#33589
Merged
Conversation
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
|
View your CI Pipeline Execution ↗ for commit f53c206
☁️ Nx Cloud last updated this comment at |
Contributor
📝 WalkthroughWalkthroughIntroduce TS config discovery for react-docgen-typescript, replace in-file docgen->argType mapping with delegation to Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant getTsConfig as getTsConfig (read tsconfig)
participant RDTS as react-docgen-typescript
participant Extract as extractArgTypes
Caller->>getTsConfig: request TS compilerOptions
getTsConfig-->>Caller: tsConfig (or {})
Caller->>RDTS: dynamic import + withCompilerOptions(tsConfig, options)
RDTS-->>Caller: parser instance
Caller->>RDTS: parser.parse(filePath) -> docgen results
RDTS-->>Caller: docgen results (__docgenInfo)
Caller->>Extract: extractArgTypes({ __docgenInfo })
Extract-->>Caller: argTypes result (or {})
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing touches
Comment |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 49 | 49 | 0 |
| Self size | 20.30 MB | 20.32 MB | 🚨 +21 KB 🚨 |
| Dependency size | 16.52 MB | 16.52 MB | 🎉 -4 B 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 538 | 538 | 0 |
| Self size | 646 KB | 646 KB | 🎉 -130 B 🎉 |
| Dependency size | 59.22 MB | 59.73 MB | 🚨 +503 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 127 | 127 | 0 |
| Self size | 1.12 MB | 1.12 MB | 🚨 +14 B 🚨 |
| Dependency size | 21.82 MB | 22.32 MB | 🚨 +494 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 159 | 159 | 0 |
| Self size | 30 KB | 30 KB | 🚨 +8 B 🚨 |
| Dependency size | 23.00 MB | 23.61 MB | 🚨 +609 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 117 | 117 | 0 |
| Self size | 35 KB | 35 KB | 🎉 -8 B 🎉 |
| Dependency size | 19.62 MB | 20.11 MB | 🚨 +494 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 278 | 278 | 0 |
| Self size | 24 KB | 24 KB | 🚨 +2 B 🚨 |
| Dependency size | 44.13 MB | 44.64 MB | 🚨 +503 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 775 KB | 775 KB | 🚨 +219 B 🚨 |
| Dependency size | 67.38 MB | 67.47 MB | 🚨 +90 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 176 | 0 |
| Self size | 30 KB | 30 KB | 🎉 -4 B 🎉 |
| Dependency size | 65.95 MB | 66.04 MB | 🚨 +91 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 1000 KB | 999 KB | 🎉 -48 B 🎉 |
| Dependency size | 36.82 MB | 36.84 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | node | node |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 57 | 57 | 0 |
| Self size | 732 KB | 1.23 MB | 🚨 +494 KB 🚨 |
| Dependency size | 12.94 MB | 12.94 MB | 🚨 +403 B 🚨 |
| Bundle Size Analyzer | Link | Link |
ndelangen
approved these changes
Jan 20, 2026
yannbf
force-pushed
the
yann/story-creation-docgen-ts-part-2
branch
from
61e987b to
6c23861
Compare
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.test.ts`:
- Line 7: Replace the non-actionable personal TODO comment in
extractReactTypescriptDocgenInfo.test.ts with a clear, actionable note or an
issue reference: remove "TODO: Norbert figure this out thank you" and either add
a short task describing what's missing (e.g., "TODO: implement X behavior for Y
edge case" or "TODO: update test to cover Z scenario") or replace it with a link
to the tracking issue (e.g., "See issue `#1234`") so future contributors know what
to do and where to follow up.
In `@code/renderers/react/src/componentManifest/reactDocgen/utils.ts`:
- Around line 129-134: In getTsConfig, remove the direct console.log({
tsconfigPath }) to avoid leaking paths at runtime; instead either drop the log
entirely or replace it with the project's logger (use the existing logger
instance/function if available) so the function (getTsConfig) no longer uses
console directly and logs via the repo's logging utilities with appropriate
context.
|
|
||
| import { extractProps } from '../../extractProps'; | ||
|
|
||
| // TODO: Norbert figure this out thank you |
Contributor
There was a problem hiding this comment.
Replace the TODO with an actionable note or issue reference.
The current TODO isn’t actionable and includes a personal note. Please remove it or replace it with a clear task and an issue link.
🤖 Prompt for AI Agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.test.ts`
at line 7, Replace the non-actionable personal TODO comment in
extractReactTypescriptDocgenInfo.test.ts with a clear, actionable note or an
issue reference: remove "TODO: Norbert figure this out thank you" and either add
a short task describing what's missing (e.g., "TODO: implement X behavior for Y
edge case" or "TODO: update test to cover Z scenario") or replace it with a link
to the tracking issue (e.g., "See issue `#1234`") so future contributors know what
to do and where to follow up.
Comment on lines
+129
to
+134
| export const getTsConfig = async () => { | ||
| try { | ||
| const ts = await import('typescript'); | ||
| const tsconfigPath = ts.findConfigFile(process.cwd(), ts.sys.fileExists); | ||
| console.log({ tsconfigPath }); | ||
| if (tsconfigPath === undefined) { |
Contributor
There was a problem hiding this comment.
Remove direct console logging in runtime utility.
console.log here will leak paths and violates the repo logging guideline. Please remove it or replace with the project logger (if needed). As per coding guidelines, avoid direct console usage.
🔧 Suggested change
- console.log({ tsconfigPath });📝 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.
Suggested change
| export const getTsConfig = async () => { | |
| try { | |
| const ts = await import('typescript'); | |
| const tsconfigPath = ts.findConfigFile(process.cwd(), ts.sys.fileExists); | |
| console.log({ tsconfigPath }); | |
| if (tsconfigPath === undefined) { | |
| export const getTsConfig = async () => { | |
| try { | |
| const ts = await import('typescript'); | |
| const tsconfigPath = ts.findConfigFile(process.cwd(), ts.sys.fileExists); | |
| if (tsconfigPath === undefined) { |
🤖 Prompt for AI Agents
In `@code/renderers/react/src/componentManifest/reactDocgen/utils.ts` around lines
129 - 134, In getTsConfig, remove the direct console.log({ tsconfigPath }) to
avoid leaking paths at runtime; instead either drop the log entirely or replace
it with the project's logger (use the existing logger instance/function if
available) so the function (getTsConfig) no longer uses console directly and
logs via the repo's logging utilities with appropriate context.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/renderers/react/src/componentManifest/reactDocgen/extractReactDocgenInfo.ts (1)
67-78: Pass raw docgen data toextractArgTypes, not the wrapper object.Line 77 passes
docgen.reactDocgen(a{ type: 'success', data }wrapper) toextractArgTypes, but it expects raw docgen data. This is inconsistent withextractReactTypescriptDocgenInfo.ts:71, which correctly passes the raw data directly. The wrapper preventsextractComponentPropsfrom accessing thepropsfield, resulting in empty ArgTypes.- return extractArgTypes({ __docgenInfo: docgen.reactDocgen }) ?? {}; + return extractArgTypes({ __docgenInfo: docgen.reactDocgen.data }) ?? {};
🤖 Fix all issues with AI agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts`:
- Around line 43-51: The getTsConfig() helper currently emits console.log output
which causes noisy console output when extractReactTypescriptDocgenInfo (which
calls getTsConfig) runs; update the logging inside getTsConfig (in the
reactDocgen utils) to use the project logger (e.g., processLogger or the
existing logger used across renderers) or remove the console.log entirely so no
console output is produced when extractReactTypescriptDocgenInfo calls
getTsConfig; ensure the replacement preserves the original message/context and
uses the same logging level convention as other renderer utilities.
Comment on lines
43
to
51
| const tsConfig = await getTsConfig(); | ||
|
|
||
| const mergedOptions = { | ||
| ...defaultOptions, | ||
| ...reactDocgenTypescriptOptions, | ||
| // Always ensure savePropValueAsString is true for consistency | ||
| savePropValueAsString: true, | ||
| }; | ||
|
|
||
| const parser = withCompilerOptions( | ||
| { esModuleInterop: true, jsx: 2 /* React */ }, | ||
| mergedOptions | ||
| ); | ||
|
|
||
| const parser = withCompilerOptions(tsConfig, mergedOptions); |
Contributor
There was a problem hiding this comment.
Avoid console output from getTsConfig path.
getTsConfig() currently logs via console.log (code/renderers/react/src/componentManifest/reactDocgen/utils.ts:128-146). Calling it here will now emit console output during docgen runs. Please switch that log to the appropriate logger or remove it.
🔧 Suggested fix in utils.ts
+import { logger } from 'storybook/internal/node-logger';
...
- console.log({ tsconfigPath });
+ logger.debug({ tsconfigPath });🤖 Prompt for AI Agents
In
`@code/renderers/react/src/componentManifest/reactDocgen/extractReactTypescriptDocgenInfo.ts`
around lines 43 - 51, The getTsConfig() helper currently emits console.log
output which causes noisy console output when extractReactTypescriptDocgenInfo
(which calls getTsConfig) runs; update the logging inside getTsConfig (in the
reactDocgen utils) to use the project logger (e.g., processLogger or the
existing logger used across renderers) or remove the console.log entirely so no
console output is produced when extractReactTypescriptDocgenInfo calls
getTsConfig; ensure the replacement preserves the original message/context and
uses the same logging level convention as other renderer utilities.
Member
|
releasing a canary now |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
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.tsMake 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-33589-sha-f53c2069. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-33589-sha-f53c2069 sandboxor in an existing project withnpx storybook@0.0.0-pr-33589-sha-f53c2069 upgrade.More information
0.0.0-pr-33589-sha-f53c2069yann/story-creation-docgen-ts-part-2f53c20691768916946)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=33589Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.