Manager: Fix system query parameters being overridable by JReinhold · Pull Request #33535 · storybookjs/storybook

JReinhold · 2026-01-14T13:18:04Z

Closes #

What I did

Fixed a case where eg. having ?id=bla-bla in the manager URL would propagate down into the preview URL, resulting in the preview having iframe?id=story-id&id=blabla which caused everything to fail.

This is specifically a problem in VSCode's new Simple Browser (in preview), that sets an id query param on all pages it visits.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

stories
unit tests
integration tests
end-to-end tests

Manual testing

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Open a Storybook with any story
Add the following to the URL: &id=bla-di-bloops and navigate
The story should not render with a "Couldn't find story matching 'bla-di-bloops'" error, it should render fine.

Documentation

Add or update documentation reflecting your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Checklist for Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts
Make 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/core team 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

Bug Fixes
- Fixed URL parameter handling to ensure internal parameters are properly excluded from preview URLs while retained in manager URLs.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…set as manager query params

nx-cloud · 2026-01-14T13:20:21Z

View your CI Pipeline Execution ↗ for commit e2263b8

Command	Status	Duration	Result
`nx run-many -t compile,check,knip,test,pretty-d...`	✅ Succeeded	9m 47s	View ↗

☁️ Nx Cloud last updated this comment at 2026-01-14 13:31:40 UTC

coderabbitai · 2026-01-14T13:20:51Z

📝 Walkthrough

Walkthrough

Modified URL parameter serialization in the manager API to use separate parameter sets for manager and preview URLs. Parameters id and viewMode are now excluded from preview URL parameters while remaining in manager URL parameters.

Changes

Cohort / File(s)	Summary
URL parameter handling `code/core/src/manager-api/modules/url.ts`	Separated `customParams` into `customManagerParams` and `customPreviewParams`. Added `omit` import from es-toolkit to exclude `id` and `viewMode` from preview parameters. Updated href construction to use appropriate parameter sets.
Parameter serialization tests `code/core/src/manager-api/tests/url.test.js`	Extended test to verify `id` and `viewMode` appear in `managerHref` but are excluded from `previewHref`. Assertions confirm parameter-specific behavior is working as intended.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing touches

📝 Generate docstrings

🧹 Recent nitpick comments

code/core/src/manager-api/tests/url.test.js (1)
366-382: Test coverage for the new exclusion behavior is good.

The assertions correctly verify that id and viewMode are included in managerHref but excluded from previewHref.

Consider splitting this into a dedicated test case for clarity, as the test name "supports additional query params, including nested objects" doesn't indicate it also validates system parameter exclusion. A separate test like 'excludes id and viewMode from preview URL' would improve discoverability and maintainability.
💡 Suggested dedicated test case
it('excludes id and viewMode from preview URL', () => {
  const { api, state } = initURL({
    store,
    provider: { channel: new EventEmitter() },
    state: { location: { pathname: '/', search: '' } },
    navigate: vi.fn(),
    fullAPI: { getCurrentStoryData: () => ({ id: 'test--story' }) },
  });
  store.setState(state);

  const { managerHref, previewHref } = api.getStoryHrefs('test--story', {
    queryParams: { id: 'external-id', viewMode: 'external-viewMode' },
  });
  
  expect(managerHref).toContain('id=external-id');
  expect(managerHref).toContain('viewMode=external-viewMode');
  expect(previewHref).not.toContain('id=external-id');
  expect(previewHref).not.toContain('viewMode=external-viewMode');
});

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce33702 and e2263b8.

📒 Files selected for processing (2)

code/core/src/manager-api/modules/url.ts
code/core/src/manager-api/tests/url.test.js

🧰 Additional context used

📓 Path-based instructions (7)

