Composition: Fix composed story search #29453
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
| Object.values(refs).forEach((ref) => { | ||
| if (ref.index) { | ||
| Object.assign(combinedIndex, ref.index); | ||
| } | ||
| }); |
There was a problem hiding this comment.
style: potential race condition if ref.index is modified during iteration - consider using Object.entries and destructuring for safer iteration
| while ( | ||
| node && | ||
| 'parent' in node && | ||
| node.parent && | ||
| combinedIndex[node.parent] && | ||
| fullStoryName.length < 24 | ||
| ) { |
There was a problem hiding this comment.
logic: check node.parent type explicitly since it could be undefined from external sources
jsingh0026
changed the title
shilman
changed the title
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 39601d1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
There was a problem hiding this comment.
@jsingh0026 thank you so much for this PR! can we please add an automated test or two to check the new functionality? 🙏
There was a problem hiding this comment.
Thanks a lot for the fix @jsingh0026! I added some tests to make sure we don't regress.
Closes #29268
What I did
This PR resolves the issue of composition failures by including external Storybook entries in the index data. Previously, the current story ID was only searched in the internal Storybook index, which caused mismatches. By incorporating external Storybook IDs into the combined index, full story names are now correctly retrieved and visible on mobile devices without breaking.
Screen.Recording.2024-10-26.at.2.39.59.AM.mov
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
yarn start.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/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 canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
Enhanced external Storybook composition by implementing index merging functionality to correctly display story paths in mobile view.
combineIndexesfunction incode/core/src/manager/components/mobile/navigation/MobileNavigation.tsxto merge root and external ref indexesuseFullStoryNamehook to use combined indexes for proper story path resolution