feat(): Add opt-in raw search input for validateSearch#6946
Open
sandy2008 wants to merge 4 commits intoTanStack:mainfrom
Open
feat(): Add opt-in raw search input for validateSearch#6946sandy2008 wants to merge 4 commits intoTanStack:mainfrom
sandy2008 wants to merge 4 commits intoTanStack:mainfrom
Conversation
…d update related documentation
|
github-actions
bot
added
documentation
Contributor
📝 WalkthroughWalkthroughPreserves raw URL search strings on locations, adds a validator marker and wrapper ( Changes
Sequence Diagram(s)sequenceDiagram
participant URL as "URL / Raw Search"
participant Router as "Router"
participant QS as "qss.decode(parser)"
participant Validator as "Search Validator"
participant Route as "Route Handler"
URL->>Router: incoming request with raw query string (?page=0&folder=123)
Router->>QS: decode(rawSearch, parser?)
QS-->>Router: parsedSearch (e.g., {page: 0, folder: "123"})
Router->>Router: withRawSearch(location, rawSearch) (attach non-enumerable _rawSearch)
Router->>Validator: validatorUsesRawSearchInput? (check marker)
alt Validator requires raw input
Router->>Validator: provide merged raw+parsed input (raw strings where applicable)
else Validator uses parsed only
Router->>Validator: provide parsedSearch
end
Validator-->>Router: validation result
Router->>Route: deliver validated params to route handler
Estimated Code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can customize the tone of the review comments and chat replies.Configure the |
This was referenced Mar 17, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/router-core/src/qss.ts (1)
64-67: TightendecodeAPI types instead ofany.Line 65 and Line 67 use
any, which weakens strict-mode guarantees at the public boundary. Prefer typedURLSearchParamsinit input and a typed object return.Type-safe signature diff
export function decode( - str: any, + str: ConstructorParameters<typeof URLSearchParams>[0], parser: (value: string) => unknown = toValue, -): any { +): Record<string, unknown> {As per coding guidelines,
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router-core/src/qss.ts` around lines 64 - 67, The public decode function currently uses `any` for its `str` parameter and return type; tighten the API by changing `str` to `URLSearchParamsInit` (the allowed inputs for URLSearchParams) and give the function a concrete return type such as `Record<string, unknown>` (or `Record<string, string | string[]>` if you want to preserve multi-value semantics). Keep the `parser` parameter as `(value: string) => unknown` (or narrow it to a generic if desired), update the `decode` signature accordingly, and ensure any internal usage of `decode` and the `toValue` parser matches the new types so callers and strict-mode type checks are satisfied.packages/router-core/src/searchValidator.ts (1)
36-39: Consider adding a brief comment explaining theas nevercast.The cast
validator(input as never)is necessary because TypeScript cannot narrowvalidatortoValidatorFnin the else branch, but a brief inline comment would help future maintainers understand why this cast is safe.📝 Suggested enhancement
+ // Fallback: wrap function validators + // The `as never` cast is safe because we've ruled out object validators above const wrapped = ((input: unknown) => validator(input as never)) as TValidator & RawSearchInputMarked wrapped[rawSearchInputMarker] = true return wrapped🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router-core/src/searchValidator.ts` around lines 36 - 39, Add a brief inline comment above the `validator(input as never)` cast in the `wrapped` creation to explain why `as never` is used and why it is safe: note that TypeScript cannot narrow `validator` to `ValidatorFn` in this branch, but we know at runtime the `validator` accepts the input shape so forcing the cast is safe; reference the `wrapped` variable, `validator` function, and `rawSearchInputMarker` assignment so maintainers can find the context quickly.packages/zod-adapter/tests/index.test.tsx (1)
4-12: Fix import sorting.ESLint reports that imports should be sorted alphabetically.
Linkshould come beforevalidateSearchWithRawInput.🔧 Suggested fix
import { createMemoryHistory, createRootRoute, createRoute, createRouter, Link, RouterProvider, validateSearchWithRawInput, } from '@tanstack/react-router'Note: The current order appears correct (
Linkat line 9 comes beforevalidateSearchWithRawInputat line 11). The ESLint error may be a false positive or related to the position ofcreateMemoryHistorywhich was added. Please verify the actual import order in your editor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/zod-adapter/tests/index.test.tsx` around lines 4 - 12, The import list is failing ESLint's alphabetical order rule; reorder the named imports so they are alphabetized (e.g., createMemoryHistory, createRootRoute, createRoute, createRouter, Link should come before RouterProvider and validateSearchWithRawInput, with validateSearchWithRawInput after Link), update the import statement that contains createMemoryHistory/createRootRoute/createRoute/createRouter/Link/RouterProvider/validateSearchWithRawInput accordingly, and re-run the linter or run eslint --fix to confirm the sorting issue is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/router-core/src/router.ts`:
- Around line 1502-1516: The lightweight matching path (matchRoutesLightweight)
and buildLocation are still using coerced location.search rather than the
raw-input selection used in the full match, so routes with validateSearch +
strict can lose raw string values; refactor the raw-input selection into a
shared helper (e.g., reuse the logic from validatorUsesRawSearchInput and the
rawLocationSearch/parentStrictSearch merge) and call that helper from both the
full-match branch (where validateSearch(...) uses searchValidationInput) and
from matchRoutesLightweight/buildLocation so validateSearch always receives the
same raw vs coerced input selection.
---
Nitpick comments:
In `@packages/router-core/src/qss.ts`:
- Around line 64-67: The public decode function currently uses `any` for its
`str` parameter and return type; tighten the API by changing `str` to
`URLSearchParamsInit` (the allowed inputs for URLSearchParams) and give the
function a concrete return type such as `Record<string, unknown>` (or
`Record<string, string | string[]>` if you want to preserve multi-value
semantics). Keep the `parser` parameter as `(value: string) => unknown` (or
narrow it to a generic if desired), update the `decode` signature accordingly,
and ensure any internal usage of `decode` and the `toValue` parser matches the
new types so callers and strict-mode type checks are satisfied.
In `@packages/router-core/src/searchValidator.ts`:
- Around line 36-39: Add a brief inline comment above the `validator(input as
never)` cast in the `wrapped` creation to explain why `as never` is used and why
it is safe: note that TypeScript cannot narrow `validator` to `ValidatorFn` in
this branch, but we know at runtime the `validator` accepts the input shape so
forcing the cast is safe; reference the `wrapped` variable, `validator`
function, and `rawSearchInputMarker` assignment so maintainers can find the
context quickly.
In `@packages/zod-adapter/tests/index.test.tsx`:
- Around line 4-12: The import list is failing ESLint's alphabetical order rule;
reorder the named imports so they are alphabetized (e.g., createMemoryHistory,
createRootRoute, createRoute, createRouter, Link should come before
RouterProvider and validateSearchWithRawInput, with validateSearchWithRawInput
after Link), update the import statement that contains
createMemoryHistory/createRootRoute/createRoute/createRouter/Link/RouterProvider/validateSearchWithRawInput
accordingly, and re-run the linter or run eslint --fix to confirm the sorting
issue is resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad2f3b39-b2b5-400d-b1b1-6f6c94b30c32
📒 Files selected for processing (10)
docs/router/api/router/RouteOptionsType.mddocs/router/guide/search-params.mdpackages/react-router/src/index.tsxpackages/router-core/src/index.tspackages/router-core/src/qss.tspackages/router-core/src/router.tspackages/router-core/src/searchValidator.tspackages/solid-router/src/index.tsxpackages/vue-router/src/index.tsxpackages/zod-adapter/tests/index.test.tsx
…s with search=true
Contributor
There was a problem hiding this comment.
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 `@packages/zod-adapter/tests/index.test.tsx`:
- Around line 4-12: The import members in the declaration containing
createMemoryHistory, createRootRoute, createRoute, createRouter, Link,
RouterProvider, and validateSearchWithRawInput are not alphabetically ordered
and fail the ESLint sort-imports rule; reorder the named imports in that single
import statement to alphabetical order (e.g., createMemoryHistory, createRoute,
createRootRoute, createRouter, Link, RouterProvider, validateSearchWithRawInput)
so the import line with those symbols satisfies the linter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d421b1c5-0fa1-4660-9655-0a1f95079435
📒 Files selected for processing (2)
packages/router-core/src/router.tspackages/zod-adapter/tests/index.test.tsx
sandy2008
changed the title
Contributor
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #537 and #6044.
Today,
validateSearchreceives search params after the router’s default parsing/coercion step. That means numeric-looking values like?folder=34324324235325352523can be converted before validation, which breaks string schemas and can lose precision for large integer-like values.This PR adds a non-breaking, opt-in way for validators to receive raw URL search values instead:
validateSearchWithRawInput(...).What changed
validateSearchWithRawInput(...)inrouter-corevalidateSearchinput and document the new opt-in helperWhy this approach
Changing the default parser behavior would be breaking because existing apps rely on values like
?page=1or?enabled=truebeing coerced before validation.This approach fixes the reported bug without changing current behavior for existing validators. Users only opt into raw string input when they need it.
Example
Testing
Ran:
CI=1 NX_DAEMON=false pnpm nx run-many --target=test:unit --projects=@tanstack/router-core,@tanstack/react-router,@tanstack/zod-adapter --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run-many --target=test:types --projects=@tanstack/router-core,@tanstack/react-router,@tanstack/solid-router,@tanstack/vue-router,@tanstack/zod-adapter --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run-many --target=test:eslint --projects=@tanstack/router-core,@tanstack/react-router,@tanstack/solid-router,@tanstack/vue-router,@tanstack/zod-adapter --outputStyle=stream --skipRemoteCacheSummary by CodeRabbit
New Features
Documentation
Tests