Skip to content

fix: handle existing query params in preview iframe URL#34617

Open
justismailmemon wants to merge 2 commits into
storybookjs:nextfrom
justismailmemon:fix/34615-iframe-preview-url-query-params
Open

fix: handle existing query params in preview iframe URL#34617
justismailmemon wants to merge 2 commits into
storybookjs:nextfrom
justismailmemon:fix/34615-iframe-preview-url-query-params

Conversation

@justismailmemon
Copy link
Copy Markdown
Contributor

@justismailmemon justismailmemon commented Apr 23, 2026

What I changed

This fixes malformed preview iframe URLs when --preview-url already contains query parameters.

Previously, previewHref always appended Storybook params with a hardcoded ?, which produced invalid URLs like:

iframe.html?foo=bar?id=<story-id>&viewMode=story

This change uses the correct separator based on whether previewBase already includes a query string:

  • ? when no query params exist yet
  • & when query params are already present

Example

Before:
iframe.html?foo=bar?id=button--primary&viewMode=story

After:
iframe.html?foo=bar&id=button--primary&viewMode=story

Why

--preview-url can be set to a URL that already includes query params. In that case, additional params must be appended with & instead of ?.

Closes #34615

Summary by CodeRabbit

  • Bug Fixes
    • Fixed URL generation to correctly handle query string delimiters, preventing invalid URLs when appending parameters to URLs that already contain query strings.
    • Preserved URL fragments (hashes) when adding query parameters so anchors remain intact.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2850802e-e64d-4cb0-80a6-837810674d91

📥 Commits

Reviewing files that changed from the base of the PR and between b8a1f7b and 9cf69c7.

📒 Files selected for processing (1)
  • code/core/src/manager-api/modules/url.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • code/core/src/manager-api/modules/url.ts

📝 Walkthrough

Walkthrough

Updated getStoryHrefs to preserve any #fragment in previewBase (removed before adding query params and re-appended after) and to choose the correct query delimiter (? or &) when building previewHref, avoiding malformed URLs with duplicated ? or misplaced fragments.

Changes

Cohort / File(s) Summary
Preview URL construction
code/core/src/manager-api/modules/url.ts
getStoryHrefs now strips and preserves #fragment from previewBase, appends query parameters using ? or & depending on existing query string, then reattaches the fragment—preventing duplicated delimiters and misplaced fragments in previewHref.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/manager-api/modules/url.ts`:
- Around line 285-288: The preview URL construction puts added query params
after any existing hash fragment (so id/viewMode land in the fragment); update
previewHref generation to split previewBase on '#' (e.g., const
[baseWithoutHash, hashPart] = previewBase.split('#', 2)), compute the separator
against baseWithoutHash (const separator = baseWithoutHash.includes('?') ? '&' :
'?'), append the id/viewMode/other params to baseWithoutHash, then reassemble
previewHref as `${baseWithoutHash}${separator}...${hashPart ? `#${hashPart}` :
''}` so all new params are in the query string before any fragment; apply the
same approach if managerBase may include a fragment when building managerHref.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 75b1e0e2-a161-4a7d-8f6e-e70cd3422e96

📥 Commits

Reviewing files that changed from the base of the PR and between b9549a6 and b8a1f7b.

📒 Files selected for processing (1)
  • code/core/src/manager-api/modules/url.ts

Comment thread code/core/src/manager-api/modules/url.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Empathy Queue (prioritized)

Development

Successfully merging this pull request may close these issues.

[Bug]: --preview-url with query string produces malformed iframe URL in 10.2+

2 participants