Repository navigation
Core: Extract ComponentMetaManager to Core - #35657
Merged
Merged
Conversation
huang-julien
added
feature request
react
core
ci:normal
Contributor
📝 WalkthroughWalkthroughAdds a shared ChangesComponent Metadata Management
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ReactComponentMetaManager
participant BaseComponentMetaManager
participant ComponentMetaProject
Caller->>ReactComponentMetaManager: request component metadata project
ReactComponentMetaManager->>BaseComponentMetaManager: resolve project for file
BaseComponentMetaManager->>ComponentMetaProject: create or retrieve configured project
ComponentMetaProject-->>BaseComponentMetaManager: resolved project
BaseComponentMetaManager-->>ReactComponentMetaManager: return project
ReactComponentMetaManager-->>Caller: extract component props
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
code/core/src/component-meta/types.ts (1)
11-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the broken
@linktarget.
ComponentMetaManager.tsis a sibling in this directory, so../ComponentMetaManagerresolves outsidecomponent-meta/and won't link.📝 Proposed fix
- * Contract a per-tsconfig project must satisfy for {`@link` ../ComponentMetaManager} to manage it. + * Contract a per-tsconfig project must satisfy for `ComponentMetaManager` to manage it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/component-meta/types.ts` around lines 11 - 18, Fix the JSDoc link in the project contract comment to reference the sibling ComponentMetaManager module within the component-meta directory, using the correct relative target and leaving the surrounding documentation unchanged.code/core/src/component-meta/ComponentMetaManager.ts (1)
292-313: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNarrow the
tryto parsing/creation.The watcher registration at Lines 303-306 is inside the same
try. IfwatchProgramSourceDirsthrows (it does syncsys.fileExistswalks), the catch returnsnulleven though the project is already inconfigProjects— the caller then silently falls back to the inferred project while the configured one stays cached and undisposed.♻️ Proposed fix
let project = this.configProjects.get(tsconfig); if (!project) { try { const getCommandLine = () => this.factory.parseCommandLine(tsconfig); project = this.factory.createConfiguredProject(getCommandLine(), tsconfig, getCommandLine); - this.configProjects.set(tsconfig, project); - - // Auto-watch the project's directories if watching is active. - // This covers projects discovered after initial startWatching(). - if (this.watching) { - this.watchDirectory(path.dirname(tsconfig)); - this.watchProgramSourceDirs(project); - } } catch (err) { logger.debug(`[component-meta] Failed to parse tsconfig ${tsconfig}: ${err}`); return null; } + this.configProjects.set(tsconfig, project); + + // Auto-watch the project's directories if watching is active. + // This covers projects discovered after initial startWatching(). + if (this.watching) { + this.watchDirectory(path.dirname(tsconfig)); + this.watchProgramSourceDirs(project); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/component-meta/ComponentMetaManager.ts` around lines 292 - 313, Restrict the try/catch in getOrCreateConfiguredProject to parseCommandLine and createConfiguredProject, storing the successfully created project before leaving the guarded section. Move the watching block, including watchDirectory and watchProgramSourceDirs, outside the catch so watcher errors are not converted into a null result after the project has been cached.code/renderers/react/src/componentManifest/componentMeta/ComponentMetaManager.ts (1)
24-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the compatibility re-export and import the helpers directly in the test.
code/renderers/react/src/componentManifest/componentMeta/ComponentMetaManager.test.tsis the only non-helper consumer, so let it import fromstorybook/internal/component-metainstead of keeping a re-export shim.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/renderers/react/src/componentManifest/componentMeta/ComponentMetaManager.ts` around lines 24 - 25, Remove the isFileInDir and sortTSConfigs compatibility re-export from ComponentMetaManager.ts, then update ComponentMetaManager.test.ts to import those helpers directly from storybook/internal/component-meta. Preserve all existing helper usage and behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/component-meta/ComponentMetaManager.ts`:
- Around line 320-366: Update onConfigChanged so a deleted configPath is always
removed from rootTsConfigs, regardless of whether configProjects contains an
instantiated project; retain project disposal and removal for instantiated
projects, and continue clearing searchedDirs after the config change.
---
Nitpick comments:
In `@code/core/src/component-meta/ComponentMetaManager.ts`:
- Around line 292-313: Restrict the try/catch in getOrCreateConfiguredProject to
parseCommandLine and createConfiguredProject, storing the successfully created
project before leaving the guarded section. Move the watching block, including
watchDirectory and watchProgramSourceDirs, outside the catch so watcher errors
are not converted into a null result after the project has been cached.
In `@code/core/src/component-meta/types.ts`:
- Around line 11-18: Fix the JSDoc link in the project contract comment to
reference the sibling ComponentMetaManager module within the component-meta
directory, using the correct relative target and leaving the surrounding
documentation unchanged.
In
`@code/renderers/react/src/componentManifest/componentMeta/ComponentMetaManager.ts`:
- Around line 24-25: Remove the isFileInDir and sortTSConfigs compatibility
re-export from ComponentMetaManager.ts, then update ComponentMetaManager.test.ts
to import those helpers directly from storybook/internal/component-meta.
Preserve all existing helper usage and behavior.
🪄 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 Plus
Run ID: bac8cfc7-18ff-40e8-9728-a8aa13bd76e1
📒 Files selected for processing (6)
code/core/build-config.tscode/core/package.jsoncode/core/src/component-meta/ComponentMetaManager.tscode/core/src/component-meta/index.tscode/core/src/component-meta/types.tscode/renderers/react/src/componentManifest/componentMeta/ComponentMetaManager.ts
huang-julien
commented
Jul 29, 2026
Contributor
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 21.40 MB | 21.42 MB | 🚨 +25 KB 🚨 |
| Dependency size | 31.22 MB | 31.22 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 531 | 531 | 0 |
| Self size | 641 KB | 641 KB | 0 B |
| Dependency size | 62.42 MB | 62.40 MB | 🎉 -15 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 94 | 94 | 0 |
| Self size | 1.42 MB | 1.42 MB | 🚨 +48 B 🚨 |
| Dependency size | 23.91 MB | 23.90 MB | 🎉 -15 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 125 | 125 | 0 |
| Self size | 29 KB | 29 KB | 🚨 +36 B 🚨 |
| Dependency size | 25.90 MB | 25.89 MB | 🎉 -15 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 83 | 83 | 0 |
| Self size | 32 KB | 32 KB | 0 B |
| Dependency size | 21.23 MB | 21.21 MB | 🎉 -15 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 272 | 272 | 0 |
| Self size | 23 KB | 23 KB | 🚨 +12 B 🚨 |
| Dependency size | 48.20 MB | 48.18 MB | 🎉 -15 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/tanstack-react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 84 | 84 | 0 |
| Self size | 114 KB | 114 KB | 0 B |
| Dependency size | 21.26 MB | 21.24 MB | 🎉 -15 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 205 | 205 | 0 |
| Self size | 829 KB | 829 KB | 🚨 +84 B 🚨 |
| Dependency size | 86.08 MB | 86.11 MB | 🚨 +25 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 32 KB | 32 KB | 0 B |
| Dependency size | 84.56 MB | 84.58 MB | 🚨 +25 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 74 | 74 | 0 |
| Self size | 1.09 MB | 1.09 MB | 🚨 +66 B 🚨 |
| Dependency size | 52.61 MB | 52.64 MB | 🚨 +25 KB 🚨 |
| Bundle Size Analyzer | node | node |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 59 | 59 | 0 |
| Self size | 1.48 MB | 1.46 MB | 🎉 -15 KB 🎉 |
| Dependency size | 12.30 MB | 12.30 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
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 (2)
code/core/src/component-meta/ComponentMetaManager.ts (2)
549-565: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInvalidate cached projects for atomic tsconfig saves.
A
renameevent for an atomic save is classified ascreatedwhen the file exists afterward. This code only reclassifies tracked source files, not tracked tsconfigs, soonConfigChanged(filePath, 'created')merely adds the path and leaves the existing cached project using stale compiler options and file membership.Reclassify tracked config paths as
changed, or make thecreatedbranch dispose an existing configured project.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/component-meta/ComponentMetaManager.ts` around lines 549 - 565, Update the reclassification logic around onConfigChanged so atomic tsconfig saves are treated as changed when the path belongs to an existing configured project, rather than remaining created. Reuse the configured-project tracking symbols to detect tracked config paths before invoking onConfigChanged, ensuring cached projects are invalidated and refreshed with the new compiler options and file membership.
477-524: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake watcher replacement atomic and handle watcher lifetime errors.
Narrower watchers are closed before
watch()is successful, so a failed creation leaves those subdirectories unwatched. Also attach an asynchronouserrorlistener to the created watcher and clean it up on failure; the surrounding synchronoustry/catchcannot cover events emitted after creation, and an erroredfs.watchwatcher is unusable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/component-meta/ComponentMetaManager.ts` around lines 477 - 524, Update the watcher setup around the recursive watch creation so replacement is atomic: create and validate the new watcher before closing/removing narrower watchers, preserving them if creation fails. Attach an asynchronous error listener to the new watcher that removes it from watchersByDir, closes it safely, and performs the required cleanup when fs.watch reports failure; do not rely on the surrounding synchronous try/catch for emitted errors.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@code/renderers/react/src/componentManifest/componentMeta/ComponentMetaManager.ts`:
- Around line 42-43: Remove provenance references, external URLs, cross-file
line references, and historical wording such as “without the fix” from comments
in ComponentMetaManager. Rewrite only comments near the affected logic to state
the current maintenance rationale or behavior they clarify; remove comments that
provide no ongoing value.
---
Outside diff comments:
In `@code/core/src/component-meta/ComponentMetaManager.ts`:
- Around line 549-565: Update the reclassification logic around onConfigChanged
so atomic tsconfig saves are treated as changed when the path belongs to an
existing configured project, rather than remaining created. Reuse the
configured-project tracking symbols to detect tracked config paths before
invoking onConfigChanged, ensuring cached projects are invalidated and refreshed
with the new compiler options and file membership.
- Around line 477-524: Update the watcher setup around the recursive watch
creation so replacement is atomic: create and validate the new watcher before
closing/removing narrower watchers, preserving them if creation fails. Attach an
asynchronous error listener to the new watcher that removes it from
watchersByDir, closes it safely, and performs the required cleanup when fs.watch
reports failure; do not rely on the surrounding synchronous try/catch for
emitted errors.
🪄 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 Plus
Run ID: 9287d6b2-e04c-4868-ae27-4d7a1264b35f
📒 Files selected for processing (4)
code/core/src/component-meta/ComponentMetaManager.tscode/core/src/component-meta/index.tscode/core/src/component-meta/types.tscode/renderers/react/src/componentManifest/componentMeta/ComponentMetaManager.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/component-meta/index.ts
huang-julien
force-pushed
the
julien/extract-component-meta-manager
branch
from
July 30, 2026 09:37
3960882 to
8c44c6a
Compare
9 tasks
huang-julien
force-pushed
the
julien/extract-component-meta-manager
branch
from
July 30, 2026 13:26
8c44c6a to
106df70
Compare
valentinpalkovic
left a comment
valentinpalkovic
left a comment
Contributor
There was a problem hiding this comment.
Second review pass. For transparency: I paired this with an AI code-quality review (the Cursor "thermo-nuclear code quality review" skill), then filtered it down to only the findings that are genuinely new on top of my earlier comments. A couple are deliberately low-confidence questions rather than blockers - flagged inline. 🙂
huang-julien
force-pushed
the
julien/extract-component-meta-manager
branch
2 times, most recently
from
July 31, 2026 09:37
9f82a37 to
52937cd
Compare
valentinpalkovic
approved these changes
Jul 31, 2026
huang-julien
force-pushed
the
julien/extract-component-meta-manager
branch
2 times, most recently
from
July 31, 2026 15:53
b0fdb21 to
9be383b
Compare
The extracted manager put typescript-coupled types on core's public surface: `typeof ts` parameters and `ts.ParsedCommandLine` in the project contract. Consumers resolve their own typescript copy (vue3-vite even pins one as a runtime dependency), and across two copies `typeof ts` is not assignable (TS2345) while comparing `ts.ParsedCommandLine` structurally walks the whole compiler-API graph and overflows the checker (TS2321) — exactly the CI failure on this branch. Core's component-meta module now imports nothing from typescript: - `ProjectCommandLine` (fileNames + projectReferences paths) replaces `ts.ParsedCommandLine` in the contract; a `CL` generic on the factory and manager lets renderers keep full command-line fidelity internally. - The constructor takes a structural `ComponentMetaFileSystem` host (`sys.fileExists`/`sys.directoryExists` — all the manager uses); a real `typeof ts` still satisfies it. - `parseTsconfigCommandLine` moves into the React renderer, its only consumer, deleting the last cross-boundary `typeof ts`. Reproduced and verified with a dual-installation probe (typescript 6.0.3 on the contract side, 5.9.3 on the consumer side): the previous contract fails with the CI error verbatim; this contract compiles clean. React's public manager surface is unchanged; all 279 renderer tests pass as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
huang-julien
force-pushed
the
julien/extract-component-meta-manager
branch
from
August 3, 2026 09:44
9be383b to
4017976
Compare
2 tasks done
This was referenced Aug 7, 2026
JReinhold
added
qa:skip
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
3 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 #
What I did
This PR extracts
ComponentMetaManagerfrom React renderer to Core for reusability across different renderer intergrzeation.parseCommandLineandcreateInferredProject(fallback for file that no config covers)For React:
For Vue:
ComponentMetaProjectwould wrap avueComponentMeta.createChecker()per tsconfigChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Check that nothing broke with react-component-meta
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.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake 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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit