Manifests: Add support for summaries in MDX files#33475
Conversation
JReinhold
added
feature request
addon: docs
ci:normal
manifest
|
View your CI Pipeline Execution ↗ for commit 571d5d7
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughAdds MDX summary extraction to the docs manifest: exports an MDX analyzer from core-server, uses it when creating DocsManifestEntry to attach optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds support for extracting summaries from MDX files to docs manifests. The summary can be specified in MDX files using <Meta summary="..." /> syntax and will be included in both attached and unattached docs manifest entries.
Key Changes
- Updates
@storybook/docs-mdxdependency to version3.2.0--canary.20.3700111.0to access theanalyzefunction with summary extraction support - Modifies
createDocsManifestEntryto extract and include summary field from MDX content - Adds comprehensive test coverage for summary extraction in both success and error scenarios
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
yarn.lock |
Updates @storybook/docs-mdx package version and checksum for the canary release |
code/core/package.json |
Updates @storybook/docs-mdx dev dependency version |
code/addons/docs/package.json |
Adds @storybook/docs-mdx to dev dependencies |
code/addons/docs/src/manifest.ts |
Imports and uses analyze function to extract summaries from MDX content, adds optional summary field to DocsManifestEntry interface |
code/addons/docs/src/manifest.test.ts |
Adds comprehensive tests for summary extraction including cases with and without summaries, and for both attached and unattached docs |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
code/addons/docs/src/manifest.ts (1)
48-74: Consider distinguishing analysis errors from file read errors.If
analyze()throws an exception, the current error handling will catch it alongside file read errors. The resulting error message may be misleading since it won't distinguish between "file not found" and "MDX analysis failed."Consider adding specific error handling for the analysis step to provide clearer error context.
🔎 Proposed refinement to error handling
export async function createDocsManifestEntry(entry: DocsIndexEntry): Promise<DocsManifestEntry> { const absolutePath = path.join(process.cwd(), entry.importPath); try { const content = await fs.readFile(absolutePath, 'utf-8'); - const { summary } = await analyze(content); + let summary: string | undefined; + try { + const result = await analyze(content); + summary = result.summary; + } catch (analysisError) { + logger.warn(`Failed to analyze MDX content for ${entry.importPath}:`, analysisError); + // Continue without summary rather than failing the entire entry + } return { id: entry.id, name: entry.name, path: entry.importPath, title: entry.title, content, ...(summary && { summary }), }; } catch (err) { return { id: entry.id, name: entry.name, path: entry.importPath, title: entry.title, error: { name: err instanceof Error ? err.name : 'Error', message: err instanceof Error ? err.message : String(err), }, }; } }code/addons/docs/src/manifest.test.ts (1)
371-424: Consider consolidating redundant test cases.The tests at Lines 371-388 and Lines 390-424 appear to duplicate coverage already provided by earlier tests:
- Lines 177-201 already verify summary in unattached docs
- Lines 134-175 already verify summary in attached docs
The new tests don't seem to add distinct assertions or edge cases beyond what's covered in the existing tests.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
code/addons/docs/package.jsoncode/addons/docs/src/manifest.test.tscode/addons/docs/src/manifest.tscode/core/package.json
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/core/package.jsoncode/addons/docs/package.jsoncode/addons/docs/src/manifest.tscode/addons/docs/src/manifest.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/core/package.jsoncode/addons/docs/package.jsoncode/addons/docs/src/manifest.tscode/addons/docs/src/manifest.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/docs/src/manifest.tscode/addons/docs/src/manifest.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/docs/src/manifest.tscode/addons/docs/src/manifest.test.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/docs/src/manifest.tscode/addons/docs/src/manifest.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/addons/docs/src/manifest.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/addons/docs/src/manifest.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Usevi.mock()to mock file system, loggers, and other external dependencies in tests
Files:
code/addons/docs/src/manifest.test.ts
🧠 Learnings (19)
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
code/core/package.jsoncode/addons/docs/package.json
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/package.jsoncode/addons/docs/package.json
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/core/package.jsoncode/addons/docs/package.json
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Applied to files:
code/core/package.jsoncode/addons/docs/package.json
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Use Node.js 22.21.1 (see `.nvmrc`) and Yarn 4.9.1 for development
Applied to files:
code/core/package.jsoncode/addons/docs/package.json
📚 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/core/package.json
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/package.json
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to {code/addons,code/frameworks}/**/README.md : Update README files for significant changes and include code examples in addon/framework documentation
Applied to files:
code/addons/docs/package.jsoncode/addons/docs/src/manifest.tscode/addons/docs/src/manifest.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/node-logger` for server-side logging in Node.js code
Applied to files:
code/addons/docs/src/manifest.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Use `vi.mock()` to mock file system, loggers, and other external dependencies in tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
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/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Applied to files:
code/addons/docs/src/manifest.test.ts
🧬 Code graph analysis (1)
code/addons/docs/src/manifest.test.ts (3)
code/core/src/shared/constants/tags.ts (2)
Tag(2-19)Tag(25-25)code/addons/docs/src/manifest.ts (1)
manifests(130-188)code/renderers/react/src/componentManifest/generator.ts (1)
manifests(99-208)
⏰ 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). (5)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (6)
code/core/package.json (1)
239-239: Consistent version with docs addon.The version matches the one added to
code/addons/docs/package.json(Line 93), maintaining consistency across packages.code/addons/docs/src/manifest.ts (2)
24-24: LGTM!The optional
summaryfield is properly typed and aligns with the conditional inclusion pattern used at Line 60.
3-3: No changes needed. The implementation correctly uses theanalyzefunction API from@storybook/docs-mdx. The function reliably extracts optional summary metadata from MDX files and integrates seamlessly with error handling.code/addons/docs/src/manifest.test.ts (3)
20-36: LGTM!The mock MDX content properly includes Meta components with summary attributes, providing realistic test fixtures for the new functionality.
167-175: LGTM!The updated assertions properly validate both the summary field and content field separately, using appropriate matchers (
toMatchObjectfor partial object matching andtoContainfor string content).
426-453: LGTM!This test case properly validates the absence of the summary field when the MDX content doesn't contain a Meta component with a summary attribute. This is an important edge case.
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/addons/docs/src/manifest.test.ts (1)
377-430: Remove redundant test cases.These two test cases duplicate coverage already provided by earlier tests:
- Lines 377-394 ("should include summary in unattached docs entries when available") is redundant with the test at line 179, which already validates summary at line 200.
- Lines 396-430 ("should include summary in attached docs entries when available") is redundant with the test at line 134, which already validates summary at line 172.
The earlier tests provide more comprehensive coverage by checking both
summaryandcontentfields.🔎 Proposed fix to remove redundant tests
- it('should include summary in unattached docs entries when available', async () => { - const manifestEntries: IndexEntry[] = [ - { - id: 'standalone--docs', - name: 'docs', - title: 'Standalone', - type: 'docs', - importPath: './Standalone.mdx', - tags: [Tag.MANIFEST, Tag.UNATTACHED_MDX], - storiesImports: [], - } satisfies DocsIndexEntry, - ]; - - const result = (await manifests(undefined, { manifestEntries } as any)) as ManifestResult; - - expect(result).toHaveProperty('docs'); - expect(result.docs?.docs['standalone--docs'].summary).toBe('A standalone documentation page'); - }); - - it('should include summary in attached docs entries when available', async () => { - const existingManifests = { - components: { - v: 0, - components: { - example: { - id: 'example', - path: './Example.stories.tsx', - name: 'Example', - stories: [], - jsDocTags: {}, - }, - }, - }, - }; - const manifestEntries: IndexEntry[] = [ - { - id: 'example--docs', - name: 'docs', - title: 'Example', - type: 'docs', - importPath: './Example.mdx', - tags: [Tag.MANIFEST, Tag.ATTACHED_MDX], - storiesImports: ['./Example.stories.tsx'], - } satisfies DocsIndexEntry, - ]; - - const result = (await manifests(existingManifests, { - manifestEntries, - } as any)) as ManifestResult; - - expect(result.components?.components.example.docs?.['example--docs'].summary).toBe( - 'An example documentation page' - ); - }); -
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/addons/docs/src/manifest.test.tscode/addons/docs/src/manifest.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/addons/docs/src/manifest.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/addons/docs/src/manifest.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/addons/docs/src/manifest.test.ts
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/addons/docs/src/manifest.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/addons/docs/src/manifest.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/docs/src/manifest.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/docs/src/manifest.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Usevi.mock()to mock file system, loggers, and other external dependencies in tests
Files:
code/addons/docs/src/manifest.test.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/docs/src/manifest.test.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to {code/addons,code/frameworks}/**/README.md : Update README files for significant changes and include code examples in addon/framework documentation
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to {code/addons,code/frameworks}/**/README.md : Update README files for significant changes and include code examples in addon/framework documentation
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Use `vi.mock()` to mock file system, loggers, and other external dependencies in tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
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/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/addons/docs/src/manifest.test.ts
🧬 Code graph analysis (1)
code/addons/docs/src/manifest.test.ts (3)
code/core/src/shared/constants/tags.ts (2)
Tag(2-19)Tag(25-25)code/addons/docs/src/manifest.ts (1)
manifests(131-189)code/renderers/react/src/componentManifest/generator.ts (1)
manifests(99-208)
⏰ 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). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (4)
code/addons/docs/src/manifest.test.ts (4)
11-14: Verify mocking pattern compliance with project guidelines.The mock does not use the
spy: trueoption. As per coding guidelines, "Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests."While the memfs replacement pattern is valid for testing filesystem operations, please verify this approach aligns with the project's mocking standards.
As per coding guidelines, the recommended pattern would be:
-vi.mock('node:fs/promises', async () => { +vi.mock('node:fs/promises', { spy: true }, async () => { const memfs = await vi.importActual<typeof import('memfs')>('memfs'); return memfs.fs.promises; });However, verify whether full module replacement (without
spy: true) is acceptable for memfs-based filesystem testing in this codebase.
20-33: LGTM!The updated mock data correctly includes MDX Meta blocks with summary attributes, aligning with the PR's objective to support summary extraction from MDX files.
167-176: LGTM!The updated assertions correctly validate the new
summaryfield while usingtoContainfor content checks. This approach is more flexible and appropriate for testing partial content matches.Also applies to: 195-204, 251-266, 363-366
432-459: LGTM!This test provides valuable coverage for the case when no summary is present in the MDX content. Unlike the tests at lines 377-430, this one adds unique value by explicitly verifying the absence of the
summaryfield.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/addons/docs/src/manifest.ts (1)
51-65: Track the performance optimization TODO.The TODO comment correctly identifies a performance concern: the MDX file is analyzed twice—once during story index generation and again here during manifest creation. Since MDX analysis requires compilation, this duplication has a tangible performance cost.
The current implementation is functionally correct with proper error handling, but the caching optimization mentioned in the TODO would improve build performance.
Would you like me to open a tracking issue for this performance optimization? The issue could track the work to cache and reuse MDX analysis results between story index generation and manifest creation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
code/addons/docs/src/manifest.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/addons/docs/src/manifest.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/addons/docs/src/manifest.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/addons/docs/src/manifest.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/docs/src/manifest.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/addons/docs/src/manifest.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to {code/addons,code/frameworks}/**/README.md : Update README files for significant changes and include code examples in addon/framework documentation
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to {code/addons,code/frameworks}/**/README.md : Update README files for significant changes and include code examples in addon/framework documentation
Applied to files:
code/addons/docs/src/manifest.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/node-logger` for server-side logging in Node.js code
Applied to files:
code/addons/docs/src/manifest.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/core-server/index.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/core-server/index.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/core-server/index.ts
⏰ 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). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (4)
code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
24-25: LGTM: Static import simplifies the code.The change from dynamic import to static top-level import makes the dependency explicit and removes unnecessary overhead. This aligns well with the new public API pattern where
analyzeMdxis exported from core-server.code/core/src/core-server/index.ts (1)
16-16: LGTM: Well-designed public API addition.The export creates a clear, stable public API for MDX analysis functionality with descriptive naming. This enables downstream packages to access the functionality through a well-defined interface.
code/addons/docs/src/manifest.ts (2)
5-5: LGTM: Import correctly references the new public API.The import properly uses the newly exported
analyzeMdxfunction from the core-server, enabling MDX analysis for summary extraction.
23-23: LGTM: Interface extension follows established patterns.The optional
summaryfield correctly extends the DocsManifestEntry interface, following the same pattern as other optional fields likecontentanderror.
Works on storybookjs/mcp#107
Depends on storybookjs/docs-mdx#20
What I did
Added support for extracting summaries from MDX files to docs manifests via:
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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.