Manifests: Support !manifest tag in preview files
#33406
Conversation
…fest preset now takes a pre-filtered list of entries, support users globally disabling manifest with '!manifest' in preview.js
There was a problem hiding this comment.
Pull request overview
This PR refactors the manifest tag filtering mechanism to enable users to globally disable manifests with !manifest in preview files and then selectively opt-in specific stories/docs. Previously, the manifest tag was synthetically applied during manifest generation; now it exists directly in the story index.
Key Changes:
- Added 'manifest' to default tags in StoryIndexGenerator, making it combinable with user-defined tags including negations
- Modified
experimental_manifestspreset API to receive pre-filteredmanifestEntriesinstead of the fullindex - Updated manifest generation logic to filter by the 'manifest' tag upstream before passing to preset implementations
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| code/core/src/core-server/utils/StoryIndexGenerator.ts | Adds 'manifest' to the default project tags, enabling tag combination and negation support |
| code/core/src/core-server/utils/manifests/manifests.ts | Introduces getManifests helper that filters entries by 'manifest' tag before passing to presets; updates API signature |
| code/core/src/core-server/utils/manifests/manifests.test.ts | Adds test coverage for manifest tag filtering and validates the new API signature |
| code/renderers/react/src/componentManifest/generator.ts | Updates to receive manifestEntries instead of index; removes local combineTags logic; updates extractStories to filter against pre-filtered entries |
| code/renderers/react/src/componentManifest/generator.test.ts | Updates test setup to provide manifestEntries instead of index; adds manifest tag to test fixtures and updates snapshots |
| code/renderers/react/src/componentManifest/fixtures.ts | Adds 'manifest' tag to all test fixture entries and includes missing 'title' properties in meta objects |
JReinhold
added
maintenance
|
View your CI Pipeline Execution ↗ for commit 6d43e7a
☁️ Nx Cloud last updated this comment at |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughCentralizes manifest extraction and propagation: StoryIndexGenerator now includes a default "manifest" tag; server-side manifests logic filters index entries for that tag via a new getManifests helper; the React component manifest generator now accepts an explicit list of manifestEntries instead of a full story index. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Dev Server / Caller
participant GM as getManifests (manifests.ts)
participant SI as StoryIndexGenerator
participant PM as experimental_manifests preset
participant CG as React Component Manifest Generator
Note over Client,SI: Server requests manifest data
Client->>GM: request manifests (presets)
GM->>SI: resolve storyIndexGenerator
SI-->>GM: return index { entries: ... }
rect rgba(120,180,120,0.12)
Note over GM: Filter entries by 'manifest' tag
GM->>GM: select entries with tag 'manifest' -> manifestEntries[]
end
GM->>PM: call experimental_manifests({ manifestEntries })
PM-->>GM: return manifests object
GM-->>Client: return manifests
Note over Client,CG: Component generator now consumes manifestEntries
Client->>CG: pass manifestEntries[]
CG->>CG: process only manifestEntries to build component manifests
CG-->>Client: component manifests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/core/src/core-server/utils/index-json.test.ts (1)
18-27: Consider addingspy: trueto mocks for consistency with coding guidelines.The coding guidelines specify using
spy: truefor all package and file mocks. While line 19 follows this pattern, lines 18 and 20 don't.🔎 Suggested refactor
-vi.mock('watchpack'); +vi.mock('watchpack', { spy: true }); vi.mock('es-toolkit/function', { spy: true }); -vi.mock('storybook/internal/node-logger'); +vi.mock('storybook/internal/node-logger', { spy: true });Note: If these mocks require custom implementations in
beforeEach, those can still be applied viavi.mocked().Based on coding guidelines for Vitest test files.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/core/src/core-server/utils/StoryIndexGenerator.test.tscode/core/src/core-server/utils/index-json.test.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/core/src/core-server/utils/index-json.test.tscode/core/src/core-server/utils/StoryIndexGenerator.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/core/src/core-server/utils/index-json.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/core/src/core-server/utils/index-json.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/core-server/utils/index-json.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-server/utils/index-json.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/core/src/core-server/utils/index-json.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/core/src/core-server/utils/index-json.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/core/src/core-server/utils/index-json.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.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: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/core/src/core-server/utils/index-json.test.ts (1)
113-491: LGTM! Snapshot correctly reflects the new manifest tag behavior.The inline snapshot has been consistently updated across all 25 index entries (docs and stories) to include the "manifest" tag. This aligns with the PR objective of adding the manifest tag by default to support
!manifestfiltering in preview files.code/core/src/core-server/utils/StoryIndexGenerator.test.ts (1)
82-2379: LGTM! Test snapshots correctly updated to reflect new manifest tagging behavior.The systematic addition of the "manifest" tag to all test snapshots aligns with the PR's objective to include manifest tags on index entries by default. The changes are consistent throughout, and the intentional omission in test cases where
projectTagsis explicitly overridden (e.g., lines 1330, 1024) demonstrates proper handling of edge cases.
Originally proposed by @cdedreuille
What I did
Refactored the way the manifest logic filters entries by the
manifesttag, so that now themanifesttag actually exists in the index instead of being synthetically applied during manifest creation. This enables support for users disabling manifests globally with the!manifesttag in a preview file, and then selectively opting-in stories/docs afterwards, instead of the default worklfow of opting-out stories/docs.It also includes a small change to the API of the
experimental_manifestspreset. It no longer takes in the raw index as input, but instead a pre-filtered list of entries, as I assume all manifest generators will want to filter by themanifesttag anyway.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
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.