**/*.{js,jsx,ts,tsx,json,md,html,css,scss}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Format code using Prettier with yarn prettier --write <file>

Files:

code/core/src/manager-api/modules/url.ts
code/core/src/manager-api/tests/url.test.js

**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Run ESLint checks using yarn lint:js:cmd <file> or the full command cross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directives to fix linting errors before committing

Files:

code/core/src/manager-api/modules/url.ts
code/core/src/manager-api/tests/url.test.js

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode across all packages

Files:

code/core/src/manager-api/modules/url.ts

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
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/manager-api/modules/url.ts
code/core/src/manager-api/tests/url.test.js

code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use logger from storybook/internal/node-logger for server-side logging in Node.js code

Files:

code/core/src/manager-api/modules/url.ts
code/core/src/manager-api/tests/url.test.js

**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access the mocked functions in Vitest tests
Implement mock behaviors in beforeEach blocks 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 without vi.mocked() in Vitest tests
Avoid mock implementations outside of beforeEach blocks in Vitest tests
Avoid mocking without the spy: true option 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 with vi.mocked() in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests

Files:

code/core/src/manager-api/tests/url.test.js

**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Use vi.mock() to mock file system, loggers, and other external dependencies in tests

Files:

code/core/src/manager-api/tests/url.test.js

🧠 Learnings (4)

📚 Learning: 2025-09-18T20:51:06.618Z

Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.

Applied to files:

code/core/src/manager-api/modules/url.ts
code/core/src/manager-api/tests/url.test.js

📚 Learning: 2025-09-18T20:51:06.618Z

Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.

Applied to files:

code/core/src/manager-api/modules/url.ts
code/core/src/manager-api/tests/url.test.js

📚 Learning: 2025-09-24T09:39:39.233Z

Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.

Applied to files:

code/core/src/manager-api/modules/url.ts
code/core/src/manager-api/tests/url.test.js

📚 Learning: 2025-12-22T22:03:40.123Z

Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Export functions from modules if they need to be tested

Applied to files:

code/core/src/manager-api/modules/url.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). (7)

GitHub Check: get-parameters
GitHub Check: get-branch
GitHub Check: Danger JS
GitHub Check: nx
GitHub Check: nx
GitHub Check: nx
GitHub Check: Core Unit Tests, windows-latest

🔇 Additional comments (2)

code/core/src/manager-api/modules/url.ts (2)

15-15: LGTM!

Good choice using omit from es-toolkit/object for cleanly filtering out system parameters. This is a minimal and appropriate dependency addition.

265-282: Clean separation of manager and preview query parameters.

The approach correctly prevents id and viewMode from propagating to the preview iframe URL while still allowing them in the manager URL. This directly addresses the VSCode Simple Browser issue.

One consideration: if additional system parameters need protection in the future, consider extracting the excluded keys (['id', 'viewMode']) into a named constant for maintainability.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

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

storybook-app-bot · 2026-01-14T13:31:26Z

Package Benchmarks

^{Commit: e2263b8, ran on 14 January 2026 at 13:31:22 UTC}

The following packages have significant changes to their size or dependencies:

`storybook`

	Before	After	Difference
Dependency count	49	49	0
Self size	20.30 MB	20.22 MB	🎉 -80 KB 🎉
Dependency size	16.52 MB	16.52 MB	0 B
Bundle Size Analyzer	Link	Link

`@storybook/angular`

	Before	After	Difference
Dependency count	192	192	0
Self size	139 KB	118 KB	🎉 -20 KB 🎉
Dependency size	30.43 MB	30.43 MB	🎉 -54 B 🎉
Bundle Size Analyzer	Link	Link

`@storybook/nextjs`

	Before	After	Difference
Dependency count	538	538	0
Self size	646 KB	646 KB	🚨 +120 B 🚨
Dependency size	59.22 MB	59.21 MB	🎉 -14 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/nextjs-vite`

	Before	After	Difference
Dependency count	127	127	0
Self size	1.12 MB	1.12 MB	🚨 +36 B 🚨
Dependency size	21.82 MB	21.81 MB	🎉 -14 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/react-native-web-vite`

	Before	After	Difference
Dependency count	159	159	0
Self size	30 KB	30 KB	0 B
Dependency size	23.00 MB	22.99 MB	🎉 -14 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/react-vite`

	Before	After	Difference
Dependency count	117	117	0
Self size	35 KB	35 KB	0 B
Dependency size	19.62 MB	19.60 MB	🎉 -14 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/react-webpack5`

	Before	After	Difference
Dependency count	278	278	0
Self size	24 KB	24 KB	🎉 -12 B 🎉
Dependency size	44.13 MB	44.12 MB	🎉 -14 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/web-components-vite`

	Before	After	Difference
Dependency count	21	21	0
Self size	19 KB	19 KB	🎉 -174 B 🎉
Dependency size	2.21 MB	2.19 MB	🎉 -20 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/cli`

	Before	After	Difference
Dependency count	183	183	0
Self size	775 KB	775 KB	🎉 -193 B 🎉
Dependency size	67.38 MB	67.30 MB	🎉 -80 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/codemod`

	Before	After	Difference
Dependency count	176	176	0
Self size	30 KB	30 KB	0 B
Dependency size	65.95 MB	65.87 MB	🎉 -80 KB 🎉
Bundle Size Analyzer	Link	Link

`create-storybook`

	Before	After	Difference
Dependency count	50	50	0
Self size	999 KB	999 KB	🎉 -137 B 🎉
Dependency size	36.82 MB	36.74 MB	🎉 -80 KB 🎉
Bundle Size Analyzer	node	node

`@storybook/react`

	Before	After	Difference
Dependency count	57	57	0
Self size	732 KB	718 KB	🎉 -14 KB 🎉
Dependency size	12.94 MB	12.94 MB	0 B
Bundle Size Analyzer	Link	Link

`@storybook/vue3`

	Before	After	Difference
Dependency count	3	3	0
Self size	63 KB	55 KB	🎉 -8 KB 🎉
Dependency size	211 KB	211 KB	0 B
Bundle Size Analyzer	Link	Link

`@storybook/web-components`

	Before	After	Difference
Dependency count	3	3	0
Self size	61 KB	41 KB	🎉 -20 KB 🎉
Dependency size	47 KB	47 KB	0 B
Bundle Size Analyzer	Link	Link

Sidnioulz

Hurray for yet another browser environment to support 😜

Code LGTM. No errors seen in VSCode Simple browser.

fix system query params (id, viewMode) being overridable if they are …

e2263b8

…set as manager query params

JReinhold self-assigned this Jan 14, 2026

JReinhold added bug ui ci:normal labels Jan 14, 2026

JReinhold requested review from Sidnioulz and ndelangen January 14, 2026 13:36

Sidnioulz approved these changes Jan 14, 2026

View reviewed changes

JReinhold merged commit 217a132 into next Jan 14, 2026
78 of 81 checks passed

JReinhold deleted the jeppe/fix-query-params-override branch January 14, 2026 15:31

This was referenced Jan 14, 2026

Release: Prerelease beta 10.2.0-beta.0 #33539

Merged

Release: Prerelease 10.2.0-alpha.19 #33551

Closed

square-pixel mentioned this pull request Apr 22, 2026

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

Open

coderabbitai Bot mentioned this pull request Apr 23, 2026

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

Open

coderabbitai Bot mentioned this pull request May 2, 2026

fix: correctly merge query params when preview-url contains query string #34685

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Manager: Fix system query parameters being overridable#33535

Manager: Fix system query parameters being overridable#33535
JReinhold merged 1 commit into
nextfrom
jeppe/fix-query-params-override

JReinhold commented Jan 14, 2026 •

edited

Loading

Uh oh!

nx-cloud Bot commented Jan 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jan 14, 2026

Walkthrough

Changes

Estimated code review effort

Uh oh!

storybook-app-bot Bot commented Jan 14, 2026

Uh oh!

Sidnioulz left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

JReinhold commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Summary by CodeRabbit

Uh oh!

nx-cloud Bot commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jan 14, 2026

Walkthrough

Changes

Estimated code review effort

Uh oh!

storybook-app-bot Bot commented Jan 14, 2026

Package Benchmarks

storybook

@storybook/angular

@storybook/nextjs

@storybook/nextjs-vite

@storybook/react-native-web-vite

@storybook/react-vite

@storybook/react-webpack5

@storybook/web-components-vite

@storybook/cli

@storybook/codemod

create-storybook

@storybook/react

@storybook/vue3

@storybook/web-components

Uh oh!

Sidnioulz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JReinhold commented Jan 14, 2026 •

edited

Loading

nx-cloud Bot commented Jan 14, 2026 •

edited

Loading

`storybook`

`@storybook/angular`

`@storybook/nextjs`

`@storybook/nextjs-vite`

`@storybook/react-native-web-vite`

`@storybook/react-vite`

`@storybook/react-webpack5`

`@storybook/web-components-vite`

`@storybook/cli`

`@storybook/codemod`

`create-storybook`

`@storybook/react`

`@storybook/vue3`

`@storybook/web-components`