perf: improve typescript performance of relative pathing and larger route trees

[chorobin]

Performance improvements

• Use template literal types to autocomplete relative paths. This eliminates recursion depth limits and is faster than splitting the union into tuples
• Only remove trailing and leading slashes instead of cleaning the whole path. I'm hoping that this is OK as its more performant
• Reduce instantiations by using a more concise way of writing ParseRoutes
• navigate now has the same performance improvements as useNavigate
• Make TPath, TContext and TAllContext invariant as this reduced extra checking

Fixes

• Resolving the full path for relative paths wasn't always including '/' at the beginning
• Checking the resolved path wasn't working due to the resolved path including '/' at the end
• The documentation suggests that the default for TFrom should be '/'. This makes the logic simpler but I think this is correct but please let me know if I should revert it
• TTo would not autocomplete in useNavigate. Seems to be related to a typescript issue. It cannot be defaulted for now

[nx-cloud[bot]]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit c5b7d82ddc85f4b035c7bd15758185c848d995d3. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

---

✅ Successfully ran 1 target

• nx affected --targets=test:format,test:lib,test:types,test:build,build --parallel=3

---

Sent with 💌 from NxCloud.

[schiller-manuel]

The documentation suggests that the default for TFrom should be '/'. This makes the logic simpler but I think this is correct but please let me know if I should revert it

Originally it was, but now TFrom should default to the union of all routes.

Here, prevParams are now {} (the params of /), but it should be the union of all params:

[chorobin]

The documentation suggests that the default for TFrom should be '/'. This makes the logic simpler but I think this is correct but please let me know if I should revert it

Originally it was, but now TFrom should default to the union of all routes.

Here, prevParams are now {} (the params of /), but it should be the union of all params:

Ah. This is confusing. So, should '/' behave like a union of all params if you're coming from the root? Or is there like a special case here for effectively from not specified then we don't know where you're coming from and therefore the previous params is a union of everything?

[schiller-manuel]

Or is there like a special case here for effectively from not specified then we don't know where you're coming from and therefore the previous params is a union of everything?

yes, exactly, the "we don't know where you're coming from" perfectly describes it.

While you are at it, can you please update the documentation to reflect this?

[chorobin]

Or is there like a special case here for effectively from not specified then we don't know where you're coming from and therefore the previous params is a union of everything?

yes, exactly, the "we don't know where you're coming from" perfectly describes it.

While you are at it, can you please update the documentation to reflect this?

I see now. I will check the performance with default of a union of all routes. But in the future would it make sense to use a different default like undefined or string or something to signal that we don't know where you're coming from and to handle this case? Only suggesting because anywhere where you use a conditional type on TFrom will distribute if it's defaulted as a union

[schiller-manuel]

we can definitely use anything else to signal the "we don't know where you are coming from" if the search/path params union behavior persists

[chorobin]

Nice. Thanks for clarifying because now I understand the reasoning behind the original default . A union makes sense in the way that you could have come from anywhere but I think the reality is that from wasn't set to anything and therefore the type should probably reflect that. We probably just want what performs correctly and the fastest.

[chorobin]

Looks like its working when set to string so maybe that was the reason it was defaulted like this in other places. I think I concluded string makes the most sense anyway.

[schiller-manuel]

ready to review?

[chorobin]

I think you can yes

[schiller-manuel]

I noticed the following problem with your PR:

When an index route has search params defined, navigate will not complain about them missing. Try running the following example on top of your PR and see that the error does not occur:

https://stackblitz.com/edit/tanstack-router-tvs9ez?file=src%2Fmain.tsx%3AL132

[chorobin]

I noticed the following problem with your PR:

When an index route has search params defined, navigate will not complain about them missing. Try running the following example on top of your PR and see that the error does not occur:

https://stackblitz.com/edit/tanstack-router-tvs9ez?file=src%2Fmain.tsx%3AL132

Yep. Think it should be fixed now?

[schiller-manuel]

I also noticed that there is a regression that broke the following functionality (probably one of my last PRs broke this...):

If an index route has search params, then navigating to the "non-index" path should use the index route's search params.

In this example, navigating to '/posts' should error if the search params of `/posts/' are missing

https://stackblitz.com/edit/tanstack-router-tvs9ez?file=src%2Fmain.tsx

 navigate(
    {
      to: '/posts',
      // <<< should error here about missing search param
    })

This part checks if an index route exists:

 TToIndex = TTo extends ''
    ? ''
    : RouteByPath<TRouteTree, `${TTo}/`> extends never
      ? TTo
      : `${TTo}/`,

In the following however, TResolved is used instead of TToIndex if it is is not never:

[TResolved] extends [never]
      ? PostProcessParams<
          RouteByPath<TRouteTree, TToIndex>['types'][TToRouteType],
          TParamVariant
        >
      : PostProcessParams<
          RouteByPath<TRouteTree, TResolved>['types'][TToRouteType],
          TParamVariant
        >,

I am not sure when TResolved can "legally" be never though, any idea?

[chorobin]

I will take a look tomorrow now. I've noticed bugs around the index routes with CheckPath as well. Sounds similar. Basically a route without an index route flips to never and kills autocomplete with ./ and ../. Still thinking what the correct fix is for this though

[chorobin]

Looking at the docs. When you have an index route a path should match the index route regardless? Meaning /something/ or /something actually both match the index route if it exists? Trying clarify the behaviour as obviously the types should reflect this as well

[schiller-manuel]

If you navigate to /something and have an index route setup at /something/, router will match the index route. That's why I added this check if the index route exists, and if it does, it shall use the search params of that index route.

[chorobin]

I think I have fixed the issue. Tried to simplify it a bit

[schiller-manuel]

amazing!

[chorobin]

Glad you think so. The type checking bottleneck is now in creating each route as far as I can see. Especially file based routing. You can shave off a bit of time here. But it gets harder and harder to find performance gains 😅

[schiller-manuel]

Looking forward to more of your 🪒 shaving 👍 Why is file based routing worse than code based?

[chorobin]

Looking forward to more of your 🪒 shaving 👍 Why is file based routing worse than code based?

I've looked a bit into it. My guess would be that file routes are checking an id against a large mapping object in the route tree gen. They are also probably doing more work because it then has to work backwards to create the path and full path. Code based routes you just define the path and it doesn't need to be type checked

[schiller-manuel]

can we generate more so TS has less work to do?

[chorobin]

Was exactly my thinking but we'd need to play with this idea. Also you don't want to necessarily undo the benefits you get from inference with code gen. But maybe generating the full path would help as you need to re-build when you change file path anyway. It needs a think but it might not be a breaking change?

[chorobin]

Out of interest. Why can't this be done the other way around? Maybe I'm missing something but why can't you just generate a dummy route with only the path, id and parent in the route gen and then the user imports it in their file and update it with the user stuff (search, loader etc)? Probably because then the user also has to create the route tree themselves? But you then won't need the mapping object and should be the same performance as code based?