Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
adbjo wants to merge4 commits intoTanStack:main
base:main
Choose a base branch
Loading
fromadbjo:do_not_add_quotes_to_search_params

Conversation

@adbjo
Copy link

@adbjoadbjo commentedDec 1, 2025
edited
Loading

By default, the router parses search params usingJSON.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=123 changes 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=1000000000000001110 becomeshttp://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.

@coderabbitai
Copy link
Contributor

coderabbitaibot commentedDec 1, 2025
edited
Loading

Walkthrough

Parse/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

Cohort / File(s)Summary
Search params logic
packages/router-core/src/searchParams.ts
parseSearchWith now assigns parser results only when the parser returns an object;stringifySearchWith only re-stringifies when the parser returns an object, otherwise original string values are preserved.
Query-string decoding
packages/router-core/src/qss.ts
Removed thetoValue coercion helper;decode now returns raw string values (and arrays of raw strings) instead of coercing to numbers/booleans.
Tests
packages/router-core/tests/searchParams.test.ts,packages/router-core/tests/qss.test.ts
Tests updated to parse-first then stringify for isomorphism checks; expectations adjusted to reflect raw-string decoding and arrays for repeated keys.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • InspectsearchParams.ts parser branches for cases where parsers return primitives vs objects.
  • Verify callers ofqss.decode and any consumers expecting coerced numeric/boolean types.
  • Run unit tests covering repeated keys, empty values, escaped literals, and custom parsers.

Possibly related PRs

Suggested reviewers

  • schiller-manuel
  • SeanCassiere

Poem

"I hopped through queries, kept each string intact,
I nudged only objects where parsers interact.
Numbers no longer sneak into string beds,
Roundtrips hum softly as tests nod their heads.
— 🐇"

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedDocstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Title check✅ PassedThe title accurately describes the main change: preventing parsing and stringifying of search params that are not JSON objects, which addresses the issue of unwanted quoting of numerical/string parameters.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for usingCodeRabbit! 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
Contributor

@coderabbitaicoderabbitaibot 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: 0

🧹 Nitpick comments (1)
packages/router-core/src/searchParams.ts (1)

74-83:Behavior change looks correct; consider tightening parser typing in this branch

Usingconst parsed = parser(val); return stringify(parsed) is the right fix: it makesstringifySearchWith actually round‑trip parseable strings (e.g."123","true","null") according to the providedparser/stringify pair, 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: sinceparser is 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 onparser in the condition, e.g.:

}elseif(parser&&typeofval==='string'){try{constparsed=parser(val)returnstringify(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

📥 Commits

Reviewing files that changed from the base of the PR and betweenafb4b24 and1af0d4b.

📒 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: nlynzaadRepo: TanStack/router PR: 5182File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5Timestamp: 2025-09-22T00:56:49.237ZLearning: 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.

@adbjoadbjo changed the titlefix: Do not add quotes to search paramsfix: Do not quote numerical search paramsDec 1, 2025
@nx-cloud
Copy link

nx-cloudbot commentedDec 1, 2025
edited
Loading

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable themin workspace settings.


View yourCI Pipeline Execution ↗ for commit1af0d4b

CommandStatusDurationResult
nx affected --targets=test:eslint,test:unit,tes...❌ Failed11m 9sView ↗
nx run-many --target=build --exclude=examples/*...✅ Succeeded1m 20sView ↗

☁️Nx Cloud last updated this comment at2025-12-01 19:37:23 UTC

@pkg-pr-new
Copy link

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@6000

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@6000

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@6000

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@6000

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@6000

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@6000

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@6000

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@6000

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@6000

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@6000

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@6000

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@6000

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@6000

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@6000

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@6000

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@6000

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@6000

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@6000

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@6000

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@6000

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@6000

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@6000

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@6000

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-ssr-query@6000

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@6000

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@6000

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@6000

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@6000

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@6000

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@6000

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@6000

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@6000

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@6000

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@6000

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@6000

commit:1af0d4b

@adbjoadbjoforce-pushed thedo_not_add_quotes_to_search_params branch from1af0d4b toc8ae18eCompareDecember 2, 2025 11:05
@adbjoadbjoforce-pushed thedo_not_add_quotes_to_search_params branch fromc8ae18e to04bd7adCompareDecember 2, 2025 11:09
@adbjoadbjo changed the titlefix: Do not quote numerical search paramsfix: Do coerce numerical search paramsDec 9, 2025
@adbjo
Copy link
Author

Fixes:#6044

@adbjoadbjo changed the titlefix: Do coerce numerical search paramsfix: Do not coerce numerical search paramsDec 9, 2025
@adbjoadbjo changed the titlefix: Do not coerce numerical search paramsfix: Do not parse / stringify search params that are not objectsDec 9, 2025
@adbjoadbjo changed the titlefix: Do not parse / stringify search params that are not objectsfix: Do not parse / stringify search params that are not json objectsDec 9, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@coderabbitaicoderabbitai[bot]coderabbitai[bot] left review comments

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@adbjo

[8]ページ先頭

©2009-2025 Movatter.jp