perf: improve performance of Link and routes #1453

chorobin · 2024-04-09T22:00:12Z

TS Performance Improvments

Purpose

Type checking large code bases with many Route's and Link's can get quite sluggish. This PR introduces a number of performance optimizations I've discovered while profiling on the large file based example. The purpose of this PR is to improve suggestion times for such code bases and build times

Extended Diagnostics

On main the following diagnostics are present for the example large file based

> tsc --extendedDiagnostics

Files:                         589
Lines of Library:            38118
Lines of Definitions:        77132
Lines of TypeScript:         12777
Lines of JavaScript:             0
Lines of JSON:                   0
Lines of Other:                  0
Identifiers:                113478
Symbols:                    351283
Types:                       62925
Instantiations:            1541766
Memory used:               311542K
Assignability cache size:   113357
Identity cache size:          3707
Subtype cache size:              0
Strict subtype cache size:  162006
I/O Read time:               0.08s
Parse time:                  0.55s
ResolveModule time:          0.11s
ResolveTypeReference time:   0.01s
ResolveLibrary time:         0.02s
Program time:                0.84s
Bind time:                   0.24s
Check time:                  5.64s
printTime time:              0.00s
Emit time:                   0.00s
Total time:                  6.72s

After the changes introduced in this PR the instantiations and check time are reduced

> tsc --extendedDiagnostics

Files:                         589
Lines of Library:            38118
Lines of Definitions:        77133
Lines of TypeScript:         12777
Lines of JavaScript:             0
Lines of JSON:                   0
Lines of Other:                  0
Identifiers:                113461
Symbols:                    189848
Types:                       50776
Instantiations:             592405
Memory used:               222842K
Assignability cache size:    34415
Identity cache size:          3718
Subtype cache size:              0
Strict subtype cache size:       0
I/O Read time:               0.07s
Parse time:                  0.54s
ResolveModule time:          0.11s
ResolveTypeReference time:   0.01s
ResolveLibrary time:         0.02s
Program time:                0.83s
Bind time:                   0.23s
Check time:                  3.82s
printTime time:              0.00s
Emit time:                   0.00s
Total time:                  4.88s

Profiling

This is the profile for the large file route on main.

Going deep into each wall of the profile. The biggest wall is the route tree definition. There is a number of things to improve here.

Performance of each individual file route types
Variance checks on each Route
Type parameter constraints on each Route
Improving performance of merging properties from each parent route
Only Expand when necessary and not eagerly

And the next significant thing is each Link. Each Link can be improved by the following:

Separating LinkComponentProps to cache props for a component passed to createLink
Use the resolved route for relative paths to narrow down the union of paths relevant for autocomplete
Use CheckPath directly on the to prop to avoid an intersection
Evaluate less of params and search until the user use these props

After the performance improvements the route tree looks more like this

and each Link looks like this

Link may seem like a small improvement for one Link but when you have many in large code bases this quickly accumulates

nx-cloud · 2024-04-09T22:00:40Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f3ba3a1. 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 2 targets

Sent with 💌 from NxCloud.

schiller-manuel · 2024-04-10T16:59:45Z

packages/react-router/src/Matches.tsx

