-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: Do not parse / stringify search params that are not json objects #6000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughParse/stringify logic updated to avoid coercing or replacing raw string values unless a parser returns an object; qss decoding no longer coerces strings to booleans/numbers; tests updated to validate parse-first then stringify roundtrips. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/router-core/src/searchParams.ts (1)
74-83: Behavior change looks correct; consider tightening parser typing in this branchUsing
const parsed = parser(val); return stringify(parsed)is the right fix: it makesstringifySearchWithactually round‑trip parseable strings (e.g."123","true","null") according to the providedparser/stringifypair, and avoids adding extra quotes for numeric/boolean‑like search params. The silent error handling and overall control flow remain consistent withparseSearchWith, so this should be a safe and targeted behavior change.One minor follow‑up: since
parseris typed as optional, calling it here can trip strict TypeScript checks (Object is possibly 'undefined'). You already guard it viahasParser, but TS doesn’t narrow from that. If you want to keep strict mode fully happy, consider narrowing directly onparserin the condition, e.g.:} else if (parser && typeof val === 'string') { try { const parsed = parser(val) return stringify(parsed) } catch (_err) { // silent } }This preserves runtime behavior while aligning with strict type expectations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-core/src/searchParams.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety throughout the codebase
Files:
packages/router-core/src/searchParams.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 11m 9s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 1m 20s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-12-01 19:37:23 UTC
1af0d4b to
c8ae18e
Compare
c8ae18e to
04bd7ad
Compare
|
Fixes: #6044 |

By default, the router parses search params using
JSON.parse, regardless of whether the param is a JSON object or not. This causes strange behavior, for example, it adds quotes around numerical strings:http://my-app.com?p=123changes tohttp://my-app.com?p="123"It also causes rounding issues when large numbers are converted into integers, for example:
http://my-app.com?p=1000000000000001110becomeshttp://my-app.com?p="1000000000000001200"Which means that resource IDs cannot be used in search params, which I guess is a common use-case.
This PR solves this by not calling the parser on params that are not json objects.