React: Auto calculate imports, support barrel files, auto detect. components and other small tweaks #32912
Conversation
|
View your CI Pipeline Execution ↗ for commit d9cc661
☁️ Nx Cloud last updated this comment at |
kasperpeulen
force-pushed
the
kasper/manifest-auto-import
branch
from
cdd661e to
2b072ba
Compare
📝 WalkthroughWalkthroughAdds AST-driven import extraction and merged import strings to the React component manifest, enriches per-story metadata (summary/description), integrates cached react-docgen with tsconfig path mapping, updates manifest UI to render an Imports block and panel CSS, and adds memoization utilities plus extensive tests and fixtures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Presets as Presets.apply
participant Generator as componentManifestGenerator
participant FindUp as cachedFindUp
participant FS as cachedReadFileSync
participant CSF as CSF parser
participant Imports as getComponentImports
participant Docgen as getReactDocgen
Note over Generator,FindUp: resolve nearest package.json & tsconfig (cached)
Presets->>Generator: invoke experimental_componentManifestGenerator
Generator->>FindUp: find nearest package.json / tsconfig
FindUp-->>Generator: path(s)
Generator->>FS: read package.json -> packageName
Generator->>CSF: read & parse story file (CSF AST)
CSF-->>Generator: csf AST
Generator->>Imports: getComponentImports(csf, packageName, storyFilePath)
Imports-->>Generator: { components[], imports[] }
Generator->>Docgen: getReactDocgen(componentPath, componentRef)
Docgen-->>Generator: { success | error }
Generator->>Generator: assemble manifest entry (imports, reactDocgen, jsDocTags, stories...)
Generator-->>Presets: return component manifest map
sequenceDiagram
autonumber
participant UI as Manifest UI
participant Card as renderComponentCard
UI->>Card: render component card with manifest
Card->>Card: check manifest.import and per-story metadata
alt manifest.import present
Card->>UI: render "Imports" pre/code block under component/story section
end
alt story has summary/description
Card->>UI: render Summary and Description blocks under the story
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/core-server/manifest.ts (1)
610-614: Update warning message to reflect automatic import derivation.The warning message references the
@importJSDoc tag, but the new implementation automatically derives imports from the package name. This guidance may confuse users who expect automatic import handling.Consider updating the message to:
- warns.push( - `Specify an @import jsdoc tag on your component or your stories meta such as @import import { ${c.name} } from 'my-design-system';` - ); + warns.push( + `Import could not be automatically determined. Ensure your component is imported from a package, or manually specify an @import JSDoc tag: @import import { ${c.name} } from 'my-design-system';` + );
🧹 Nitpick comments (2)
code/renderers/react/src/componentManifest/getComponentImports.ts (2)
59-86: Add comment explaining the Bucket data structure.The
Buckettype and its purpose in organizing imports per source could benefit from a comment to improve maintainability.+ // Bucket: groups import specifiers by source, tracking defaults, namespaces, named imports, and encounter order type Bucket = { source: t.StringLiteral; defaults: t.Identifier[]; namespaces: t.Identifier[]; named: t.ImportSpecifier[]; order: number; };
120-136: Document the default import conversion logic.The logic for converting default imports to named imports when rewriting relative paths is non-obvious and would benefit from a brief comment explaining the rationale.
if (t.isImportDefaultSpecifier(s)) { if (isRelToPkg) { - // Convert default to named when rewriting relative -> package + // Convert default to named when rewriting relative -> package + // This ensures consistent import style: import { Button } from 'package' instead of import Button from 'package' const id = s.local;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
code/core/src/core-server/manifest.ts(3 hunks)code/renderers/react/package.json(1 hunks)code/renderers/react/src/componentManifest/generator.test.ts(11 hunks)code/renderers/react/src/componentManifest/generator.ts(4 hunks)code/renderers/react/src/componentManifest/getComponentImports.test.ts(1 hunks)code/renderers/react/src/componentManifest/getComponentImports.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.tscode/renderers/react/package.jsoncode/renderers/react/src/componentManifest/generator.tscode/core/src/core-server/manifest.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/core-server/manifest.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/core-server/manifest.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/core-server/manifest.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Mock external dependencies in tests using `vi.mock()` (e.g., filesystem, loggers)
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to access and implement mock behaviors
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to type and access mocked functions
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with vi.mocked()
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without vi.mocked()
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/.storybook/** : Place internal UI Storybook configuration in `code/.storybook/` and maintain it there
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/README.md : Update package or feature README files when making significant changes
Applied to files:
code/core/src/core-server/manifest.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.ts
🧬 Code graph analysis (4)
code/renderers/react/src/componentManifest/generator.test.ts (2)
code/renderers/react/src/componentManifest/generator.ts (1)
componentManifestGenerator(22-170)code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
StoryIndexGenerator(104-874)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(281-1042)
code/renderers/react/src/componentManifest/generator.ts (2)
code/core/src/types/modules/core-common.ts (1)
PresetPropertyFn(650-655)code/renderers/react/src/componentManifest/getComponentImports.ts (1)
getComponentImports(11-238)
code/renderers/react/src/componentManifest/getComponentImports.test.ts (2)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
getComponentImports(11-238)code/core/src/csf-tools/CsfFile.ts (1)
loadCsf(1060-1064)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Danger JS
- GitHub Check: normal
🔇 Additional comments (14)
code/renderers/react/package.json (1)
70-70: LGTM!The
empathicdevDependency is appropriately added to support the new package.json path resolution functionality used in tests.code/core/src/core-server/manifest.ts (2)
529-535: LGTM!The vertical spacing logic correctly applies margin only when panels are visible, improving the layout consistency.
548-565: LGTM!The code block styling improvements provide better wrapping behavior for long lines and consistent typography. The
inline-size: min(100%, 120ch)constraint is a good choice for readability.code/renderers/react/src/componentManifest/generator.test.ts (2)
10-23: LGTM!The mock for
storybook/internal/commonproperly usesimportOriginalto preserve existing exports while adding the JsPackageManagerFactory mock for testing package name resolution.
227-230: LGTM!The generator invocation correctly passes the
configDiroption and uses optional chaining to handle potentially undefined return values, aligning with the updated generator signature.code/renderers/react/src/componentManifest/generator.ts (2)
22-24: LGTM!The function signature correctly uses
PresetPropertyFnto provide proper typing and parameter structure, aligning with Storybook's preset system conventions.
79-79: LGTM!The
importfield correctly uses the calculated imports, integrating the automatic import derivation into the manifest structure.code/renderers/react/src/componentManifest/getComponentImports.test.ts (4)
9-13: LGTM!The helper function
getImportsprovides a clean abstraction for test setup, making the test cases more readable and maintainable.
15-44: LGTM!The test correctly verifies component extraction from JSX and import reconstruction, covering both
meta.componentand JSX element usage.
337-357: LGTM!The test properly validates the conversion of relative default imports to named imports when a
packageNameis provided, which is a key feature of the automatic import handling.
427-459: LGTM!Excellent test coverage for import merging behavior, verifying that multiple imports from the same package are correctly consolidated while preserving default/named import distinctions.
code/renderers/react/src/componentManifest/getComponentImports.ts (3)
4-9: LGTM!The helper functions provide clean abstractions for JSX name handling and base identifier extraction, improving code readability.
11-44: LGTM!The component collection logic correctly identifies React components by:
- Scanning JSX opening elements for uppercase-starting identifiers
- Handling JSX member expressions (e.g.,
UI.Button)- Including
meta.componentwhen present
173-235: LGTM!The import declaration merging logic correctly handles the complex rules for combining default, namespace, and named imports:
- Namespaces cannot be combined with named imports
- Defaults can be combined with named imports
- Multiple defaults/namespaces require separate statements
The implementation preserves encounter order and properly deduplicates imports.
| .card > .tg-warn:checked ~ .panels .panel-warn { | ||
| display: grid; | ||
| } | ||
| .card > .tg-stories:checked ~ .panels .panel-stories { | ||
| display: grid; | ||
| } |
There was a problem hiding this comment.
Remove or complete empty CSS rule blocks.
These CSS rule blocks appear to be incomplete or placeholder code. They should either be removed or completed with the intended styles.
-
- .card > .tg-warn:checked ~ .panels .panel-warn {
- display: grid;
- }
-
- .card > .tg-stories:checked ~ .panels .panel-stories {
- display: grid;
- }📝 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.
| .card > .tg-warn:checked ~ .panels .panel-warn { | |
| display: grid; | |
| } | |
| .card > .tg-stories:checked ~ .panels .panel-stories { | |
| display: grid; | |
| } |
🤖 Prompt for AI Agents
In code/core/src/core-server/manifest.ts around lines 520 to 527, there are two
CSS rule blocks that look like incomplete placeholders (.card > .tg-warn:checked
~ .panels .panel-warn and .card > .tg-stories:checked ~ .panels .panel-stories);
either remove these rules entirely if they are not needed, or complete them with
the intended panel styles (for example set grid-template-columns/rows, gap,
align-items, padding, and any show/hide behavior) so they’re not empty
placeholders — choose one of those two options and update the file accordingly.
| ${ | ||
| c.import | ||
| ? `<div class="note ok"> | ||
| <div class="row"> | ||
| <span class="ex-name">Imports</span> | ||
| </div> | ||
| <pre><code>${c.import}</code></pre> | ||
| </div>` | ||
| : '' | ||
| } |
There was a problem hiding this comment.
Escape import content to prevent XSS.
The import string c.import is rendered without escaping, which could lead to XSS vulnerabilities if the import content contains malicious HTML/script tags.
Apply the esc() function:
${
c.import
? `<div class="note ok">
<div class="row">
<span class="ex-name">Imports</span>
</div>
- <pre><code>${c.import}</code></pre>
+ <pre><code>${esc(c.import)}</code></pre>
</div>`
: ''
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In code/core/src/core-server/manifest.ts around lines 782 to 793, the template
renders c.import unescaped which can lead to XSS; replace the raw interpolation
with an escaped value (e.g., use esc(c.import) or esc(String(c.import))) when
inserting into the <pre><code> block, and ensure the esc function is
imported/available in this module; keep the conditional logic the same but wrap
the displayed content in esc(...) to prevent HTML/script injection.
| vi.mock('empathic/find', async () => ({ | ||
| up: (path: string) => '/app/package.json', | ||
| })); |
There was a problem hiding this comment.
Mock empathic/find without spy: true option.
According to the coding guidelines, all package and file mocks in Vitest tests should use vi.mock() with the spy: true option.
As per coding guidelines
Apply this diff:
-vi.mock('empathic/find', async () => ({
+vi.mock('empathic/find', { spy: true }, async () => ({
up: (path: string) => '/app/package.json',
}));📝 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.
| vi.mock('empathic/find', async () => ({ | |
| up: (path: string) => '/app/package.json', | |
| })); | |
| vi.mock('empathic/find', async () => ({ | |
| up: (path: string) => '/app/package.json', | |
| }), { spy: true }); |
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/generator.test.ts around lines
27-29, the Vitest mock for 'empathic/find' is missing the required spy option;
update the vi.mock call to pass the options object with { spy: true } as the
third argument while keeping the async factory intact (i.e.,
vi.mock('empathic/find', async () => ({ up: (path: string) =>
'/app/package.json' }), { spy: true })).
| const nearestPkg = find.up('package.json', { | ||
| cwd: path.dirname(storyAbsPath), | ||
| last: process.cwd(), | ||
| }); | ||
| const packageName = nearestPkg | ||
| ? JSON.parse(await readFile(nearestPkg, 'utf-8')).name | ||
| : undefined; |
There was a problem hiding this comment.
Add error handling for package.json parsing.
The JSON.parse call on Line 68 could throw if the package.json file is malformed. Consider wrapping it in a try-catch block to prevent uncaught exceptions.
const nearestPkg = find.up('package.json', {
cwd: path.dirname(storyAbsPath),
last: process.cwd(),
});
- const packageName = nearestPkg
- ? JSON.parse(await readFile(nearestPkg, 'utf-8')).name
- : undefined;
+ let packageName: string | undefined;
+ if (nearestPkg) {
+ try {
+ packageName = JSON.parse(await readFile(nearestPkg, 'utf-8')).name;
+ } catch (e) {
+ // Skip if package.json is malformed
+ packageName = undefined;
+ }
+ }🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/generator.ts around lines 63–69,
the direct JSON.parse of the nearest package.json can throw on malformed JSON;
wrap the parse in a try/catch so a bad package.json does not crash the process —
read the file as before, then attempt JSON.parse inside a try block, on success
set packageName to the parsed.name, on error catch and set packageName =
undefined (and optionally log a warning with the file path and error) so
execution continues safely.
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/renderers/react/src/componentManifest/getComponentImports.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
🧬 Code graph analysis (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(281-1042)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (7)
code/renderers/react/src/componentManifest/getComponentImports.ts (7)
4-18: LGTM! Clean and focused helper functions.The helper functions are well-structured and handle their specific tasks correctly. The use of optional chaining in
baseIdentifierand the genericaddUniqueBypattern are good practices.
27-60: LGTM! Correct component detection logic.The traversal correctly identifies React components by uppercase naming convention and properly handles both simple identifiers and member expressions (e.g.,
UI.Button). The inclusion ofmeta.componentensures comprehensive component detection.
117-159: LGTM! Correct filtering and namespace handling.The logic correctly distinguishes between locally-defined and imported components, preventing local components from appearing in the import list. The namespace member extraction properly handles dotted notation like
UI.Button.
207-240: LGTM! Intelligent import rewriting and namespace expansion.The code correctly handles the conversion of relative imports to package imports:
- Default imports are converted to named imports (lines 207-214)
- Namespace imports are expanded to only the members actually used (lines 222-240)
- Both conversions only occur for relative imports being rewritten to package imports
This selective expansion preserves the original import style for non-relative imports while enabling cleaner package imports.
254-325: LGTM! Correct ES module import generation.The import declaration generation correctly handles all valid ES module import combinations:
- Properly combines default with namespace in a single statement
- Separates named imports from namespace imports (per ES module spec)
- Correctly handles multiple defaults/namespaces with separate statements
- Respects encounter order for deterministic output
327-347: LGTM! Clean component metadata generation.The component object building correctly handles both simple identifiers and member expressions, properly extracting import metadata for each component. The final sort ensures deterministic output.
104-107: I need to examine the test case and the implementation more closely to understand how this conversion is handled and whether errors are accounted for.<!-- [scratchpad_start]
[task review_file_1/1 review_segment_3/9 verify_default_import_assumption]
Gather direct evidence from test and implementation[observations]
- Web search results don't provide specific storybook component manifest information
- Test file at line 409 is named "Converts default relative import to named when packageName provided"
- Need to directly inspect test and implementation to verify behavior
- Original review comment concerns about name assumptions need direct code evidence
[analysis]
- Test exists but haven't seen what it expects
- May need to look directly at test case to understand expected behavior
- The concern is real per JS module semantics, but may be handled or documented
[pending]
- Get test case details
- Check implementation for error handling
- Verify if this is by design or a limitation
[actions]
Query the test file directly to see test expectations and implementation logic
[scratchpad_start] -->Based on my investigation, I need to examine the actual test and implementation code to verify whether the assumption mentioned in the review comment is handled appropriately. The test from line 409 explicitly names this behavior "Converts default relative import to named when packageName provided," which indicates this is intentional. However, I need to confirm whether this is documented, tested, and whether error cases are handled.
Verify the assumptions about named export availability when converting default imports to package imports.
When converting a relative default import to a package import, the generated code assumes the package exports a named export matching the local name. This is indeed intentional (confirmed by the test case naming at line 409), but verify whether:
- This limitation is documented in comments or PR description
- Error handling exists for cases where the named export doesn't match
- Tests cover both success and failure scenarios
If this is a known limitation intended for initial release, document it clearly in code comments. If runtime failures are possible, consider adding validation or fallback logic.
code/renderers/react/src/componentManifest/getComponentImports.ts
Outdated
Show resolved
Hide resolved
code/renderers/react/src/componentManifest/getComponentImports.ts
Outdated
Show resolved
Hide resolved
code/renderers/react/src/componentManifest/getComponentImports.ts
Outdated
Show resolved
Hide resolved
| // Remaining defaults: each in its own statement | ||
|
|
||
| // Remaining defaults: each in its own statement | ||
|
|
||
| // Remaining defaults: each in its own statement |
There was a problem hiding this comment.
Remove duplicate comments.
The comment "// Remaining defaults: each in its own statement" appears three times on consecutive lines (290-294).
Apply this diff:
}
// Remaining defaults: each in its own statement
-
- // Remaining defaults: each in its own statement
-
- // Remaining defaults: each in its own statement
for (const d of defaults.slice(1)) {📝 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.
| // Remaining defaults: each in its own statement | |
| // Remaining defaults: each in its own statement | |
| // Remaining defaults: each in its own statement | |
| } | |
| // Remaining defaults: each in its own statement | |
| for (const d of defaults.slice(1)) { |
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/getComponentImports.ts around
lines 290 to 294, there are three duplicate consecutive comments "// Remaining
defaults: each in its own statement"; remove the redundant duplicates so the
comment appears only once (keep a single instance on the most appropriate line)
to eliminate repetition and clutter.
| // Remaining namespaces: each in its own statement | ||
|
|
||
| // Remaining namespaces: each in its own statement | ||
|
|
||
| // Remaining namespaces: each in its own statement |
There was a problem hiding this comment.
Remove duplicate comments.
The comment "// Remaining namespaces: each in its own statement" appears three times on consecutive lines (299-303).
Apply this diff:
}
// Remaining namespaces: each in its own statement
-
- // Remaining namespaces: each in its own statement
-
- // Remaining namespaces: each in its own statement
for (const ns of namespaces.slice(1)) {🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/getComponentImports.ts around
lines 299 to 303, there are three identical consecutive comments "// Remaining
namespaces: each in its own statement"; remove the duplicate comments leaving a
single instance of that comment (or remove all if redundant) so the code has no
repeated comment lines and retains one clear comment where needed.
code/renderers/react/src/componentManifest/getComponentImports.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/core/src/core-server/build-static.ts(1 hunks)code/core/src/core-server/dev-server.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/core/src/core-server/build-static.tscode/core/src/core-server/dev-server.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/core/src/core-server/build-static.tscode/core/src/core-server/dev-server.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/core/src/core-server/build-static.tscode/core/src/core-server/dev-server.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-server/build-static.tscode/core/src/core-server/dev-server.ts
🧬 Code graph analysis (2)
code/core/src/core-server/build-static.ts (2)
code/renderers/react/src/componentManifest/generator.ts (1)
componentManifestGenerator(25-206)code/renderers/react/src/preset.ts (1)
componentManifestGenerator(11-11)
code/core/src/core-server/dev-server.ts (3)
code/renderers/react/src/componentManifest/generator.ts (1)
componentManifestGenerator(25-206)code/renderers/react/src/preset.ts (1)
componentManifestGenerator(11-11)code/core/src/types/modules/core-common.ts (1)
ComponentManifestGenerator(365-367)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/core/src/core-server/build-static.ts (1)
170-172: LGTM! Type inference from preset system.The removal of the explicit
ComponentManifestGeneratortype annotation allows TypeScript to infer the correct type frompresets.apply(). This aligns with the updated generator signature that now usesPresetPropertyFn<'experimental_componentManifestGenerator'>.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 533 | 533 | 0 |
| Self size | 749 KB | 749 KB | 0 B |
| Dependency size | 58.93 MB | 58.95 MB | 🚨 +18 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 124 | 124 | 0 |
| Self size | 3.83 MB | 3.83 MB | 🚨 +72 B 🚨 |
| Dependency size | 21.76 MB | 21.78 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 157 | 157 | 0 |
| Self size | 31 KB | 31 KB | 0 B |
| Dependency size | 23.11 MB | 23.13 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 114 | 114 | 0 |
| Self size | 37 KB | 37 KB | 🎉 -24 B 🎉 |
| Dependency size | 19.67 MB | 19.69 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 273 | 273 | 0 |
| Self size | 25 KB | 25 KB | 🎉 -36 B 🎉 |
| Dependency size | 43.89 MB | 43.91 MB | 🚨 +18 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 57 | 57 | 0 |
| Self size | 812 KB | 830 KB | 🚨 +17 KB 🚨 |
| Dependency size | 12.94 MB | 12.94 MB | 🚨 +12 B 🚨 |
| Bundle Size Analyzer | Link | Link |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
code/renderers/react/src/componentManifest/generator.ts (2)
58-59: GuardJSON.parseon package.json (duplicate of prior feedback).
JSON.parsewill throw on malformedpackage.json, crashing manifest generation. Please wrap the parse in try/catch (as previously requested) so a bad file doesn’t break the process.
65-66: Use||so the fallback import triggers (duplicate of prior feedback).
.trim()returns''for empty imports, and??treats that as a valid value. Use|| fallbackImportso we actually fall back when no imports were discovered.- const calculatedImports = componentImports.imports.join('\n').trim() ?? fallbackImport; + const calculatedImports = componentImports.imports.join('\n').trim() || fallbackImport;
🧹 Nitpick comments (1)
code/renderers/react/src/componentManifest/reactDocgen.ts (1)
60-75: Caches are never populated.
getReactDocgenCacheandgetTsConfigCacheare cleared and read, but no code ever stores results into them. This leaves cache hits impossible and makesinvalidateCache()meaningless. Please assign the freshly computed values to the respective maps before returning so repeated lookups can benefit from the cache.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/renderers/react/src/componentManifest/generator.ts(5 hunks)code/renderers/react/src/componentManifest/reactDocgen.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen.ts
🧬 Code graph analysis (2)
code/renderers/react/src/componentManifest/generator.ts (6)
code/core/src/types/modules/core-common.ts (2)
ComponentManifest(348-358)PresetPropertyFn(650-655)code/renderers/react/src/componentManifest/reactDocgen.ts (4)
DocObj(22-26)invalidateCache(62-65)matchPath(46-58)getReactDocgen(78-137)code/core/src/csf-tools/CsfFile.ts (2)
loadCsf(1060-1064)stories(968-970)code/renderers/react/src/componentManifest/getComponentImports.ts (1)
getComponentImports(20-348)code/core/src/common/utils/interpret-files.ts (1)
resolveImport(25-58)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(6-227)
code/renderers/react/src/componentManifest/reactDocgen.ts (3)
code/core/src/common/utils/interpret-files.ts (1)
supportedExtensions(6-17)code/core/src/common/utils/paths.ts (1)
getProjectRoot(11-57)code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
| resolved = resolveImport(matchedPath, { basedir: dirname(storyAbsPath) }); | ||
| componentPath = resolved; | ||
| } catch (e) { | ||
| console.error(e); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use Storybook’s logger instead of console.error.
Per repository guidelines for code/** modules, use storybook/internal/client-logger or storybook/internal/node-logger rather than the raw console. Please swap this call to the appropriate logger.
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/generator.ts around lines 83 to
87, replace the console.error usage with Storybook's node logger: import the
logger from "storybook/internal/node-logger" at the top of the file (e.g.,
import { logger } from 'storybook/internal/node-logger') and change
console.error(e) to logger.error with the error (and brief context message).
Ensure the import is added only once and use logger.error(...) in the catch so
the error is reported via Storybook's logging system.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
code/renderers/react/src/componentManifest/generator.ts (3)
53-59: Add error handling for package.json parsing.The
JSON.parsecall could throw if the package.json file is malformed. Consider wrapping it in a try-catch block to prevent uncaught exceptions.Apply this diff to add error handling:
const nearestPkg = find.up('package.json', { cwd: path.dirname(storyAbsPath), last: process.cwd(), }); -const packageName = nearestPkg - ? JSON.parse(await readFile(nearestPkg, 'utf-8')).name - : undefined; +let packageName: string | undefined; +if (nearestPkg) { + try { + packageName = JSON.parse(await readFile(nearestPkg, 'utf-8')).name; + } catch (e) { + // Skip if package.json is malformed + packageName = undefined; + } +}
61-65: Fix logical operator for empty string fallback.The
??(nullish coalescing) operator only checks fornullorundefined, but.trim()on an empty string returns''(which is falsy but not nullish). Use||instead to properly fallback when the trimmed result is an empty string.Apply this diff:
-const calculatedImports = componentImports.imports.join('\n').trim() ?? fallbackImport; +const calculatedImports = componentImports.imports.join('\n').trim() || fallbackImport;
75-88: Use Storybook's logger instead of console.error.Per repository guidelines for
code/**modules, usestorybook/internal/node-loggerrather than the raw console.Based on coding guidelines.
Apply this diff:
+import { logger } from 'storybook/internal/node-logger'; + // ... in the component path resolution block: try { resolved = resolveImport(matchedPath, { basedir: dirname(storyAbsPath) }); componentPath = resolved; } catch (e) { - console.error(e); + logger.error('Failed to resolve component path:', e); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/renderers/react/src/componentManifest/generator.ts(5 hunks)code/renderers/react/src/componentManifest/reactDocgen.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to scripts/**/*.{ts,js,mjs} : In Node.js scripts, use `storybook/internal/node-logger` instead of `console.*`
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to {code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs} : Do not use `console.log`, `console.warn`, or `console.error` directly unless in isolated files where importing loggers would significantly increase bundle size
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen.ts
🧬 Code graph analysis (2)
code/renderers/react/src/componentManifest/generator.ts (4)
code/core/src/types/modules/core-common.ts (2)
ComponentManifest(348-358)PresetPropertyFn(650-655)code/renderers/react/src/componentManifest/reactDocgen.ts (4)
DocObj(23-27)invalidateCache(63-66)matchPath(47-59)getReactDocgen(79-143)code/renderers/react/src/componentManifest/getComponentImports.ts (1)
getComponentImports(20-348)code/core/src/common/utils/interpret-files.ts (1)
resolveImport(25-58)
code/renderers/react/src/componentManifest/reactDocgen.ts (4)
code/core/src/common/utils/interpret-files.ts (1)
supportedExtensions(6-17)code/core/src/common/utils/paths.ts (1)
getProjectRoot(11-57)code/frameworks/react-vite/src/plugins/react-docgen.ts (1)
getReactDocgenImporter(100-130)code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (15)
code/renderers/react/src/componentManifest/reactDocgen.ts (6)
34-45: LGTM - Enhanced matching logic with importName parameter.The updated
getMatchingDocgenfunction correctly prioritizesexportNamematch whenimportNameis provided, falling back todisplayNameoractualName. The logic is sound and handles the single-docgen case efficiently.
47-59: LGTM - Path matching with tsconfig support.The
matchPathfunction correctly uses tsconfig-paths to resolve module paths according to the project's path mappings. The fallback to the originalidwhen tsconfig resolution fails is appropriate.
61-66: LGTM - Cache invalidation for both caches.The
invalidateCachefunction correctly clears both the docgen cache and tsconfig cache.
79-143: LGTM - Robust getReactDocgen with comprehensive error handling.The function correctly:
- Caches results by path and importName
- Handles file read failures with clear error messages
- Distinguishes between missing definitions and parsing errors
- Returns a discriminated union for type-safe error handling
- Provides helpful debugging guidance for missing component definitions
Note: There's a minor race condition where simultaneous calls before caching could execute the expensive parse operation multiple times. This is unlikely to cause issues in practice but could be mitigated with async locking if needed.
145-170: LGTM - Simplified importer using internal matchPath.The refactored
getReactDocgenImportercorrectly uses the internalmatchPathfunction, simplifying the API while maintaining the react-native to react-native-web mapping.
172-176: LGTM - Import tag extraction.The
getImportTagfunction correctly extracts the import tag from JSDoc comments usingextractJSDocInfo.code/renderers/react/src/componentManifest/generator.ts (9)
2-2: LGTM - Imports support enhanced manifest generation.The new and updated imports (
dirname,resolveImport, component types,find,getComponentImports, docgen utilities) are all properly used in the updated manifest generation flow.Also applies to: 5-11, 13-13, 17-17, 19-19
26-28: LGTM - Correct type annotation for preset property.The function is properly typed as
PresetPropertyFn<'experimental_componentManifestGenerator'>, which ensures type safety for the preset configuration.
30-30: LGTM - Cache invalidation at generator start.Calling
invalidateCache()at the beginning ensures fresh docgen and tsconfig data for each manifest generation run.
44-48: LGTM - Story file resolution and component extraction.The code correctly resolves the story file path and extracts the component name from the CSF meta.
67-73: LGTM - Component matching with fallback assignment.The logic correctly finds the component by matching local or import names, with appropriate fallback to title matching when componentName is not specified. The nullish assignment on Line 73 is a clean way to set componentName when it's initially undefined.
107-114: LGTM - Base manifest structure with import field.The base manifest correctly includes all required fields including the new
importfield set tocalculatedImports.
116-134: LGTM - Appropriate error handling for missing component.The code correctly checks for missing
componentPathand returns appropriate error messages distinguishing between missingmeta.componentand unresolvable component imports.
136-155: LGTM - Docgen integration with discriminated union handling.The code correctly:
- Calls
getReactDocgenwithcomponentPathandimportName- Extracts docgen data and errors from the discriminated union
- Populates the manifest with description, summary, import, and JSDoc tags
- Includes the reactDocgen data and any errors in the final manifest
90-105: The review comment is incorrect. No changes needed.The code properly handles cases where
componentNameis undefined through the existinginvariantcheck ingetCodeSnippet. WhencomponentNameis undefined and the function cannot generate a snippet, the invariant throws with a clear error message: "Could not generate snippet without component name." This error is caught by the try-catch block surrounding thegetCodeSnippetcall at line 90–105 in generator.ts, and the error is gracefully captured and returned as part of the stories array rather than crashing the entire process.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
108-108: Remove duplicate comment.The comment "// missing binding -> keep (will become null import)" appears twice on line 108.
Apply this diff:
- } // missing binding -> keep (will become null import) // missing binding -> keep (will become null import) + } // missing binding -> keep (will become null import)
🧹 Nitpick comments (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
23-27: Consider using a Set for deduplication.The
addUniqueByhelper usesarray.find()for uniqueness checks, which can result in O(n²) behavior when called repeatedly in loops. For typical import counts this is unlikely to be problematic, but using a Set with custom keys would be more efficient.Example approach:
// Track seen items with a Set of string keys const addUniqueWithSet = <T>( arr: T[], seen: Set<string>, item: T, keyFn: (a: T) => string ) => { const key = keyFn(item); if (!seen.has(key)) { seen.add(key); arr.push(item); } };Then maintain a
Set<string>alongside each array (defaults, namespaces, named) in the bucket and use the Set for O(1) lookups.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/renderers/react/src/componentManifest/getComponentImports.test.ts(1 hunks)code/renderers/react/src/componentManifest/getComponentImports.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/renderers/react/src/componentManifest/getComponentImports.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to {code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs} : Do not use `console.log`, `console.warn`, or `console.error` directly unless in isolated files where importing loggers would significantly increase bundle size
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
🧬 Code graph analysis (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(281-1042)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (4)
code/renderers/react/src/componentManifest/getComponentImports.ts (4)
1-11: LGTM!The imports and
ComponentReftype definition are well-structured. The type clearly captures all necessary metadata for component import resolution.
29-158: Well-structured component extraction logic.The
getComponentsfunction correctly:
- Identifies React components from JSX (uppercase convention)
- Handles member expressions (e.g.,
Namespace.Component)- Filters type-only imports at both declaration and specifier levels
- Distinguishes imported components from locally-defined ones
- Resolves namespace imports with member access
The logic for building
localToImportand filtering locally-defined components is sound.
160-327: Complex but correct import merging logic.The
getImportsfunction handles the intricate task of merging and rewriting import declarations. Key strengths:
- Correctly respects ES module semantics (namespace imports can combine with default but not named specifiers)
- Handles packageName rewrites by converting relative defaults to named and expanding namespaces to used members
- Preserves original import order through the
orderfield- Includes defensive guards for TypeScript (lines 216-218, 236-238, 256-258)
The logic for bucketing, rewriting, and printing is sound, though admittedly complex due to the various import shapes and rewrite rules.
329-339: LGTM!The
getComponentImportsfunction provides a clean public API by orchestratinggetComponentsandgetImports. The implementation is straightforward and well-typed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
code/renderers/react/src/componentManifest/generator.test.ts (1)
11-35: Addspy: trueto every Vitest mock.Per the repository guidelines, all
vi.mock()calls in tests must opt intospy: true. That’s currently missing forstorybook/internal/common,node:fs/promises,node:fs,empathic/find, andtsconfig-paths. Please update each call accordingly. As per coding guidelinesApply this diff:
-vi.mock('storybook/internal/common', async (importOriginal) => ({ +vi.mock('storybook/internal/common', async (importOriginal) => ({ ...(await importOriginal()), // Keep it simple: hardcode known inputs to expected outputs for this test. resolveImport: (id: string) => { return { './Button': './src/stories/Button.tsx', './Header': './src/stories/Header.tsx', }[id]; }, JsPackageManagerFactory: { getPackageManager: () => ({ primaryPackageJson: { packageJson: { name: 'some-package', }, }, }), }, -})); -vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises); -vi.mock('node:fs', async () => (await import('memfs')).fs); +}), { spy: true }); +vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises, { spy: true }); +vi.mock('node:fs', async () => (await import('memfs')).fs, { spy: true }); -vi.mock('empathic/find', async () => ({ +vi.mock('empathic/find', async () => ({ up: (path: string) => '/app/package.json', -})); -vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) })); +}), { spy: true }); +vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) }), { + spy: true, +});code/renderers/react/src/componentManifest/generator.ts (1)
59-61: Add error handling for package.json parsing.The
JSON.parsecall could throw if package.json is malformed. This was flagged in a previous review but remains unaddressed.- const packageName = nearestPkg - ? JSON.parse(await readFile(nearestPkg, 'utf-8')).name - : undefined; + let packageName: string | undefined; + if (nearestPkg) { + try { + packageName = JSON.parse(await readFile(nearestPkg, 'utf-8')).name; + } catch { + packageName = undefined; + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
code/core/src/core-server/manifest.ts(4 hunks)code/core/src/types/modules/core-common.ts(1 hunks)code/renderers/react/src/componentManifest/generator.test.ts(10 hunks)code/renderers/react/src/componentManifest/generator.ts(5 hunks)code/renderers/react/src/componentManifest/getComponentImports.test.ts(1 hunks)code/renderers/react/src/componentManifest/getComponentImports.ts(1 hunks)code/renderers/react/src/componentManifest/reactDocgen.test.ts(1 hunks)code/renderers/react/src/componentManifest/reactDocgen.ts(3 hunks)code/renderers/react/src/componentManifest/test-utils.ts(1 hunks)code/renderers/react/src/componentManifest/utils.test.ts(1 hunks)code/renderers/react/src/componentManifest/utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- code/core/src/core-server/manifest.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/renderers/react/src/componentManifest/getComponentImports.ts
🧰 Additional context used
📓 Path-based instructions (7)
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/reactDocgen.test.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/reactDocgen.test.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/types/modules/core-common.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/reactDocgen.test.tscode/renderers/react/src/componentManifest/reactDocgen.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/reactDocgen.test.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/types/modules/core-common.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/reactDocgen.test.tscode/renderers/react/src/componentManifest/reactDocgen.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/types/modules/core-common.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/reactDocgen.test.tscode/renderers/react/src/componentManifest/reactDocgen.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/types/modules/core-common.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/reactDocgen.test.tscode/renderers/react/src/componentManifest/reactDocgen.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
🧠 Learnings (28)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/reactDocgen.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of beforeEach blocks
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should be placed in beforeEach blocks
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to scripts/**/*.{ts,js,mjs} : In Node.js scripts, use `storybook/internal/node-logger` instead of `console.*`
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to {code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs} : Do not use `console.log`, `console.warn`, or `console.error` directly unless in isolated files where importing loggers would significantly increase bundle size
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Mock external dependencies in tests using `vi.mock()` (e.g., filesystem, loggers)
Applied to files:
code/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed
Applied to files:
code/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to access and implement mock behaviors
Applied to files:
code/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in beforeEach blocks
Applied to files:
code/renderers/react/src/componentManifest/test-utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the spy: true option
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without vi.mocked()
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/utils.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to type and access mocked functions
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with vi.mocked()
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/.storybook/** : Place internal UI Storybook configuration in `code/.storybook/` and maintain it there
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
🧬 Code graph analysis (7)
code/renderers/react/src/componentManifest/getComponentImports.test.ts (2)
code/renderers/react/src/componentManifest/test-utils.ts (1)
fsMocks(3-112)code/renderers/react/src/componentManifest/getComponentImports.ts (2)
getImports(183-356)getComponentImports(358-387)
code/renderers/react/src/componentManifest/generator.ts (7)
code/core/src/types/modules/core-common.ts (2)
ComponentManifest(348-364)PresetPropertyFn(656-661)code/renderers/react/src/componentManifest/reactDocgen.ts (2)
DocObj(24-28)getReactDocgen(76-131)code/renderers/react/src/componentManifest/utils.ts (1)
invalidateCache(95-99)code/renderers/react/src/componentManifest/getComponentImports.ts (1)
getComponentImports(358-387)code/core/src/csf-tools/enrichCsf.ts (1)
extractDescription(158-178)code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(6-227)
code/renderers/react/src/componentManifest/reactDocgen.test.ts (1)
code/renderers/react/src/componentManifest/reactDocgen.ts (1)
parseWithReactDocgen(67-74)
code/renderers/react/src/componentManifest/reactDocgen.ts (4)
code/core/src/common/utils/interpret-files.ts (1)
supportedExtensions(6-17)code/renderers/react/src/componentManifest/utils.ts (1)
cached(36-93)code/core/src/common/utils/paths.ts (1)
getProjectRoot(11-57)code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)
code/renderers/react/src/componentManifest/test-utils.ts (1)
scripts/utils/tools.ts (1)
dedent(118-118)
code/renderers/react/src/componentManifest/generator.test.ts (2)
code/renderers/react/src/componentManifest/test-utils.ts (1)
fsMocks(3-112)code/renderers/react/src/componentManifest/generator.ts (1)
componentManifestGenerator(24-180)
code/renderers/react/src/componentManifest/utils.test.ts (1)
code/renderers/react/src/componentManifest/utils.ts (4)
groupBy(2-12)invariant(15-23)cached(36-93)invalidateCache(95-99)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (12)
code/renderers/react/src/componentManifest/generator.ts (5)
71-71: Verify the nullish coalescing operator behavior with empty strings.The
??operator only catchesnullorundefined. Whenimportsis empty,.join('\n').trim()returns''(empty string), which is falsy but not nullish. If the intent is to fall back tofallbackImportwhen the trimmed result is empty, use||instead.A previous review marked this as addressed, but the
??operator is still present. Please confirm whether empty string should trigger the fallback.- const calculatedImports = componentImports.imports.join('\n').trim() ?? fallbackImport; + const calculatedImports = componentImports.imports.join('\n').trim() || fallbackImport;
46-48: LGTM: Manifest tag filtering at component level.Correctly filters out components tagged with
!manifestearly in the pipeline.
73-87: LGTM: Comprehensive component resolution logic.The component matching uses both name-based and title-based fallback strategies, with a clear precedence chain for
componentNameresolution. This handles various import patterns robustly.
95-97: LGTM: Story-level manifest tag filtering.Properly excludes individual stories tagged with
!manifestwhile still processing the component.
148-151: LGTM: Proper discriminated union handling for docgen results.The docgen result handling correctly extracts
dataorerrorbased on the discriminatedtypefield, ensuring type safety.code/renderers/react/src/componentManifest/reactDocgen.ts (7)
76-131: LGTM: Comprehensive error handling in getReactDocgen.The function properly handles multiple error scenarios with structured error returns:
- File read failures (lines 84-94)
- Missing component definitions (lines 96-108, 114-115, 126-128)
- General docgen evaluation failures (lines 117-123)
The discriminated union return type ensures type-safe error handling in consumers. The error messages are descriptive and include actionable guidance for users.
35-46: LGTM: Well-structured docgen matching logic.The function correctly handles multiple matching strategies with clear precedence: single docgen, exportName match, displayName/actualName fallback, or first docgen as last resort.
48-60: LGTM: Proper tsconfig path resolution.The
matchPathfunction correctly loads tsconfig via the cached helper and uses tsconfig-paths to resolve module aliases with supported extensions. The nullish coalescing fallback is appropriate here.
67-74: LGTM: Clean docgen parsing wrapper.The
parseWithReactDocgenfunction properly configures react-docgen with custom resolver, handlers, importer, and filename for accurate parsing.
133-158: LGTM: React-native to react-native-web fallback logic.The custom importer properly resolves imports via tsconfig paths, handles the react-native → react-native-web polyfill mapping, validates supported extensions, and throws descriptive errors for unsupported imports.
160-164: LGTM: Extraction of JSDoc import tag.Correctly extracts the
@importtag from component docgen description for manual import overrides.
62-65: Caching implementation is correct and properly memoizes getTsConfig.The
cached()wrapper correctly:
- Derives stable cache keys from the
cwdargument usingJSON.stringify(args)- Populates the cache on first invocation (
syncStore.set(k, result))- Returns cached results on subsequent calls with the same
cwdwithout recomputingMultiple calls to
getTsConfigwith the same working directory will hit the cache and avoid redundant tsconfig reloading.
| vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises); | ||
| vi.mock('node:fs', async () => (await import('memfs')).fs); | ||
| vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) })); | ||
|
|
||
| // Mock resolveImport to deterministically resolve known relative imports for these tests | ||
| vi.mock('storybook/internal/common', async (importOriginal) => ({ | ||
| ...(await importOriginal()), | ||
| resolveImport: (id: string) => { | ||
| return { | ||
| './Button': './src/stories/Button.tsx', | ||
| './Header': './src/stories/Header.tsx', | ||
| }[id]; | ||
| }, | ||
| })); |
There was a problem hiding this comment.
Ensure Vitest mocks use spy: true.
The coding guidelines require { spy: true } on every vi.mock() in tests. Please add the option to the mocks for node:fs/promises, node:fs, tsconfig-paths, and storybook/internal/common. As per coding guidelines
Apply this diff:
-vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises);
-vi.mock('node:fs', async () => (await import('memfs')).fs);
-vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) }));
+vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises, { spy: true });
+vi.mock('node:fs', async () => (await import('memfs')).fs, { spy: true });
+vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) }), {
+ spy: true,
+});
// Mock resolveImport to deterministically resolve known relative imports for these tests
-vi.mock('storybook/internal/common', async (importOriginal) => ({
+vi.mock('storybook/internal/common', async (importOriginal) => ({
...(await importOriginal()),
resolveImport: (id: string) => {
return {
'./Button': './src/stories/Button.tsx',
'./Header': './src/stories/Header.tsx',
}[id];
},
-}));
+}), { spy: true });🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/getComponentImports.test.ts around
lines 11 to 24, the Vitest mocks are missing the required option { spy: true };
update each vi.mock call for 'node:fs/promises', 'node:fs', 'tsconfig-paths',
and 'storybook/internal/common' to include the { spy: true } argument (e.g.,
vi.mock('module', async () => ..., { spy: true })) so all mocks comply with the
coding guideline.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
code/renderers/react/src/componentManifest/generator.test.ts (2)
11-35: Restore requiredspy: trueon every Vitest mock
All module mocks still omit the{ spy: true }option mandated by the repo test guidelines, so the previous feedback hasn’t been applied. Please add the spy flag to eachvi.mockin this block.-vi.mock('storybook/internal/common', async (importOriginal) => ({ +vi.mock('storybook/internal/common', async (importOriginal) => ({ ...(await importOriginal()), resolveImport: (id: string) => { return { './Button': './src/stories/Button.tsx', './Header': './src/stories/Header.tsx', }[id]; }, -})); -vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises); -vi.mock('node:fs', async () => (await import('memfs')).fs); -vi.mock('empathic/find', async () => ({ +}), { spy: true }); +vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises, { spy: true }); +vi.mock('node:fs', async () => (await import('memfs')).fs, { spy: true }); +vi.mock('empathic/find', async () => ({ up: (path: string) => '/app/package.json', -})); -vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) })); +}), { spy: true }); +vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) }), { + spy: true, +});As per coding guidelines.
120-123: Seed memfs under/app
The virtual FS is still written at the default root, so the generator reads/app/...paths that don’t exist and the tests fail immediately. Please seed the volume under/appas previously requested.- vol.fromJSON(fsMocks); + vol.fromJSON(fsMocks, '/app');code/renderers/react/src/componentManifest/generator.ts (2)
73-74: Use||so fallback imports work
trim()returns'', which isn’t nullish, so the fallback never executes and we emit empty import strings. This regresses manifest output (and triggers a warning). Please revert to||.- const calculatedImports = componentImports.imports.join('\n').trim() ?? fallbackImport; + const calculatedImports = componentImports.imports.join('\n').trim() || fallbackImport;
57-63: Guard package.json parsing
JSON.parsecan still throw on malformed package.json, collapsing manifest generation. Wrap it in try/catch so we degrade gracefully and continue without the package name.- const packageName = nearestPkg - ? JSON.parse(cachedReadFileSync(nearestPkg, 'utf-8') as string).name - : undefined; + let packageName: string | undefined; + if (nearestPkg) { + try { + packageName = JSON.parse(cachedReadFileSync(nearestPkg, 'utf-8') as string).name; + } catch (error) { + logger.warn(`Failed to parse ${nearestPkg}: ${(error as Error).message}`); + } + }code/core/src/core-server/manifest.ts (1)
787-794: Escapec.importbefore rendering
We’re interpolatingc.importdirectly into HTML, allowing crafted story metadata to inject arbitrary markup. Please wrap it withesc(...).- <pre><code>${c.import}</code></pre> + <pre><code>${esc(c.import)}</code></pre>code/renderers/react/src/componentManifest/getComponentImports.test.ts (1)
11-24: Add{ spy: true }to these mocks as well
The Vitest mocks in this file still miss the required spy configuration, so we’re not following the repo test rules. Please add{ spy: true }to eachvi.mockcall.-vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises); -vi.mock('node:fs', async () => (await import('memfs')).fs); -vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) })); +vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises, { spy: true }); +vi.mock('node:fs', async () => (await import('memfs')).fs, { spy: true }); +vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) }), { + spy: true, +}); // Mock resolveImport to deterministically resolve known relative imports for these tests -vi.mock('storybook/internal/common', async (importOriginal) => ({ +vi.mock('storybook/internal/common', async (importOriginal) => ({ ...(await importOriginal()), resolveImport: (id: string) => { return { './Button': './src/stories/Button.tsx', './Header': './src/stories/Header.tsx', }[id]; }, -})); +}), { spy: true });As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
code/core/src/core-server/dev-server.ts(3 hunks)code/core/src/core-server/manifest.ts(4 hunks)code/renderers/react/src/componentManifest/generator.test.ts(14 hunks)code/renderers/react/src/componentManifest/generator.ts(2 hunks)code/renderers/react/src/componentManifest/getComponentImports.test.ts(1 hunks)code/renderers/react/src/componentManifest/getComponentImports.ts(1 hunks)code/renderers/react/src/componentManifest/reactDocgen.ts(3 hunks)code/renderers/react/src/componentManifest/utils.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/core/src/core-server/dev-server.tscode/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/core-server/manifest.tscode/renderers/react/src/componentManifest/getComponentImports.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/reactDocgen.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/core/src/core-server/dev-server.tscode/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/core-server/manifest.tscode/renderers/react/src/componentManifest/getComponentImports.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/reactDocgen.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/core/src/core-server/dev-server.tscode/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/core-server/manifest.tscode/renderers/react/src/componentManifest/getComponentImports.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/reactDocgen.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-server/dev-server.tscode/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.tscode/core/src/core-server/manifest.tscode/renderers/react/src/componentManifest/getComponentImports.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/reactDocgen.ts
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
🧠 Learnings (29)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the spy: true option
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Mock external dependencies in tests using `vi.mock()` (e.g., filesystem, loggers)
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without vi.mocked()
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to access and implement mock behaviors
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to type and access mocked functions
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with vi.mocked()
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/getComponentImports.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of beforeEach blocks
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in beforeEach blocks
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.test.tscode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to scripts/**/*.{ts,js,mjs} : In Node.js scripts, use `storybook/internal/node-logger` instead of `console.*`
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to {code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs} : Do not use `console.log`, `console.warn`, or `console.error` directly unless in isolated files where importing loggers would significantly increase bundle size
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should be placed in beforeEach blocks
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Place all mocks at the top of the test file before any test cases
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/.storybook/** : Place internal UI Storybook configuration in `code/.storybook/` and maintain it there
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
🧬 Code graph analysis (6)
code/core/src/core-server/dev-server.ts (3)
code/renderers/react/src/componentManifest/generator.ts (1)
componentManifestGenerator(22-183)code/renderers/react/src/preset.ts (1)
componentManifestGenerator(11-11)code/renderers/react/src/componentManifest/utils.ts (1)
invariant(23-31)
code/renderers/react/src/componentManifest/getComponentImports.test.ts (3)
code/renderers/react/src/componentManifest/test-utils.ts (1)
fsMocks(3-112)code/renderers/react/src/componentManifest/getComponentImports.ts (2)
getImports(188-438)getComponentImports(440-468)code/core/src/csf-tools/CsfFile.ts (1)
loadCsf(1060-1064)
code/renderers/react/src/componentManifest/generator.ts (7)
code/core/src/types/modules/core-common.ts (2)
ComponentManifest(348-364)PresetPropertyFn(656-661)code/renderers/react/src/componentManifest/reactDocgen.ts (2)
DocObj(23-27)getReactDocgen(81-146)code/renderers/react/src/componentManifest/utils.ts (4)
invalidateCache(117-121)cachedReadFileSync(123-123)cachedFindUp(125-125)invariant(23-31)code/renderers/react/src/componentManifest/getComponentImports.ts (1)
getComponentImports(440-468)code/core/src/csf-tools/enrichCsf.ts (1)
extractDescription(158-178)code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(6-227)
code/renderers/react/src/componentManifest/getComponentImports.ts (3)
code/renderers/react/src/componentManifest/utils.ts (1)
cached(44-115)code/core/src/common/utils/interpret-files.ts (1)
resolveImport(25-58)code/renderers/react/src/componentManifest/reactDocgen.ts (3)
matchPath(47-59)getReactDocgen(81-146)getImportTag(175-179)
code/renderers/react/src/componentManifest/generator.test.ts (3)
code/renderers/react/src/componentManifest/test-utils.ts (1)
fsMocks(3-112)code/renderers/react/src/componentManifest/generator.ts (1)
componentManifestGenerator(22-183)code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
StoryIndexGenerator(104-874)
code/renderers/react/src/componentManifest/reactDocgen.ts (6)
code/core/src/common/utils/interpret-files.ts (1)
supportedExtensions(6-17)code/renderers/react/src/componentManifest/utils.ts (2)
cached(44-115)cachedReadFileSync(123-123)code/core/src/common/utils/paths.ts (1)
getProjectRoot(11-57)code/presets/react-webpack/src/loaders/react-docgen-loader.ts (1)
getReactDocgenImporter(150-169)code/frameworks/react-vite/src/plugins/react-docgen.ts (1)
getReactDocgenImporter(100-130)code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
1-11: Replace console.error with Storybook loggerLine 179 still emits
console.error, which violates the repository rule to use Storybook’s loggers incode/**. Switch tostorybook/internal/node-loggerinstead. As per coding guidelines.import { resolveImport } from 'storybook/internal/common'; import { type CsfFile } from 'storybook/internal/csf-tools'; +import { logger } from 'storybook/internal/node-logger'; @@ - } catch (e) { - console.error(e); + } catch (e) { + logger.error( + `Failed to resolve import "${component.importId}" from "${storyFilePath ?? 'unknown'}":`, + e + ); }[scratchpad_end -->
Oops need to correct comment this contains scratchpad closing? Wait we included [scratchpad_end?], we must keep comment.Need to ensure comment ended properly:
<!-- review_comment_end -->. Also remove stray[scratchpad_end -->we added by accident. Let's fix final snippet.Redo comment:
Need to produce final comment with correct format, no stray.
Let's craft.
Also applies to: 178-180
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
code/renderers/react/src/componentManifest/generator.ts (1)
60-62: Add error handling for malformed package.json.The
JSON.parsecall on line 61 can throw if package.json is malformed, potentially crashing the manifest generation. WhilecachedReadFileSynchandles file I/O errors, it doesn't protect against invalid JSON.Apply this diff to add a try-catch:
- const packageName = nearestPkg - ? JSON.parse(cachedReadFileSync(nearestPkg, 'utf-8') as string).name - : undefined; + let packageName: string | undefined; + if (nearestPkg) { + try { + packageName = JSON.parse(cachedReadFileSync(nearestPkg, 'utf-8') as string).name; + } catch (e) { + // Skip if package.json is malformed + packageName = undefined; + } + }This ensures manifest generation continues even if a package.json file is corrupted or syntactically invalid.
🧹 Nitpick comments (1)
code/renderers/react/src/componentManifest/reactDocgen.ts (1)
167-211: Consider using a more stable cache key.Line 210 concatenates
path + JSON.stringify(component)as the cache key. If thecomponentobject has properties added in non-deterministic order, this could produce different keys for semantically identical inputs, reducing cache effectiveness.Consider using specific fields from
componentfor the key:- { name: 'getReactDocgen', key: (path, component) => path + JSON.stringify(component) } + { + name: 'getReactDocgen', + key: (path, component) => + `${path}:${component.componentName}:${component.importName ?? ''}:${component.localImportName ?? ''}` + }Alternatively, ensure
componentproperties are serialized in a stable order, or rely on the default JSON.stringify behavior if property insertion order is guaranteed in your runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code/renderers/react/src/componentManifest/generator.ts(2 hunks)code/renderers/react/src/componentManifest/getComponentImports.ts(1 hunks)code/renderers/react/src/componentManifest/reactDocgen.ts(3 hunks)code/renderers/react/src/componentManifest/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/renderers/react/src/componentManifest/getComponentImports.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/reactDocgen.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to scripts/**/*.{ts,js,mjs} : In Node.js scripts, use `storybook/internal/node-logger` instead of `console.*`
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to {code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs} : Do not use `console.log`, `console.warn`, or `console.error` directly unless in isolated files where importing loggers would significantly increase bundle size
Applied to files:
code/renderers/react/src/componentManifest/generator.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen.ts
🧬 Code graph analysis (2)
code/renderers/react/src/componentManifest/generator.ts (7)
code/core/src/types/modules/core-common.ts (2)
ComponentManifest(348-364)PresetPropertyFn(656-661)code/renderers/react/src/componentManifest/reactDocgen.ts (2)
DocObj(24-28)getReactDocgen(167-211)code/renderers/react/src/componentManifest/utils.ts (4)
invalidateCache(118-122)cachedReadFileSync(124-124)cachedFindUp(126-126)invariant(24-32)code/renderers/react/src/componentManifest/getComponentImports.ts (1)
getComponentImports(438-466)code/core/src/csf-tools/enrichCsf.ts (1)
extractDescription(158-178)code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(6-227)
code/renderers/react/src/componentManifest/reactDocgen.ts (5)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
ComponentRef(11-19)code/core/src/common/utils/interpret-files.ts (1)
supportedExtensions(6-17)code/renderers/react/src/componentManifest/utils.ts (3)
cached(45-116)cachedResolveImport(128-128)cachedReadFileSync(124-124)code/core/src/common/utils/paths.ts (1)
getProjectRoot(11-57)code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (5)
code/renderers/react/src/componentManifest/utils.ts (1)
34-116: Well-designed caching implementation.The module-level memoization stores and generic
cached()helper provide a solid foundation for performance optimization. Key strengths:
- WeakMap-based stores allow GC when functions are dereferenced
- Separate handling for sync/async results with resolved-value-only caching
- Graceful fallback on serialization errors
- Verbose logging for cache diagnostics
Note: The implementation intentionally does not de-duplicate concurrent calls (as documented in the comment on line 44), so simultaneous calls for the same uncached key will both compute the result.
code/renderers/react/src/componentManifest/reactDocgen.ts (2)
35-55: Improved component matching logic.The refactored
getMatchingDocgennow checks multiple identifier variants (importName,localImportName,componentName) against docgen metadata (exportName,displayName,actualName), providing more robust matching for different import patterns and component definitions.
71-211: Robust docgen integration with caching and error handling.The cached helpers (
getTsConfig,parseWithReactDocgen,gatherDocgensForPath,getReactDocgen) form a well-structured pipeline:
- TypeScript path mapping support via
matchPath- Recursive re-export resolution with depth limiting (max depth 5, filters node_modules)
- Discriminated union return type clearly separates success/error cases
- Detailed error messages with analyzed file context for debugging
The use of
cached()wrappers eliminates the cache-population bug flagged in earlier reviews.code/renderers/react/src/componentManifest/generator.ts (2)
25-29: Good practice: Cache invalidation and performance instrumentation.Calling
invalidateCache()at the start (line 25) ensures fresh results per build, avoiding stale cached data across runs. The performance timing logs (lines 27-29, 174) provide valuable diagnostics for optimization efforts.
42-172: Streamlined manifest generation with improved import resolution.The refactored flow effectively integrates the new caching infrastructure:
- Uses
cachedReadFileSyncto minimize I/O overhead- Leverages
getComponentImportsfor comprehensive import analysis- Matches components by multiple identifiers (name, localImportName, importName) and title patterns
- Enriches per-story metadata with JSDoc-derived descriptions and summaries
- Returns discriminated union from
getReactDocgenfor explicit success/error handlingThe fallback error messages for missing meta.component and unresolved imports provide clear guidance to users.
| const keyOf: (...args: A) => string = | ||
| opts.key ?? | ||
| ((...args: A) => { | ||
| try { | ||
| // Prefer a stable string key based on the full arguments list | ||
| return JSON.stringify(args) ?? String(args[0]); | ||
| } catch { | ||
| // Fallback: use the first argument if it is not serializable | ||
| return String(args[0]); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Remove dead code in fallback logic.
The nullish coalescing operator on line 54 is unreachable. JSON.stringify(args) either returns a string or throws (caught by the catch block); it never returns null or undefined. The ?? String(args[0]) fallback is dead code.
Apply this diff to simplify:
- return JSON.stringify(args) ?? String(args[0]);
+ return JSON.stringify(args);The catch block already handles serialization failures by falling back to String(args[0]), making this ternary unnecessary.
📝 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.
| const keyOf: (...args: A) => string = | |
| opts.key ?? | |
| ((...args: A) => { | |
| try { | |
| // Prefer a stable string key based on the full arguments list | |
| return JSON.stringify(args) ?? String(args[0]); | |
| } catch { | |
| // Fallback: use the first argument if it is not serializable | |
| return String(args[0]); | |
| } | |
| }); | |
| const keyOf: (...args: A) => string = | |
| opts.key ?? | |
| ((...args: A) => { | |
| try { | |
| // Prefer a stable string key based on the full arguments list | |
| return JSON.stringify(args); | |
| } catch { | |
| // Fallback: use the first argument if it is not serializable | |
| return String(args[0]); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/utils.ts around lines 49 to 59,
the fallback operator "?? String(args[0])" after JSON.stringify(args) is dead
code because JSON.stringify either returns a string or throws; remove the
nullish coalescing and simplify the function to return JSON.stringify(args)
directly in the try block and keep the existing catch that falls back to
String(args[0]).
Closes #
What I did
The idea is:
This might not be enough in various scenarios:
For all those scenarios, I think we should recommend:
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
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-32912-sha-d9cc661f. Try it out in a new sandbox by runningnpx [email protected] sandboxor in an existing project withnpx [email protected] upgrade.More information
0.0.0-pr-32912-sha-d9cc661fkasper/manifest-auto-importd9cc661f1762206923)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=32912Summary by CodeRabbit
New Features
Improvements
Tests
Chores