Manifests: Fix Attached MDX causing wrong component entries#34101
Conversation
|
View your CI Pipeline Execution ↗ for commit 6ae40b3
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Pull request overview
Fixes React component manifest generation when attached MDX docs are present, ensuring component entries resolve to the correct CSF stories file and that attached docs storiesImports ordering correctly prioritizes the <Meta of={...} /> target.
Changes:
- Update React manifest generation to select a single “best” index entry per component id, preferring CSF story entries over attached-MDX docs entries.
- Fix
StoryIndexGeneratordependency ordering sostoriesImports[0]is the “primary” CSF file referenced by<Meta of={...} />. - Add regression tests + mock MDX fixture covering both failure modes.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| code/renderers/react/src/componentManifest/generator.ts | Replaces uniqBy selection with deterministic component-entry selection that prefers story entries over attached docs for the same component id. |
| code/renderers/react/src/componentManifest/generator.test.ts | Adds a regression test ensuring story entries win over attached-MDX docs entries when both exist. |
| code/core/src/core-server/utils/mockdata/complex/MetaOfImportOrder.mdx | New fixture where import order differs from <Meta of={...} /> target, to validate storiesImports ordering. |
| code/core/src/core-server/utils/StoryIndexGenerator.ts | Fixes sorting of docs dependencies so the <Meta of={...} /> story file is placed first in storiesImports. |
| code/core/src/core-server/utils/StoryIndexGenerator.test.ts | Adds a regression test asserting the correct storiesImports order for the new MDX fixture. |
You can also share your feedback on Copilot code review. Take the survey.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPrioritizes CSF (story) files when resolving docs Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/core-server/utils/__mockdata__/complex/MetaOfImportOrder.mdx`:
- Around line 7-9: The wording in the MDX fixture is incorrect: it currently
says "even when it is not the last import" but since AStories (A.stories) is
imported second it is actually the last import; update the sentence to "even
when it is not the first import" (or alternatively reorder the import statements
so A.stories is first). Locate the text referencing A.stories, AStories and
storiesImports in MetaOfImportOrder.mdx and change the phrase accordingly to
keep the fixture self-explanatory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f5e42bf-e7a3-4e75-833a-c42a4c2fbf38
📒 Files selected for processing (5)
code/core/src/core-server/utils/StoryIndexGenerator.test.tscode/core/src/core-server/utils/StoryIndexGenerator.tscode/core/src/core-server/utils/__mockdata__/complex/MetaOfImportOrder.mdxcode/renderers/react/src/componentManifest/generator.test.tscode/renderers/react/src/componentManifest/generator.ts
…ortOrder.mdx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Closes #
What I did
This PR fixes an issue where attached MDX files would sometimes cause the manifest generator to point component entries to the wrong stories file.
It also fixes a bug in
StoryIndexGenerator, where it wouldn't correctly sortstoriesImportsand put the "primary" story (as defined via<Meta of={PrimaryStoryFile} />) first in the array, even though it should.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
See unit tests
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This pull request has been released as version
0.0.0-pr-34101-sha-6ae40b39. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34101-sha-6ae40b39 sandboxor in an existing project withnpx storybook@0.0.0-pr-34101-sha-6ae40b39 upgrade.More information
0.0.0-pr-34101-sha-6ae40b39jeppe/fix-manifest-attached-mdx6ae40b391773232895)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=34101Summary by CodeRabbit
Bug Fixes
Tests