Repository navigation
Manager: Resolve preview iframe URL against the document base URI - #35245
Conversation
|
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 (11)
🚧 Files skipped from review as they are similar to previous changes (11)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. WalkthroughAdds Changesiframe URL resolution via document.baseURI
Fixture manifest and MCP formatting
Merge Risk: ⚪ Minimal · up to The PR updates preview iframe URL resolution for sub-path and custom base-URI hosting; no actionable merge-blocking risk remains beyond normal checks and review. Comment |
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
…ath-34733 # Conflicts: # code/core/src/manager-api/modules/url.ts
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Closes #34733
What I did
When Storybook is hosted under a sub-path, the preview iframe may fail to load, and the manager may hang on the loading spinner indefinitely.
The manager computed the
iframe.htmlURL by string-manipulating the router pathname (location.pathname) instead of resolving it as the browser does for the manager's own relative assets. This produced a wrong URL in two cases:/storybook): the sub-path was wrongly prepended, pointing the iframe at a non-existent…/iframe.html→ preview never connects, infinite loading.<base href>(the documented way to host under a sub-path): it was ignored entirely, so the iframe pointed at the wrong location.The fix resolves
iframe.htmlagainst the manager document's base URI (document.baseURI) using theURLconstructor - the same base the browser uses for the manager's own assets. This honors a custom<base href>, follows the browser's directory rules for paths with/without a trailing slash, and keeps the iframe on the manager's origin. Theorigin/networkoptions forgetStoryHrefs(used for shareable "open in isolation" links) are unchanged.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Reproduces the on-prem sub-path hosting from the issue:
yarn task --task build --start-from auto --template react-webpack5/default-ts(or any template).http://localhost:8080/storybook(no trailing slash). A quick way: serve a parent folder and place the build under./storybook, then openhttp://localhost:8080/storybook.srcresolves to a non-existent…/storybook/iframe.html)./iframe.html.<base href>: in.storybook/main.tsset the builderbaseto/design-system/and addmanagerHead: (head) => \${head}`, build, and serve under/design-system/. The iframe should resolve to/design-system/iframe.html` and stories should load.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.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake 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>