@@ -603,10 +603,10 @@ export function useLoaderData<
    ...opts,
    select: (s) => {
      return typeof opts.select === 'function'
-        ? opts.select(s.loaderData)
+        ? opts.select(s.loaderData as TRouteMatch)


shouldn't this be TRouteMatch['loaderData']?

Yeah. This is weird. I will have to double check why this was complaining

chorobin · 2024-04-10T18:24:53Z

packages/react-router/src/fileRoute.ts


 export type FileRoutePath<
  TParentRoute extends AnyRoute,
  TFilePath extends string,
  TResolvedFilePath = ResolveFilePath<TParentRoute, TFilePath>,
-> = TResolvedFilePath extends `_${infer _}`
+> = TResolvedFilePath extends `_${string}`


This is a simple change but reduces instantiations because the type variable is not used

chorobin · 2024-04-10T18:25:45Z

packages/react-router/src/fileRoute.ts

-      string,
-      any
-    > = TSearchSchemaInput extends SearchSchemaInput
+    TSearchSchemaUsed = TSearchSchemaInput extends SearchSchemaInput


For type parameters which act more like variables, its better not to use generic constraints on them. This is because typescript will do eager checking on the value you assign to it.

should we remove the RouteConstraints type then fully?

I don't want to say constraints are bad because you need them to check generics inferred by the user

But if it's not inferred by the user, I assume it can be trusted

chorobin · 2024-04-10T18:26:14Z

packages/react-router/src/fileRoute.ts

@@ -190,7 +173,6 @@ export class FileRoute<
      ? undefined
      : TLoaderDataReturn,
    TChildren extends RouteConstraints['TChildren'] = unknown,
-    TRouteTree extends RouteConstraints['TRouteTree'] = AnyRoute,


I don't think this was needed

chorobin · 2024-04-10T18:27:56Z

packages/react-router/src/link.tsx

@@ -84,67 +84,78 @@ export type RemoveLeadingSlashes<T> = T extends `/${infer R}`
  ? RemoveLeadingSlashes<R>
  : T

+export type ResolvePaths<TRouteTree extends AnyRoute, TSearchPath> =


For relative routes starting with '../' and '.' we can narrow the union of the paths down to the descendents of the resolved path.

Basically, because we know TFrom is a route, we can actually only get descendants of the resolved route. This can make relative pathing faster because we're not dealing with such a large union

chorobin · 2024-04-10T18:29:25Z

packages/react-router/src/link.tsx

-  TPaths,
-  TSearchedPaths = SearchPaths<TPaths, TSearchPath>,
-> = TSearchedPaths extends string ? `${TTo}/${TSearchedPaths}` : never
+> = `${TTo}/${SearchPaths<TRouteTree, TSearchPath>}`


This was a silly mistake by me before. Template literal types automatically distribute. Its a small optimization but it does help a bit

chorobin · 2024-04-10T18:30:07Z

packages/react-router/src/link.tsx

@@ -189,13 +200,13 @@ export type ToSubOptions<
  // The source route path. This is automatically set when using route-level APIs, but for type-safe relative routing on the router itself, this is required
  from?: RoutePathsAutoComplete<TRouteTree, TFrom>
  // // When using relative route paths, this option forces resolution from the current path, instead of the route API's path or `from` path
-} & CheckPath<TRouteTree, {}, TFrom, TTo> &


Intersections are slow because we have to merge a union with another union for to. Better just to change to prop directly

chorobin · 2024-04-10T18:30:41Z

packages/react-router/src/link.tsx

+  TFrom,
+  TToParams,
+> = TParamVariant extends 'SEARCH'
+  ? Expand<{


Think I need to do a few more Expand

chorobin · 2024-04-10T18:31:44Z

packages/react-router/src/link.tsx

@@ -644,30 +713,32 @@ export type LinkProps<
    | ((state: { isActive: boolean }) => React.ReactNode)
 }

+type LinkComponentProps<TComp> = React.PropsWithoutRef<


This is nice. Basically because TComp doesn't change on every Link we can seperate these props into a seperate type and TS will cache this for all Link's

chorobin · 2024-04-10T18:32:40Z

packages/react-router/src/routeInfo.ts

@@ -14,7 +14,7 @@ export type RoutesById<TRouteTree extends AnyRoute> = {
 }

 export type RouteById<TRouteTree extends AnyRoute, TId> = Extract<
-  Extract<ParseRoute<TRouteTree>, { id: TId }>,
+  RoutesById<TRouteTree>[TId],


Turns out mapped types are faster than Extract on a union ;). Makes sense when you think about it. But it only shows when you start to have larger code bases

chorobin · 2024-04-10T18:33:27Z

packages/react-router/src/router.ts

-  TRouteTree extends AnyRoute = AnyRoute,
-  TDehydrated extends Record<string, any> = Record<string, any>,
-  TSerializedError extends Record<string, any> = Record<string, any>,
+  in out TRouteTree extends AnyRoute = AnyRoute,


We can make these invariant to stop TS from doing variance checks on them. I don't think they're necessary in this case

chorobin · 2024-04-10T18:33:50Z

packages/react-router/src/utils.ts

