router
router copied to clipboard
params became a required property for string typed paths after v1.0.8
Describe the bug
See this line:
router.matchRoute({ to: '/posts' as string });
I am deliberately casting the path to string to show the issue. Because sometimes the path is a variable and you don't know its exact type. This used to work before v1.0.8.
It works if it is written like:
router.matchRoute({ to: '/posts' as string, params: {} });
The problem is, these two codes do the same thing. If params
accept {}
, it should also accept undefined
.
Your Example Website or App
https://stackblitz.com/edit/tanstack-router-u6suse?file=src%2Fmain.tsx
Steps to Reproduce the Bug or Issue
Visit the stackblitz example above and see the typescript error
Expected behavior
If params
accept {}
, it should also accept undefined
. It should be optional to write {}
Screenshots or Videos
No response
Platform
- Version >= 1.0.8
Additional context
Although I used matchRoute
API above, this is also a problem in other APIs. You can try passing a string typed path to Link
and observe the same issue.
When you pass in a string
instead of a typed string literal, router does not know the route at compile time. Thusparams
will be the union of all possible path params of all routes. In your example, this is:
{} | {} | { postId: string; }
The reasoning behind this is:
You need to find out the route at runtime and then decide which value should be used as params
. At runtime, the route could be postRoute
, and then you would need to supply postId
.
If params
would accept e.g. undefined
, what would happen for postRoute
?
Per your logic, I shouldn't be able to pass {}
, because it would be invalid for postRoute
, but I can.
What I am coming to is, whenever params
accepts {}
, it should also accept undefined
, because they are essentially doing the same thing (replaces nothing).
{} | {} | { postId: string; }
can be assigned {}
, so assigning undefined
should also be possible.
Also, to be exactly sure that all possible paths are satisfied, we should pass an intersection of all possible path params, not an union. So it should be {} & {} & { postId: string; }
. But that would be a DX disaster. We should be pragmatic about it.
Per your logic, I shouldn't be able to pass
{}
, because it would be invalid forpostRoute
, but I can.
No, you need to deliberately choose which of the union members you want to pass in.
Allowing undefined
would mean you can simply ignore the search params for any route...
@KurtGokhan stated that before 1.0.8, this worked, and while that's true, it was working when it shouldn't have. We tightened up the screws on some things that were loose.
The rules around to
are pretty simple:
-
to
must be constrained to astring
-
to
's actual type value can resolve to an individual route id,to: '/posts'
or to multiple unique route ids,to: '/posts' | '/posts/$postId'
-
to
can also resolved to a relative navigation path, which should also end up at a route ID - Whatever routes
to
resolves will be considered for other props, includingparams
Under these circumstances, I don't see a problem since if to
actually does resolve to string
, then you must truly have a circumstance where every single route technically is a possibility, in which case, you would want to at least supply params: true
. I personally feel though that this outcome is usually per lack of an as const
somewhere that would allow your to
value to be more constrained.
Despite it erroneously working a few versions ago (we are still fixing some things), a simple as any
to let the compiler know that you know better can go a long way.
More context on why I don't think we should do anything here:
to: string
is technically an allowed constraint so we can also support relative path traversal, so we can't get away from that, but while it is a constraint, it's not really a supported resolved type for to
, since as @schiller-manuel mentioned, it causes the destination route to be all of the routes, merged together.
If we loosen up the types on the resulting params
to accept nothing when to
resolved to string
, then we're opening up a huge type safety loss for vast majority of people that will inevitably miss an as const
or write some reusable function for navigate that ultimately passes a widened to: string
type and unwittingly sees zero errors.
I get your point. But trying to achieve perfect type safety seems futile to me. People will eventually need escape-hatches and string
seems like the best type for that. This is especially true for some APIs like matchRoute
where you are usually working with a dynamic variable rather than a constant path. (Why would I use matchRoute
when I know exactly which route I am trying to match?)
I am ok with keeping it as i. I agree it is somewhat safer to require the user to explicitly type-out the params.
to: string is technically an allowed constraint so we can also support relative path traversal, so we can't get away from that, but while it is a constraint, it's not really a supported resolved type for to, since as @schiller-manuel mentioned, it causes the destination route to be all of the routes, merged together.
Wouldn't string
be a valid type for path if I define a catch-all route though?