fix: type error when passing number to Link.params
#5560
Conversation
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
Contributor
WalkthroughThe TypeScript type signature for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5–10 minutes The change is a straightforward TypeScript type union update affecting a single file's public API. The modification is localized to one type signature with clear intent and minimal surface area, though it does touch a public interface and should be validated against the documented behavior. Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 6m 5s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 1m 24s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-21 02:15:04 UTC
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-router-ssr-query
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
scarf005
force-pushed
the
fix/5559/link-params-number-type-error
branch
from
997874b to
a05b6f2
Compare
for some reason the type error isn't reproducible in `TanStack/router` repo. to check that the changes work: 1. create an external repo (e.g vite tanstack router template) 2. build packages with the changes 3. locally install router packages
scarf005
force-pushed
the
fix/5559/link-params-number-type-error
branch
from
a05b6f2 to
e530391
Compare
![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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-core/src/link.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-core/src/link.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/link.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). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (1)
packages/router-core/src/link.ts (1)
577-586: Verify the fix in an external project setup.Since the issue is not reproducible within the TanStack/router repository but only in external projects, please verify that this change actually resolves the type error in a real-world setup.
As suggested in the PR description, please:
- Build the packages with these changes
- Create a minimal reproduction project (e.g., using Vite TanStack Router template)
- Install the locally built packages
- Verify that
<Link to="/$foo/bar" params={{ foo: 3 }}>no longer produces a type errorThis verification is critical since the issue manifests differently in external projects versus the monorepo.
Comment on lines
+577
to
+586
| params: MakeRequiredParamsReducer< | ||
| TRouter, | ||
| 'PATH', | ||
| TFrom, | ||
| TTo | ||
| > extends infer Params | ||
| ? { | ||
| [K in keyof Params]: Params[K] | number | ||
| } & {} | ||
| : never |
Contributor
There was a problem hiding this comment.
Apply the same number allowance to optional path params.
The change allows numbers for required path params, but MakeOptionalPathParams (lines 547-553) doesn't have the same allowance. If numbers are auto-stringified at runtime per the documentation, this behavior should be consistent for both required and optional path parameters.
After fixing the type logic issue above, apply similar logic to MakeOptionalPathParams:
export interface MakeOptionalPathParams<
in out TRouter extends AnyRouter,
in out TFrom,
in out TTo,
> {
params?: true | (ParamsReducer<TRouter, 'PATH', TFrom, TTo> extends infer Params
? Params extends true
? true
: Params extends (...args: any[]) => any
? Params
: {
[K in keyof Params]: Params[K] | number
} & {}
: never)
}🤖 Prompt for AI Agents
In packages/router-core/src/link.ts around lines 547-553 and 577-586,
MakeOptionalPathParams currently doesn't allow numeric path param values while
MakeRequiredParamsReducer does; update MakeOptionalPathParams so its mapped type
also widens each param type to Params[K] | number (i.e., when resolving Params
into an object map, transform each key's type to allow number in addition to the
original type) and keep the existing true/functional branches intact so optional
params behave consistently with required params.
Fix type logic to handle true and function cases.
The mapped type { [K in keyof Params]: Params[K] | number } assumes Params is an object with parameter keys, but MakeRequiredParamsReducer can also resolve to:
true(when params don't need to change between routes)- A function of type
ParamsReducerFn
When Params is true, keyof true evaluates to never, causing the mapped type to become {} and lose the intended true semantic. When Params is a function, keyof Function includes properties like "length", "name", "call", etc., creating an incorrect type.
Consider this approach that explicitly handles all three cases:
- params: MakeRequiredParamsReducer<
- TRouter,
- 'PATH',
- TFrom,
- TTo
- > extends infer Params
- ? {
- [K in keyof Params]: Params[K] | number
- } & {}
- : never
+ params: MakeRequiredParamsReducer<TRouter, 'PATH', TFrom, TTo> extends infer Params
+ ? Params extends true
+ ? true
+ : Params extends (...args: any[]) => any
+ ? Params
+ : {
+ [K in keyof Params]: Params[K] | number
+ } & {}
+ : never📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| params: MakeRequiredParamsReducer< | |
| TRouter, | |
| 'PATH', | |
| TFrom, | |
| TTo | |
| > extends infer Params | |
| ? { | |
| [K in keyof Params]: Params[K] | number | |
| } & {} | |
| : never | |
| params: MakeRequiredParamsReducer<TRouter, 'PATH', TFrom, TTo> extends infer Params | |
| ? Params extends true | |
| ? true | |
| : Params extends (...args: any[]) => any | |
| ? Params | |
| : { | |
| [K in keyof Params]: Params[K] | number | |
| } & {} | |
| : never |
🤖 Prompt for AI Agents
In packages/router-core/src/link.ts around lines 577 to 586, the current mapped
type assumes Params is an object and uses {[K in keyof Params]: Params[K] |
number} which breaks when MakeRequiredParamsReducer resolves to true (loses the
true semantic) or to a function (pulls in function keys); change the type to a
conditional that explicitly handles all three cases: if Params extends true then
keep true, else if Params extends the ParamsReducerFn type then keep Params (or
Params | number if numbers are valid for reducer functions per usage), else
treat Params as an object and map its keys to Params[K] | number; remove the
fallback to an empty intersection (& {}) and ensure the function case is
excluded from key remapping by using the conditional branches.
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.
fixes #5559
for some reason the type error isn't reproducible in
TanStack/routerrepo. to check that the changes work:Summary by CodeRabbit
Release Notes