-// eslint-disable-next-line @typescript-eslint/naming-convention
-export type MakeDifferenceOptional<T, U> = Omit<U, keyof T> &
-  Partial<Pick<U, keyof T & keyof U>> &
-  PickRequired<Omit<U, keyof PickRequired<T>>>


I'm not sure why this existed but it works without it?

I added this here:

92923cc#diff-5fd7a0f05758a78cf8b73c09e4d71d21af63fe3d9b312b0ee75b04041ef078e7R222

do we have a test case that tests if when navigating e.g. from /foo/$bar to /foo/$bar/baz/$id, $bar is optional whereas $id is not?

Yes. I thought this was covered by the code already here.

This is the closest test case but maybe I should write a specific one!

router/packages/react-router/tests/link.test-d.tsx

Line 253 in 10c7175

test('when navigating from a route to a route with the same params', () => {

yeah would be cool to have that

chorobin · 2024-04-10T18:34:19Z

packages/react-router/tests/link.test-d.tsx


-  params.returns.branded.toEqualTypeOf<{ invoiceId: string }>()


We should always Expand instead of relying on branded in tests. It means we have complicated intersections

chorobin · 2024-04-10T18:34:50Z

packages/react-router/tests/route.test-d.tsx

@@ -102,13 +101,12 @@ test('when creating a child route with a loader from the root route with context
  const invoicesRoute = createRoute({
    path: 'invoices',
    getParentRoute: () => rootRoute,
-    loader: async () => [{ id: 'invoice1' }, { id: 'invoice2' }] as const,
+    loader: async (opts) => {
+      expectTypeOf(opts).toMatchTypeOf<{ context: { userId: string } }>()


I decided this approach is better because its testing the inference flow

chorobin · 2024-04-15T14:46:24Z

packages/react-router/src/link.tsx

@@ -183,19 +194,19 @@ export type ToSubOptions<
  TFrom extends RoutePaths<TRouteTree> | string = string,
  TTo extends string = '',
 > = {
-  to?: ToPathOption<TRouteTree, TFrom, TTo>
+  to?: ToPathOption<TRouteTree, TFrom, TTo> & {}


This is a simple way to expand the complex type so it appears nice

chorobin force-pushed the f/perf-link-routes branch 2 times, most recently from 0ac0db2 to 39d4484 Compare April 10, 2024 16:45

schiller-manuel reviewed Apr 10, 2024

View reviewed changes

chorobin force-pushed the f/perf-link-routes branch from 39d4484 to cbc692c Compare April 10, 2024 17:24

chorobin commented Apr 10, 2024

View reviewed changes

perf: improve performance of TS for routes and Link

f3ba3a1

chorobin force-pushed the f/perf-link-routes branch from cbc692c to f3ba3a1 Compare April 15, 2024 14:43

chorobin marked this pull request as ready for review April 15, 2024 14:44

chorobin commented Apr 15, 2024

View reviewed changes

schiller-manuel merged commit 3ece907 into TanStack:main Apr 15, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: improve performance of Link and routes #1453

perf: improve performance of Link and routes #1453

chorobin commented Apr 9, 2024 •

edited

Loading

nx-cloud bot commented Apr 9, 2024 •

edited

Loading

schiller-manuel Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024 •

edited

Loading

schiller-manuel Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024 •

edited

Loading

chorobin Apr 10, 2024

chorobin Apr 10, 2024 •

edited

Loading

chorobin Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024

schiller-manuel Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024

schiller-manuel Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 10, 2024

chorobin Apr 15, 2024


		params.returns.branded.toEqualTypeOf<{ invoiceId: string }>()

perf: improve performance of Link and routes #1453

perf: improve performance of Link and routes #1453

Conversation

chorobin commented Apr 9, 2024 • edited Loading

TS Performance Improvments

Purpose

Extended Diagnostics

Profiling

nx-cloud bot commented Apr 9, 2024 • edited Loading

☁️ Nx Cloud Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chorobin Apr 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chorobin Apr 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chorobin Apr 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chorobin commented Apr 9, 2024 •

edited

Loading

nx-cloud bot commented Apr 9, 2024 •

edited

Loading

chorobin Apr 10, 2024 •

edited

Loading

chorobin Apr 10, 2024 •

edited

Loading

chorobin Apr 10, 2024 •

edited

Loading