From 4d5896442da940b3db0bbe164a11b791a0b9f727 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sun, 6 Sep 2026 00:33:02 +0200 Subject: [PATCH 1/7] test(router-core): cover navigation promise overhead --- .../tests/navigation-awaitable.bench.ts | 88 +++++++ .../tests/navigation-awaitable.test.ts | 227 ++++++++++++++++++ .../tests/navigation-wait.bench.ts | 37 +++ .../tests/server-awaitable.bench.ts | 71 ++++++ .../tests/server-static-ssr.test.ts | 93 +++++++ 5 files changed, 516 insertions(+) create mode 100644 packages/router-core/tests/navigation-awaitable.bench.ts create mode 100644 packages/router-core/tests/navigation-awaitable.test.ts create mode 100644 packages/router-core/tests/navigation-wait.bench.ts create mode 100644 packages/router-core/tests/server-awaitable.bench.ts create mode 100644 packages/router-core/tests/server-static-ssr.test.ts diff --git a/packages/router-core/tests/navigation-awaitable.bench.ts b/packages/router-core/tests/navigation-awaitable.bench.ts new file mode 100644 index 00000000000..ce58210c3d5 --- /dev/null +++ b/packages/router-core/tests/navigation-awaitable.bench.ts @@ -0,0 +1,88 @@ +import { createHook } from 'node:async_hooks' +import { bench, describe, expect } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute } from '../src' +import { createTestRouter } from './routerTestUtils' +import type { AnyRoute } from '../src' + +// Use the same cases on both implementations. Allocation counts are collected +// outside the timed loop so async-hooks instrumentation cannot bias timings. +for (const mode of ['none', 'sync', 'async', 'mixed', 'chunks'] as const) { + const root = new BaseRootRoute({}) + let chunkCalls = 0 + let parent: AnyRoute = root + for (let index = 0; index < 8; index++) { + const parentRoute = parent + const value = { [`level${index}`]: index } + const child = new BaseRoute({ + getParentRoute: () => parentRoute, + path: `level${index}`, + beforeLoad: + mode === 'none' || mode === 'chunks' + ? undefined + : () => + mode === 'async' || (mode === 'mixed' && index % 4 === 0) + ? Promise.resolve(value) + : value, + component: + mode === 'chunks' + ? (Object.assign(() => null, { + preload: () => { + chunkCalls++ + return Promise.resolve() + }, + }) as any) + : undefined, + }) + parent.addChildren([child]) + parent = child + } + const router = createTestRouter({ + routeTree: root, + history: createMemoryHistory({ + initialEntries: [ + '/level0/level1/level2/level3/level4/level5/level6/level7', + ], + }), + }) + await router.load() + const navigate = () => router.navigate({ to: '.', replace: true }) + await navigate() + expect(router.state.matches).toHaveLength(9) + expect( + router.state.matches.every((match) => match.status === 'success'), + ).toBe(true) + if (mode === 'chunks') { + expect(chunkCalls).toBe(16) + } else if (mode !== 'none') { + expect(router.state.matches[8]!.context).toMatchObject({ + level0: 0, + level7: 7, + }) + } + + let promises = 0 + const hook = createHook({ + init(_id, type) { + if (type === 'PROMISE') { + promises++ + } + }, + }) + hook.enable() + await navigate() + hook.disable() + console.info(`${mode}: ${promises} Promises per navigation`) + + describe(`${mode} beforeLoad`, () => { + bench( + '10 navigations through 8 routes', + async () => { + for (let index = 0; index < 10; index++) { + await navigate() + } + }, + { time: 1500, warmupTime: 300 }, + ) + }) +} diff --git a/packages/router-core/tests/navigation-awaitable.test.ts b/packages/router-core/tests/navigation-awaitable.test.ts new file mode 100644 index 00000000000..d691529de5b --- /dev/null +++ b/packages/router-core/tests/navigation-awaitable.test.ts @@ -0,0 +1,227 @@ +import { runInNewContext } from 'node:vm' +import { describe, expect, test, vi } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute, notFound, redirect } from '../src' +import { createTestRouter, loadServerResponse } from './routerTestUtils' + +describe.each([false, true])('awaitable hooks (server=%s)', (isServer) => { + test.each([ + 'sync', + 'promise', + 'thenable', + 'foreign promise', + 'callable thenable', + ])('inherits the result of a %s beforeLoad', async (mode) => { + const value = { token: 'parent context' } + const root = new BaseRootRoute({ + beforeLoad: () => { + switch (mode) { + case 'promise': + return Promise.resolve(value) + case 'thenable': + return { then: (resolve: any) => resolve(value) } as any + case 'foreign promise': + return runInNewContext('Promise.resolve(value)', { value }) + case 'callable thenable': + return Object.assign(() => {}, { + then: (resolve: any) => resolve(value), + }) as any + default: + return value + } + }, + }) + const loader = vi.fn(({ context }) => context.token) + const child = new BaseRoute({ + getParentRoute: () => root, + path: '/', + loader, + }) + const router = createTestRouter({ + routeTree: root.addChildren([child]), + history: createMemoryHistory({ initialEntries: ['/'] }), + isServer, + }) + if (isServer) { + expect((await loadServerResponse(router, '/')).status).toBe(200) + } else { + await router.load() + } + expect(loader).toHaveBeenCalledOnce() + expect(router.state.matches.at(-1)?.loaderData).toBe(value.token) + }) + + test('normalizes a throwing then getter as a beforeLoad error', async () => { + const error = new Error('cannot read then') + const onError = vi.fn() + const loader = vi.fn() + const root = new BaseRootRoute({ + beforeLoad: () => ({ + get then(): never { + throw error + }, + }), + loader, + onError, + }) + const router = createTestRouter({ + routeTree: root, + history: createMemoryHistory({ initialEntries: ['/'] }), + isServer, + }) + if (isServer) { + expect((await loadServerResponse(router, '/')).status).toBe(500) + } else { + await router.load() + } + expect(onError).toHaveBeenCalledExactlyOnceWith(error) + expect(loader).not.toHaveBeenCalled() + expect(router.state.matches[0]?.error).toBe(error) + }) + + test('reads a beforeLoad then getter once', async () => { + let reads = 0 + const root = new BaseRootRoute({ + beforeLoad: () => + Object.defineProperty({}, 'then', { + get() { + reads++ + return reads === 1 + ? (resolve: (value: unknown) => void) => + resolve({ token: 'resolved' }) + : undefined + }, + }), + }) + const child = new BaseRoute({ + getParentRoute: () => root, + path: '/', + loader: ({ context }) => (context as { token?: string }).token, + }) + const router = createTestRouter({ + routeTree: root.addChildren([child]), + history: createMemoryHistory({ initialEntries: ['/'] }), + isServer, + }) + if (isServer) { + await loadServerResponse(router, '/') + } else { + await router.load() + } + expect(reads).toBe(1) + expect(router.state.matches.at(-1)?.loaderData).toBe('resolved') + }) +}) + +test.each(['immediate', 'microtask'] as const)( + 'a %s replacement from beforeLoad does not start its stale loader', + async (mode) => { + const root = new BaseRootRoute({}) + const loader = vi.fn() + const stale = new BaseRoute({ + getParentRoute: () => root, + path: '/stale', + beforeLoad: ({ navigate }) => { + const replace = () => { + void navigate({ to: '/current' }) + } + if (mode === 'microtask') { + queueMicrotask(replace) + } else { + replace() + } + return { stale: true } + }, + loader, + }) + const current = new BaseRoute({ + getParentRoute: () => root, + path: '/current', + }) + const router = createTestRouter({ + routeTree: root.addChildren([stale, current]), + history: createMemoryHistory({ initialEntries: ['/stale'] }), + }) + await router.load() + expect(router.state.location.pathname).toBe('/current') + expect(loader).not.toHaveBeenCalled() + }, +) + +test.each(['throw', 'reject'] as const)( + 'a normal component preload can %s a redirect', + async (mode) => { + const root = new BaseRootRoute({}) + const from = new BaseRoute({ + getParentRoute: () => root, + path: '/from', + component: Object.assign(() => null, { + preload: () => { + const result = redirect({ to: '/to' }) + if (mode === 'throw') { + throw result + } + return Promise.reject(result) + }, + }) as any, + }) + const to = new BaseRoute({ getParentRoute: () => root, path: '/to' }) + const router = createTestRouter({ + routeTree: root.addChildren([from, to]), + history: createMemoryHistory({ initialEntries: ['/from'] }), + }) + await router.load() + expect(router.state.location.pathname).toBe('/to') + expect(router.state.matches.at(-1)?.status).toBe('success') + }, +) + +test.each(['throw', 'reject'] as const)( + 'a chunk %s supports reentrant onError control flow', + async (mode) => { + for (const control of ['navigate', 'redirect', 'notFound'] as const) { + const error = new Error('chunk failed') + const root = new BaseRootRoute({}) + const onError = vi.fn(() => { + if (control === 'navigate') { + void router.navigate({ to: '/current' }) + } else if (control === 'redirect') { + throw redirect({ to: '/current' }) + } else { + throw notFound() + } + }) + const stale = new BaseRoute({ + getParentRoute: () => root, + path: '/stale', + component: Object.assign(() => null, { + preload: () => { + if (mode === 'throw') { + throw error + } + return Promise.reject(error) + }, + }) as any, + notFoundComponent: (() => null) as any, + onError, + }) + const current = new BaseRoute({ + getParentRoute: () => root, + path: '/current', + }) + const router = createTestRouter({ + routeTree: root.addChildren([stale, current]), + history: createMemoryHistory({ initialEntries: ['/stale'] }), + }) + await router.load() + expect(onError).toHaveBeenCalledExactlyOnceWith(error) + if (control === 'notFound') { + expect(router.state.matches.at(-1)?.status).toBe('notFound') + } else { + expect(router.state.location.pathname).toBe('/current') + expect(router.state.matches.at(-1)?.status).toBe('success') + } + expect(router._flights?.size ?? 0).toBe(0) + } + }, +) diff --git a/packages/router-core/tests/navigation-wait.bench.ts b/packages/router-core/tests/navigation-wait.bench.ts new file mode 100644 index 00000000000..cc4fee656ad --- /dev/null +++ b/packages/router-core/tests/navigation-wait.bench.ts @@ -0,0 +1,37 @@ +import { bench, describe, expect } from 'vitest' +import { waitFor } from '../src/load-client' + +const signal = new AbortController().signal +for (const mode of ['value', 'promise', 'rejection'] as const) { + const input = + mode === 'value' + ? 42 + : mode === 'promise' + ? Promise.resolve(42) + : Promise.reject(42) + // Consume the rejected input before registering timed cases. + if (mode === 'rejection') { + await expect(waitFor(input, signal)).rejects.toBe(42) + } else { + await expect(waitFor(input, signal)).resolves.toBe(42) + } + describe(`${mode} waits`, () => { + bench( + '80 waits on one signal', + async () => { + for (let index = 0; index < 80; index++) { + if (mode === 'rejection') { + try { + await waitFor(input, signal) + } catch { + // A rejected value still exercises listener cleanup. + } + } else { + await waitFor(input, signal) + } + } + }, + { time: 1500, warmupTime: 300 }, + ) + }) +} diff --git a/packages/router-core/tests/server-awaitable.bench.ts b/packages/router-core/tests/server-awaitable.bench.ts new file mode 100644 index 00000000000..b11fbf5a62e --- /dev/null +++ b/packages/router-core/tests/server-awaitable.bench.ts @@ -0,0 +1,71 @@ +import { createHook } from 'node:async_hooks' +import { bench, describe, expect } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute } from '../src' +import { loadServerRoute } from '../src/load-server' +import { createTestRouter } from './routerTestUtils' +import type { AnyRoute } from '../src' + +for (const mode of ['static', 'sync', 'async', 'mixed'] as const) { + const root = new BaseRootRoute({}) + let policyCalls = 0 + let parent: AnyRoute = root + for (let index = 0; index < 8; index++) { + const parentRoute = parent + const child = new BaseRoute({ + getParentRoute: () => parentRoute, + path: `level${index}`, + ssr: + mode === 'static' || (mode === 'mixed' && index % 4 !== 0) + ? true + : () => { + policyCalls++ + return mode === 'sync' ? true : Promise.resolve(true) + }, + }) + parent.addChildren([child]) + parent = child + } + const router = createTestRouter({ + routeTree: root, + history: createMemoryHistory({ + initialEntries: [ + '/level0/level1/level2/level3/level4/level5/level6/level7', + ], + }), + isServer: true, + }) + const load = () => loadServerRoute(router) + await load() + expect(router.state.matches).toHaveLength(9) + expect( + router.state.matches.every((match) => match.status === 'success'), + ).toBe(true) + expect(router.state.matches.every((match) => match.ssr === true)).toBe(true) + expect(policyCalls).toBe(mode === 'static' ? 0 : mode === 'mixed' ? 2 : 8) + + let promises = 0 + const hook = createHook({ + init(_id, type) { + if (type === 'PROMISE') { + promises++ + } + }, + }) + hook.enable() + await load() + hook.disable() + console.info(`${mode}: ${promises} Promises per server load`) + + describe(`${mode} server hooks`, () => { + bench( + '10 loads through 8 routes', + async () => { + for (let index = 0; index < 10; index++) { + await load() + } + }, + { time: 1500, warmupTime: 300 }, + ) + }) +} diff --git a/packages/router-core/tests/server-static-ssr.test.ts b/packages/router-core/tests/server-static-ssr.test.ts new file mode 100644 index 00000000000..81cd8f824e2 --- /dev/null +++ b/packages/router-core/tests/server-static-ssr.test.ts @@ -0,0 +1,93 @@ +import { runInNewContext } from 'node:vm' +import { expect, test, vi } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute } from '../src' +import { createTestRouter, loadServerResponse } from './routerTestUtils' + +test.each(['false', 'data-only', 'default'] as const)( + 'inherits %s SSR through static and functional children', + async (policy) => { + for (const mode of [ + 'undefined', + 'true', + 'sync', + 'promise', + 'foreign', + 'thenable', + ]) { + const loader = vi.fn() + const root = new BaseRootRoute({ + ssr: + policy === 'default' + ? undefined + : policy === 'false' + ? false + : 'data-only', + }) + const child = new BaseRoute({ + getParentRoute: () => root, + path: '/', + ssr: + mode === 'undefined' + ? undefined + : mode === 'true' + ? true + : () => { + if (mode === 'foreign') { + return runInNewContext('Promise.resolve(true)') + } + if (mode === 'thenable') { + return { then: (resolve: any) => resolve(true) } as any + } + return mode === 'sync' ? true : Promise.resolve(true) + }, + loader, + }) + const router = createTestRouter({ + routeTree: root.addChildren([child]), + history: createMemoryHistory({ initialEntries: ['/'] }), + isServer: true, + }) + router.options.defaultSsr = policy === 'default' ? 'data-only' : true + expect((await loadServerResponse(router, '/')).status).toBe(200) + expect(router.state.matches.map((match) => match.ssr)).toEqual( + policy === 'false' ? [false, false] : ['data-only', 'data-only'], + ) + expect(loader).toHaveBeenCalledTimes(policy === 'false' ? 0 : 1) + } + }, +) + +test.each(['return', 'throw'] as const)( + 'request cancellation wins when an SSR callback aborts then %ss', + async (mode) => { + const controller = new AbortController() + const cancellation = new Error('disconnected') + const context = vi.fn() + const loader = vi.fn() + const onError = vi.fn() + const root = new BaseRootRoute({ + ssr: () => { + controller.abort(cancellation) + if (mode === 'throw') { + throw new Error('obsolete policy error') + } + return true + }, + context, + loader, + onError, + }) + const router = createTestRouter({ + routeTree: root, + history: createMemoryHistory({ initialEntries: ['/'] }), + isServer: true, + }) + await expect( + loadServerResponse(router, '/', controller.signal), + ).rejects.toBe(cancellation) + expect(context).not.toHaveBeenCalled() + expect(loader).not.toHaveBeenCalled() + expect(onError).not.toHaveBeenCalled() + }, +) From d65e90ec19d907157c689a7ab6939db31ca55dcf Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sun, 6 Sep 2026 00:54:29 +0200 Subject: [PATCH 2/7] test(router-core): cover blocking readiness and queued SSR cancellation --- .../tests/navigation-awaitable.bench.ts | 26 ++++++++++++++++--- .../tests/navigation-awaitable.test.ts | 1 + .../tests/server-static-ssr.test.ts | 10 ++++--- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/router-core/tests/navigation-awaitable.bench.ts b/packages/router-core/tests/navigation-awaitable.bench.ts index ce58210c3d5..69ce980ddb9 100644 --- a/packages/router-core/tests/navigation-awaitable.bench.ts +++ b/packages/router-core/tests/navigation-awaitable.bench.ts @@ -7,7 +7,14 @@ import type { AnyRoute } from '../src' // Use the same cases on both implementations. Allocation counts are collected // outside the timed loop so async-hooks instrumentation cannot bias timings. -for (const mode of ['none', 'sync', 'async', 'mixed', 'chunks'] as const) { +for (const mode of [ + 'none', + 'sync', + 'async', + 'mixed', + 'chunks', + 'blocking', +] as const) { const root = new BaseRootRoute({}) let chunkCalls = 0 let parent: AnyRoute = root @@ -18,14 +25,14 @@ for (const mode of ['none', 'sync', 'async', 'mixed', 'chunks'] as const) { getParentRoute: () => parentRoute, path: `level${index}`, beforeLoad: - mode === 'none' || mode === 'chunks' + mode === 'none' || mode === 'chunks' || mode === 'blocking' ? undefined : () => mode === 'async' || (mode === 'mixed' && index % 4 === 0) ? Promise.resolve(value) : value, component: - mode === 'chunks' + mode === 'chunks' || mode === 'blocking' ? (Object.assign(() => null, { preload: () => { chunkCalls++ @@ -33,6 +40,14 @@ for (const mode of ['none', 'sync', 'async', 'mixed', 'chunks'] as const) { }, }) as any) : undefined, + shouldReload: mode === 'blocking' ? true : undefined, + loader: + mode === 'blocking' + ? { + handler: () => Promise.resolve(value), + staleReloadMode: 'blocking', + } + : undefined, }) parent.addChildren([child]) parent = child @@ -52,8 +67,11 @@ for (const mode of ['none', 'sync', 'async', 'mixed', 'chunks'] as const) { expect( router.state.matches.every((match) => match.status === 'success'), ).toBe(true) - if (mode === 'chunks') { + if (mode === 'chunks' || mode === 'blocking') { expect(chunkCalls).toBe(16) + if (mode === 'blocking') { + expect(router.state.matches[8]!.loaderData).toEqual({ level7: 7 }) + } } else if (mode !== 'none') { expect(router.state.matches[8]!.context).toMatchObject({ level0: 0, diff --git a/packages/router-core/tests/navigation-awaitable.test.ts b/packages/router-core/tests/navigation-awaitable.test.ts index d691529de5b..5411e21a0e0 100644 --- a/packages/router-core/tests/navigation-awaitable.test.ts +++ b/packages/router-core/tests/navigation-awaitable.test.ts @@ -203,6 +203,7 @@ test.each(['throw', 'reject'] as const)( }, }) as any, notFoundComponent: (() => null) as any, + loader: control === 'navigate' ? () => 'obsolete data' : undefined, onError, }) const current = new BaseRoute({ diff --git a/packages/router-core/tests/server-static-ssr.test.ts b/packages/router-core/tests/server-static-ssr.test.ts index 81cd8f824e2..21c1b8775a9 100644 --- a/packages/router-core/tests/server-static-ssr.test.ts +++ b/packages/router-core/tests/server-static-ssr.test.ts @@ -58,7 +58,7 @@ test.each(['false', 'data-only', 'default'] as const)( }, ) -test.each(['return', 'throw'] as const)( +test.each(['return', 'throw', 'microtask throw'] as const)( 'request cancellation wins when an SSR callback aborts then %ss', async (mode) => { const controller = new AbortController() @@ -68,8 +68,12 @@ test.each(['return', 'throw'] as const)( const onError = vi.fn() const root = new BaseRootRoute({ ssr: () => { - controller.abort(cancellation) - if (mode === 'throw') { + if (mode === 'microtask throw') { + queueMicrotask(() => controller.abort(cancellation)) + } else { + controller.abort(cancellation) + } + if (mode !== 'return') { throw new Error('obsolete policy error') } return true From 220dbd950551785897734489e29c50ddab32284f Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sun, 6 Sep 2026 01:00:51 +0200 Subject: [PATCH 3/7] perf(router-core): reduce navigation promise chains --- .changeset/cool-streets-punch.md | 5 + RESULT-optimization-navigation-promises.md | 197 +++++++++++++++++++++ packages/router-core/INTERNALS.md | 6 + packages/router-core/src/load-client.ts | 65 +++---- packages/router-core/src/load-server.ts | 17 +- 5 files changed, 254 insertions(+), 36 deletions(-) create mode 100644 .changeset/cool-streets-punch.md create mode 100644 RESULT-optimization-navigation-promises.md diff --git a/.changeset/cool-streets-punch.md b/.changeset/cool-streets-punch.md new file mode 100644 index 00000000000..85a4396f603 --- /dev/null +++ b/.changeset/cool-streets-punch.md @@ -0,0 +1,5 @@ +--- +'@tanstack/router-core': patch +--- + +Reduce Promise allocations during client navigation and static server SSR policy resolution while preserving hook cancellation and thenable handling. diff --git a/RESULT-optimization-navigation-promises.md b/RESULT-optimization-navigation-promises.md new file mode 100644 index 00000000000..d2b8ec853c2 --- /dev/null +++ b/RESULT-optimization-navigation-promises.md @@ -0,0 +1,197 @@ +# Navigation Promise overhead investigation + +Baseline production commit: `07b3bc971d`. Tests and benchmark harnesses were +committed separately in `4d5896442d` and `d65e90ec19`. +Before/after measurements use identical benchmark files and +flags, temporarily restoring only the implementation under test in this worktree. + +## Retained changes + +- **Client chunk readiness:** combine the chunk task and its nested loader-outcome + continuations into one async task. Catch only component-loading failures; await + the loader outcome outside that catch. Publish readiness only after both are + settled, with the existing success, pending-state, and cancellation checks. +- **Client completion:** remove an empty trailing `.then()` from the transaction + completion chain. +- **Server SSR policies:** return static policy values directly. Functional + results are assimilated with `Promise.resolve`; synchronous functional throws + become rejected Promises so queued request cancellation still wins before + context/error hooks run. + +Client `beforeLoad`, `waitFor`, controller ownership, and public types stay +unchanged. The final patch reduces Promise allocation without assuming that an +apparently synchronous hook result can safely skip the existing wait boundary. + +## Allocation results + +Counts are Promise resources reported by `async_hooks` around one operation; +absolute counts include harness awaits. Each lane has a root plus eight nested +routes. The same-harness deltas are the useful result. + +| Scenario | Before | After | Fewer Promises | +| ----------------------------------------------- | -----: | ----: | -------------: | +| Client, no hooks/chunks | 147 | 119 | 28 | +| Client, eight synchronous beforeLoad hooks | 187 | 159 | 28 | +| Client, eight Promise beforeLoad hooks | 187 | 159 | 28 | +| Client, mixed hooks | 187 | 159 | 28 | +| Client, asynchronous component preloads | 187 | 159 | 28 | +| Client, blocking loaders and component preloads | 275 | 247 | 28 | +| Server, static SSR policies | 68 | 50 | 18 | +| Server, synchronous functional policies | 84 | 74 | 10 | +| Server, Promise functional policies | 84 | 74 | 10 | +| Server, mixed policies | 72 | 56 | 16 | + +The client saving is three Promise resources per match plus one per navigation +in these cases. **Controller allocation is unchanged.** Client loader controllers +belong to shared flights and cached data. Server controllers support request +cancellation, boundary retirement, and deferred data cleanup. A synchronous +loader return does not make those controllers disposable. + +There is a separate server opportunity: fresh matching allocates controllers, +then server lane cloning allocates replacements. Removing that duplication would +need to distinguish private fresh matches from reused/exposed controllers; +blindly reusing the incoming controller is not safe for repeated loads. That +ownership change is not part of this patch. + +## Candidate attribution and rejected ideas + +| Independent client candidate | React minimal gzip delta | Decision | +| --------------------------------------------------------- | -----------------------: | ----------------------------------------- | +| Skip awaiting synchronous beforeLoad results | +7 B | Rejected: three regression tests fail | +| Move chunk catch inside the async function only | -2 B | Superseded by consolidated readiness task | +| Consolidate chunk and loader readiness | -5 B | Retained | +| Remove completion's empty then | -5 B | Retained | +| Skip Promise.all for synchronous head/scripts | +18 B | Left out to limit bundle growth | +| Clean up abort listeners in existing settlement callbacks | +2 B | Rejected: direct waits slowed down | + +The beforeLoad guard reads a custom `then` getter twice: once to detect it and +again during assimilation. A stateful getter resolves incorrectly on both client +and server. Removing the client await also lets a hook that queues a replacement +navigation schedule stale loader work before cancellation. All three regressions +pass with the original hook waiting behavior retained. + +The listener-cleanup candidate removed one Promise per wait but added callback +work. In the same 80-wait harness, mean times regressed by 3.1% for plain values, +6.6% for fulfilled Promises, and 5.8% for rejected Promises (RME 0.12–0.42%). It +was removed despite reducing allocations. Direct-wait benchmark coverage remains +available for future alternatives. + +## Bundle results + +The final composition is **85,821 → 85,815 gzip bytes** in React Router minimal +(**-6 B**). Initial gzip is **85,681 → 85,676 B**; raw JS is **268,520 → 268,523 B**; +Brotli is **74,720 → 74,698 B**. Its two-file JS split is unchanged. + +Across all 18 scenarios, 16 shrink, one is unchanged, and one increases by 2 B. +Gzip deltas range from **-7 to +2 B**. There are no chunk-count changes. Gzip +changes are not additive, so independent hunk results must not be summed. + +| Scenario | Before gzip B | After gzip B | Gzip delta | Initial gzip delta | Raw delta | Brotli delta | +| -------------------------------- | ------------: | -----------: | ---------: | -----------------: | --------: | -----------: | +| react-router.minimal | 85821 | 85815 | -6 | -5 | +3 | -22 | +| react-router.full | 89430 | 89425 | -5 | -4 | +3 | +68 | +| solid-router.minimal | 33985 | 33982 | -3 | -3 | +3 | +47 | +| solid-router.full | 38927 | 38921 | -6 | -5 | +3 | +44 | +| vue-router.minimal | 50706 | 50700 | -6 | -5 | +3 | +13 | +| vue-router.full | 56457 | 56453 | -4 | -4 | +3 | -2 | +| react-start.minimal | 99061 | 99063 | +2 | +1 | +3 | -42 | +| react-start.query-integration | 106571 | 106569 | -2 | -3 | +3 | -66 | +| react-start.deferred-hydration | 99804 | 99797 | -7 | 0 | +3 | -77 | +| react-start.full | 102303 | 102301 | -2 | -3 | +3 | +23 | +| react-start.rsbuild.minimal | 102446 | 102444 | -2 | -2 | +2 | +50 | +| react-start.rsbuild.minimal-iife | 102856 | 102856 | 0 | 0 | +2 | -46 | +| react-start.rsbuild.full | 105840 | 105837 | -3 | -3 | +2 | -158 | +| solid-start.minimal | 47143 | 47138 | -5 | -5 | +3 | +89 | +| solid-start.deferred-hydration | 50295 | 50293 | -2 | -5 | +3 | +7 | +| solid-start.full | 52342 | 52341 | -1 | -4 | +3 | +31 | +| vue-start.minimal | 67241 | 67238 | -3 | 0 | +3 | -9 | +| vue-start.full | 71150 | 71147 | -3 | -4 | +3 | -57 | + +## Timing results and limits + +All times below are milliseconds per batch of ten navigations or server loads. +These are warm in-process microbenchmarks, not browser latency measurements. +The synchronous/mixed client and blocking-loader means have substantial outliers; +**do not interpret the mean deltas as a general navigation speedup or slowdown**. +For those cases, allocation counts are firmer evidence than timing. Static SSR +improved about 3% in the sampled runs with low within-run RME; that remains a +single-machine result. + +| Case | Before mean ms | After mean ms | Mean delta | Before / after RME | +| --------------------------- | -------------: | ------------: | ---------: | -----------------: | +| Client: none beforeLoad | 0.2718 | 0.2631 | -3.2% | 2.84% / 2.62% | +| Client: sync beforeLoad | 0.3549 | 0.3307 | -6.8% | 12.84% / 8.71% | +| Client: async beforeLoad | 0.3212 | 0.3063 | -4.6% | 1.03% / 1.02% | +| Client: mixed beforeLoad | 0.3152 | 0.3382 | 7.3% | 1.18% / 16.14% | +| Client: chunks beforeLoad | 0.3283 | 0.2749 | -16.3% | 18.30% / 0.99% | +| Client: blocking beforeLoad | 0.9235 | 0.9057 | -1.9% | 7.81% / 5.42% | +| Server: static server hooks | 0.1263 | 0.1226 | -2.9% | 0.30% / 0.34% | +| Server: sync server hooks | 0.1435 | 0.1381 | -3.7% | 0.47% / 0.37% | +| Server: async server hooks | 0.1406 | 0.1390 | -1.1% | 0.58% / 0.76% | +| Server: mixed server hooks | 0.1348 | 0.1334 | -1.1% | 1.13% / 1.30% | + +
+Sampling details (times in milliseconds) + +| Case | Hz | SD | Median | p99 | p999 | Samples | +| ------------------------------------ | -----: | -----: | -----: | -----: | ------: | ------: | +| Client: none beforeLoad (Before) | 3679.7 | 0.2921 | 0.2497 | 0.9792 | 1.8268 | 5520 | +| Client: none beforeLoad (After) | 3800.4 | 0.2658 | 0.2420 | 0.4923 | 1.9607 | 5703 | +| Client: sync beforeLoad (Before) | 2817.5 | 1.5111 | 0.3077 | 1.3887 | 3.0216 | 4227 | +| Client: sync beforeLoad (After) | 3024.1 | 0.9898 | 0.2955 | 1.1453 | 1.8660 | 4537 | +| Client: async beforeLoad (Before) | 3113.6 | 0.1151 | 0.3024 | 1.1680 | 1.5653 | 4671 | +| Client: async beforeLoad (After) | 3264.4 | 0.1116 | 0.2907 | 1.1553 | 1.6467 | 4897 | +| Client: mixed beforeLoad (Before) | 3172.3 | 0.1311 | 0.2977 | 1.3473 | 1.6404 | 4759 | +| Client: mixed beforeLoad (After) | 2956.9 | 1.8543 | 0.2916 | 1.3904 | 1.8401 | 4436 | +| Client: chunks beforeLoad (Before) | 3046.0 | 2.0724 | 0.2674 | 2.4513 | 2.8285 | 4569 | +| Client: chunks beforeLoad (After) | 3637.6 | 0.1026 | 0.2614 | 1.1515 | 1.3950 | 5457 | +| Client: blocking beforeLoad (Before) | 1082.8 | 1.4826 | 0.7633 | 6.9268 | 12.0854 | 1625 | +| Client: blocking beforeLoad (After) | 1104.1 | 1.0201 | 0.7586 | 8.3123 | 9.4379 | 1657 | +| Server: static server hooks (Before) | 7919.3 | 0.0208 | 0.1228 | 0.2827 | 0.3400 | 11879 | +| Server: static server hooks (After) | 8158.7 | 0.0235 | 0.1194 | 0.2931 | 0.3859 | 12238 | +| Server: sync server hooks (Before) | 6970.9 | 0.0353 | 0.1367 | 0.2933 | 0.5290 | 10457 | +| Server: sync server hooks (After) | 7241.7 | 0.0271 | 0.1336 | 0.2879 | 0.4732 | 10863 | +| Server: async server hooks (Before) | 7113.7 | 0.0433 | 0.1356 | 0.3400 | 0.4499 | 10671 | +| Server: async server hooks (After) | 7196.3 | 0.0559 | 0.1335 | 0.3129 | 0.5320 | 10795 | +| Server: mixed server hooks (Before) | 7416.7 | 0.0817 | 0.1270 | 0.2965 | 0.5972 | 11126 | +| Server: mixed server hooks (After) | 7497.8 | 0.0935 | 0.1236 | 0.2864 | 0.8237 | 11247 | + +
+ +Each benchmark case samples for 1.5 seconds after 0.3 seconds of warmup. Client +and server cases batch ten operations; direct wait cases batch 80 waits on one +signal. Setup, assertions, and Promise instrumentation run outside timing. The +client harness asserts repeated chunk preload calls and includes a forced +blocking-loader case. Server cases isolate static and functional SSR policies +without beforeLoad work obscuring attribution. + +Run the three files independently through: + +`CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:unit --outputStyle=stream --skipRemoteCache -- bench tests/.bench.ts --run --outputJson=` + +Files: `navigation-awaitable`, `server-awaitable`, `navigation-wait`. + +Named bundle runs are under `benchmarks/bundle-size/results/runs/`: +`navigation-before` and `navigation-flat-full` hold the complete comparison; +`navigation-flat-alone` and `navigation-completion` isolate retained client +hunks. `navigation-final-before` repeats the original minimal bundle result. +Timing JSON and logs are in `/tmp/router-navigation-investigation/`, with final +client results named `client-final-before` / `client-final-after` and final +server results `server-before` / `server-final`. + +## Validation + +- Before: 1,717 core tests passed, four existing expected failures; the additional + queued SSR cancellation test also passed separately on the original source. +- After: 1,718 core tests passed, four existing expected failures. +- Router-core type checks: TypeScript 5.6, 5.7, 5.8, 5.9, 6, and 7 passed. +- Router-core ESLint: no errors; 26 existing warnings. +- React pending/presentation regression tests: 27 passed. +- React Router Chromium redirect tests: 33 passed. +- React Start Vite SSR head and hydration tests: three passed. +- Prettier and `git diff --check` passed. + +CI will provide the broader correctness and performance check: full unit and +E2E coverage, bundle-size comparisons, and CodSpeed CPU simulation benchmarks +for client navigation and SSR. CodSpeed memory benchmarks are excluded from +the performance assessment. diff --git a/packages/router-core/INTERNALS.md b/packages/router-core/INTERNALS.md index d39c229768f..a2ec1c62b58 100644 --- a/packages/router-core/INTERNALS.md +++ b/packages/router-core/INTERNALS.md @@ -339,6 +339,12 @@ uses the active preload entry as its additional authority. `beforeLoad` context is not a cache. +Client `beforeLoad` results still pass through Promise assimilation and a +cancellable wait. A synchronous-looking result can have a custom `then` getter, +and a hook can queue a replacement navigation before its loader is planned. +Avoid probing `then` and then assimilating the same value again, or removing +that scheduling boundary without accounting for both behaviors. + A completed client preload never stores reusable `beforeLoad` output. When its loader data enters the route cache, the merged context is discarded; the same-ID route-local `_ctx` may remain reusable. A later navigation rebuilds the diff --git a/packages/router-core/src/load-client.ts b/packages/router-core/src/load-client.ts index fc966253e77..929f384fa5b 100644 --- a/packages/router-core/src/load-client.ts +++ b/packages/router-core/src/load-client.ts @@ -894,38 +894,39 @@ function createLoaderTask( reloadFailure ?? [SUCCESS, match.loaderData], ) - // The async wrapper catches synchronous preload failures without deferring work. - const chunkOutcome = (async (): Promise => { - const chunk = loadRouteChunk(route, undefined, onLazyReady) - if (chunk) { - await waitFor(chunk, options[0 /* controller */].signal) - } - })().catch((cause): IndexedOutcome | undefined => - lane[1 /* matches */].some( - (candidate, candidateIndex) => - candidateIndex <= index && - (candidate.status === 'error' || - candidate.status === 'notFound' || - candidate._notFound), - ) - ? undefined - : [index, normalizeLaneError(router, lane, route, cause, options)], - ) - const chunkFailure = chunkOutcome.then((failure) => - outcome.then((result) => { - if ( - blocking && - !failure && - result[0 /* kind */] === SUCCESS && - match.status === 'pending' && - !options[0 /* controller */].signal.aborted - ) { - match.status = 'success' - onReady?.() + // Keep thrown preloads and rejected chunks in the same task promise. + const chunkFailure = (async (): Promise => { + let failure: IndexedOutcome | undefined + try { + const chunk = loadRouteChunk(route, undefined, onLazyReady) + if (chunk) { + await waitFor(chunk, options[0 /* controller */].signal) } - return failure - }), - ) + } catch (cause) { + failure = lane[1 /* matches */].some( + (candidate, candidateIndex) => + candidateIndex <= index && + (candidate.status === 'error' || + candidate.status === 'notFound' || + candidate._notFound), + ) + ? undefined + : [index, normalizeLaneError(router, lane, route, cause, options)] + } + // Readiness requires both the component chunk and loader data. + const result = await outcome + if ( + blocking && + !failure && + result[0 /* kind */] === SUCCESS && + match.status === 'pending' && + !options[0 /* controller */].signal.aborted + ) { + match.status = 'success' + onReady?.() + } + return failure + })() tasks.push([index, outcome, chunkFailure]) if (!background) { return outcome.then((result) => getParentSnapshot(match, result)) @@ -1982,7 +1983,7 @@ export async function loadClientRoute( ) const done = opts?.sync ? new Promise((resolve) => (settle = resolve)) - : Promise.resolve().then(run).then() + : Promise.resolve().then(run) const tx: LoadTransaction = [ controller, redirects, diff --git a/packages/router-core/src/load-server.ts b/packages/router-core/src/load-server.ts index f04011d1a4a..1991828d4fd 100644 --- a/packages/router-core/src/load-server.ts +++ b/packages/router-core/src/load-server.ts @@ -155,11 +155,11 @@ function waitFor(value: Promise, signal?: AbortSignal): Promise { return signal ? waitForReason(value, signal) : value } -async function resolveSsr( +function resolveSsr( router: AnyRouter, lane: MatchedLane, index: number, -): Promise { +): SSROption | Promise { const match = lane.matches[index]! const route = getRoute(router, match) const parentSsr = lane.matches[index - 1]?.ssr @@ -203,7 +203,14 @@ async function resolveSsr( ssr: candidate.ssr, })), } - return inherit((await option(context)) ?? defaultSsr) + try { + return Promise.resolve(option(context)).then((value) => + inherit(value ?? defaultSsr), + ) + } catch (cause) { + // Functional failures keep their asynchronous cancellation checkpoint. + return Promise.reject(cause) + } } function stampNotFound( @@ -232,7 +239,9 @@ async function contextualize( const match = lane.matches[index]! const route = getRoute(router, match) try { - match.ssr = await resolveSsr(router, lane, index) + const ssr = resolveSsr(router, lane, index) + // Functional policies are assimilated into a native Promise above. + match.ssr = ssr instanceof Promise ? await ssr : ssr } catch (cause) { signal?.throwIfAborted() failure = [ From c069c4becafa1288874194faf70434be294ae812 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sun, 6 Sep 2026 01:17:31 +0200 Subject: [PATCH 4/7] test(router-core): focus awaitable coverage on standard promises --- .../tests/navigation-awaitable.bench.ts | 7 +- .../tests/navigation-awaitable.test.ts | 182 ++++++++---------- 2 files changed, 82 insertions(+), 107 deletions(-) diff --git a/packages/router-core/tests/navigation-awaitable.bench.ts b/packages/router-core/tests/navigation-awaitable.bench.ts index 69ce980ddb9..4e6b2d88b3d 100644 --- a/packages/router-core/tests/navigation-awaitable.bench.ts +++ b/packages/router-core/tests/navigation-awaitable.bench.ts @@ -10,6 +10,7 @@ import type { AnyRoute } from '../src' for (const mode of [ 'none', 'sync', + 'void', 'async', 'mixed', 'chunks', @@ -30,7 +31,9 @@ for (const mode of [ : () => mode === 'async' || (mode === 'mixed' && index % 4 === 0) ? Promise.resolve(value) - : value, + : mode === 'void' + ? undefined + : value, component: mode === 'chunks' || mode === 'blocking' ? (Object.assign(() => null, { @@ -72,7 +75,7 @@ for (const mode of [ if (mode === 'blocking') { expect(router.state.matches[8]!.loaderData).toEqual({ level7: 7 }) } - } else if (mode !== 'none') { + } else if (mode !== 'none' && mode !== 'void') { expect(router.state.matches[8]!.context).toMatchObject({ level0: 0, level7: 7, diff --git a/packages/router-core/tests/navigation-awaitable.test.ts b/packages/router-core/tests/navigation-awaitable.test.ts index 5411e21a0e0..1a0f165f93d 100644 --- a/packages/router-core/tests/navigation-awaitable.test.ts +++ b/packages/router-core/tests/navigation-awaitable.test.ts @@ -5,112 +5,42 @@ import { BaseRootRoute, BaseRoute, notFound, redirect } from '../src' import { createTestRouter, loadServerResponse } from './routerTestUtils' describe.each([false, true])('awaitable hooks (server=%s)', (isServer) => { - test.each([ - 'sync', - 'promise', - 'thenable', - 'foreign promise', - 'callable thenable', - ])('inherits the result of a %s beforeLoad', async (mode) => { - const value = { token: 'parent context' } - const root = new BaseRootRoute({ - beforeLoad: () => { - switch (mode) { - case 'promise': - return Promise.resolve(value) - case 'thenable': - return { then: (resolve: any) => resolve(value) } as any - case 'foreign promise': - return runInNewContext('Promise.resolve(value)', { value }) - case 'callable thenable': - return Object.assign(() => {}, { - then: (resolve: any) => resolve(value), - }) as any - default: - return value - } - }, - }) - const loader = vi.fn(({ context }) => context.token) - const child = new BaseRoute({ - getParentRoute: () => root, - path: '/', - loader, - }) - const router = createTestRouter({ - routeTree: root.addChildren([child]), - history: createMemoryHistory({ initialEntries: ['/'] }), - isServer, - }) - if (isServer) { - expect((await loadServerResponse(router, '/')).status).toBe(200) - } else { - await router.load() - } - expect(loader).toHaveBeenCalledOnce() - expect(router.state.matches.at(-1)?.loaderData).toBe(value.token) - }) - - test('normalizes a throwing then getter as a beforeLoad error', async () => { - const error = new Error('cannot read then') - const onError = vi.fn() - const loader = vi.fn() - const root = new BaseRootRoute({ - beforeLoad: () => ({ - get then(): never { - throw error + test.each(['sync', 'promise', 'foreign promise'])( + 'inherits the result of a %s beforeLoad', + async (mode) => { + const value = { token: 'parent context' } + const root = new BaseRootRoute({ + beforeLoad: () => { + switch (mode) { + case 'promise': + return Promise.resolve(value) + case 'foreign promise': + return runInNewContext('Promise.resolve(value)', { value }) + default: + return value + } }, - }), - loader, - onError, - }) - const router = createTestRouter({ - routeTree: root, - history: createMemoryHistory({ initialEntries: ['/'] }), - isServer, - }) - if (isServer) { - expect((await loadServerResponse(router, '/')).status).toBe(500) - } else { - await router.load() - } - expect(onError).toHaveBeenCalledExactlyOnceWith(error) - expect(loader).not.toHaveBeenCalled() - expect(router.state.matches[0]?.error).toBe(error) - }) - - test('reads a beforeLoad then getter once', async () => { - let reads = 0 - const root = new BaseRootRoute({ - beforeLoad: () => - Object.defineProperty({}, 'then', { - get() { - reads++ - return reads === 1 - ? (resolve: (value: unknown) => void) => - resolve({ token: 'resolved' }) - : undefined - }, - }), - }) - const child = new BaseRoute({ - getParentRoute: () => root, - path: '/', - loader: ({ context }) => (context as { token?: string }).token, - }) - const router = createTestRouter({ - routeTree: root.addChildren([child]), - history: createMemoryHistory({ initialEntries: ['/'] }), - isServer, - }) - if (isServer) { - await loadServerResponse(router, '/') - } else { - await router.load() - } - expect(reads).toBe(1) - expect(router.state.matches.at(-1)?.loaderData).toBe('resolved') - }) + }) + const loader = vi.fn(({ context }) => context.token) + const child = new BaseRoute({ + getParentRoute: () => root, + path: '/', + loader, + }) + const router = createTestRouter({ + routeTree: root.addChildren([child]), + history: createMemoryHistory({ initialEntries: ['/'] }), + isServer, + }) + if (isServer) { + expect((await loadServerResponse(router, '/')).status).toBe(200) + } else { + await router.load() + } + expect(loader).toHaveBeenCalledOnce() + expect(router.state.matches.at(-1)?.loaderData).toBe(value.token) + }, + ) }) test.each(['immediate', 'microtask'] as const)( @@ -148,6 +78,48 @@ test.each(['immediate', 'microtask'] as const)( }, ) +test.each(['native', 'foreign'] as const)( + 'supersedes an unresolved %s Promise beforeLoad and observes its late rejection', + async (mode) => { + let rejectValue!: (error: Error) => void + const capture = (_resolve: unknown, reject: typeof rejectValue) => { + rejectValue = reject + } + const pending = + mode === 'native' + ? new Promise(capture) + : runInNewContext('new Promise(capture)', { capture }) + const beforeLoad = vi.fn(() => pending) + const loader = vi.fn() + const onError = vi.fn() + const root = new BaseRootRoute({}) + const stale = new BaseRoute({ + getParentRoute: () => root, + path: '/stale', + beforeLoad, + loader, + onError, + }) + const current = new BaseRoute({ + getParentRoute: () => root, + path: '/current', + }) + const router = createTestRouter({ + routeTree: root.addChildren([stale, current]), + history: createMemoryHistory({ initialEntries: ['/stale'] }), + }) + const staleLoad = router.load() + await vi.waitFor(() => expect(beforeLoad).toHaveBeenCalledOnce()) + await router.navigate({ to: '/current' }) + await staleLoad + rejectValue(new Error('late failure')) + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(router.state.location.pathname).toBe('/current') + expect(loader).not.toHaveBeenCalled() + expect(onError).not.toHaveBeenCalled() + }, +) + test.each(['throw', 'reject'] as const)( 'a normal component preload can %s a redirect', async (mode) => { From b8e8fd5a7fd8a363549de77a08a0f4b2291f43fb Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sun, 6 Sep 2026 01:21:53 +0200 Subject: [PATCH 5/7] perf(router-core): bypass cancellable waits for synchronous beforeLoad --- .changeset/cool-streets-punch.md | 2 +- RESULT-optimization-navigation-promises.md | 125 +++++++++++++++++---- packages/router-core/INTERNALS.md | 9 +- packages/router-core/src/load-client.ts | 19 ++-- 4 files changed, 117 insertions(+), 38 deletions(-) diff --git a/.changeset/cool-streets-punch.md b/.changeset/cool-streets-punch.md index 85a4396f603..2f813f30e22 100644 --- a/.changeset/cool-streets-punch.md +++ b/.changeset/cool-streets-punch.md @@ -2,4 +2,4 @@ '@tanstack/router-core': patch --- -Reduce Promise allocations during client navigation and static server SSR policy resolution while preserving hook cancellation and thenable handling. +Reduce Promise allocations during client navigation and static server SSR policy resolution. Skip cancellable waits for synchronous beforeLoad results while preserving navigation cancellation. diff --git a/RESULT-optimization-navigation-promises.md b/RESULT-optimization-navigation-promises.md index d2b8ec853c2..699d29832a8 100644 --- a/RESULT-optimization-navigation-promises.md +++ b/RESULT-optimization-navigation-promises.md @@ -5,7 +5,86 @@ committed separately in `4d5896442d` and `d65e90ec19`. Before/after measurements use identical benchmark files and flags, temporarily restoring only the implementation under test in this worktree. -## Retained changes +## Standard-Promise follow-up + +The supported hook contract assumes ordinary Promises; custom stateful `then` +getters do not constrain this optimization. Those tests were removed. Native and +foreign Promise coverage remains, including cancellation before settlement and a +late rejection. An `undefined` hook-return benchmark was added. + +Client `beforeLoad` now detects Promise results before calling `waitFor`. +Synchronous context skips the wrapper and abort listener but still crosses an +`await`, allowing queued replacement navigation to cancel stale work. Server +`beforeLoad` already awaits directly, so this follow-up changes only the client. + +Compared with the initial PR (`220dbd9505`), the same nine-match harness reports: + +| Hook distribution | Initial PR | Follow-up | Fewer Promise resources | +| ---------------------------- | ---------: | --------: | ----------------------: | +| Eight synchronous objects | 159 | 135 | 24 | +| Eight undefined returns | 159 | 135 | 24 | +| Mixed: six sync, two Promise | 159 | 141 | 18 | +| Eight Promises | 159 | 159 | 0 | + +Each synchronous hook also avoids one abort listener registration and removal. +Controller allocation remains unchanged. Against the original production +baseline, the synchronous-object case saves **52 Promise resources** per +navigation, including the earlier chunk/completion changes. + +The shared `isPromise` helper variant added 35 gzip bytes to React Router +minimal. The retained inline guard adds 13 bytes to the initial PR instead. +The complete patch is now **85,821 → 85,828 gzip bytes (+7 B)** in React Router +minimal. Across all 18 scenarios the complete patch adds **7–16 gzip bytes**. +This is a small size tradeoff for removing the synchronous hook wait scaffolding. +JS file counts are unchanged. Full measurements are in +`navigation-standard-final`; the initial-PR comparison is `navigation-standard-inline`. + +Fresh timing comparisons use the test-only commit `c069c4beca` as BEFORE and the +same source with the client guard as AFTER. Raw results are +`/tmp/router-navigation-investigation/standard-before.json` and `standard-after.json`. +Several means have high RME or large outliers, including unchanged control paths; +these timings are diagnostic only. CI CPU simulation is the performance criterion, +and CodSpeed memory results are excluded. + +| Case | Before mean ms | After mean ms | Before / after RME | Before / after median ms | +| ------------------- | -------------: | ------------: | -----------------: | -----------------------: | +| none beforeLoad | 0.2701 | 0.2573 | 3.46% / 1.32% | 0.2470 / 0.2400 | +| sync beforeLoad | 0.3343 | 0.2962 | 8.89% / 8.68% | 0.2973 / 0.2714 | +| void beforeLoad | 0.2942 | 0.2707 | 1.00% / 0.91% | 0.2795 / 0.2594 | +| async beforeLoad | 0.3404 | 0.3238 | 16.79% / 9.85% | 0.2910 / 0.2897 | +| mixed beforeLoad | 0.3039 | 0.2891 | 0.95% / 0.89% | 0.2886 / 0.2765 | +| chunks beforeLoad | 0.2725 | 0.3127 | 0.99% / 18.07% | 0.2595 / 0.2633 | +| blocking beforeLoad | 1.0469 | 0.8812 | 25.64% / 4.56% | 0.7606 / 0.7544 | + +
+Follow-up timing distribution + +| Case | Hz | SD ms | p99 ms | p999 ms | Samples | +| ---------------------------- | -----: | -----: | -----: | ------: | ------: | +| none beforeLoad (Before) | 3701.8 | 0.3556 | 0.5220 | 2.2237 | 5553 | +| none beforeLoad (After) | 3886.3 | 0.1327 | 0.8483 | 1.7761 | 5830 | +| sync beforeLoad (Before) | 2991.5 | 1.0154 | 1.1645 | 1.8701 | 4488 | +| sync beforeLoad (After) | 3375.7 | 0.9331 | 1.0310 | 1.2777 | 5064 | +| void beforeLoad (Before) | 3399.2 | 0.1067 | 1.0191 | 1.7078 | 5099 | +| void beforeLoad (After) | 3694.3 | 0.0941 | 1.0325 | 1.2072 | 5542 | +| async beforeLoad (Before) | 2937.3 | 1.9359 | 1.3031 | 1.9019 | 4406 | +| async beforeLoad (After) | 3088.3 | 1.1084 | 1.0832 | 1.8670 | 4638 | +| mixed beforeLoad (Before) | 3290.3 | 0.1033 | 1.0402 | 1.5141 | 4936 | +| mixed beforeLoad (After) | 3458.5 | 0.0948 | 1.0727 | 1.3303 | 5188 | +| chunks beforeLoad (Before) | 3669.2 | 0.1025 | 1.1019 | 1.4850 | 5504 | +| chunks beforeLoad (After) | 3197.8 | 2.0485 | 1.2917 | 2.0227 | 5051 | +| blocking beforeLoad (Before) | 955.2 | 5.1849 | 8.1289 | 10.1187 | 1433 | +| blocking beforeLoad (After) | 1134.8 | 0.8455 | 6.7124 | 8.6870 | 1703 | + +
+ +Follow-up validation: 1,712 core tests passed with four existing expected +failures; core types passed on TypeScript 5.6–7, and ESLint passed with the same +26 existing warnings. All 33 Chromium redirect tests passed again. +The lower unit-test count reflects removal of the custom +thenable/getter cases and addition of two pending-Promise cancellation tests. + +## Initial PR changes (`220dbd9505`) - **Client chunk readiness:** combine the chunk task and its nested loader-outcome continuations into one async task. Catch only component-loading failures; await @@ -18,11 +97,11 @@ flags, temporarily restoring only the implementation under test in this worktree become rejected Promises so queued request cancellation still wins before context/error hooks run. -Client `beforeLoad`, `waitFor`, controller ownership, and public types stay -unchanged. The final patch reduces Promise allocation without assuming that an -apparently synchronous hook result can safely skip the existing wait boundary. +The initial PR left client `beforeLoad`, `waitFor`, controller ownership, and +public types unchanged. The standard-Promise follow-up below also optimizes +client `beforeLoad` while retaining its cancellation checkpoint. -## Allocation results +## Initial PR allocation results Counts are Promise resources reported by `async_hooks` around one operation; absolute counts include harness awaits. Each lane has a root plus eight nested @@ -55,20 +134,20 @@ ownership change is not part of this patch. ## Candidate attribution and rejected ideas -| Independent client candidate | React minimal gzip delta | Decision | -| --------------------------------------------------------- | -----------------------: | ----------------------------------------- | -| Skip awaiting synchronous beforeLoad results | +7 B | Rejected: three regression tests fail | -| Move chunk catch inside the async function only | -2 B | Superseded by consolidated readiness task | -| Consolidate chunk and loader readiness | -5 B | Retained | -| Remove completion's empty then | -5 B | Retained | -| Skip Promise.all for synchronous head/scripts | +18 B | Left out to limit bundle growth | -| Clean up abort listeners in existing settlement callbacks | +2 B | Rejected: direct waits slowed down | - -The beforeLoad guard reads a custom `then` getter twice: once to detect it and -again during assimilation. A stateful getter resolves incorrectly on both client -and server. Removing the client await also lets a hook that queues a replacement -navigation schedule stale loader work before cancellation. All three regressions -pass with the original hook waiting behavior retained. +| Independent client candidate | React minimal gzip delta | Decision | +| --------------------------------------------------------- | -----------------------: | ------------------------------------------------ | +| Skip awaiting synchronous beforeLoad results | +7 B | Rejected: queued replacement starts stale loader | +| Move chunk catch inside the async function only | -2 B | Superseded by consolidated readiness task | +| Consolidate chunk and loader readiness | -5 B | Retained | +| Remove completion's empty then | -5 B | Retained | +| Skip Promise.all for synchronous head/scripts | +18 B | Left out to limit bundle growth | +| Clean up abort listeners in existing settlement callbacks | +2 B | Rejected: direct waits slowed down | + +Removing the client await lets a hook that queues a replacement navigation +schedule stale loader work before cancellation. The original investigation also +flagged custom stateful `then` getters; those are outside the agreed standard- +Promise contract and are no longer a reason to reject the guard. The follow-up +keeps the await while bypassing the cancellable wrapper for synchronous results. The listener-cleanup candidate removed one Promise per wait but added callback work. In the same 80-wait harness, mean times regressed by 3.1% for plain values, @@ -76,9 +155,9 @@ work. In the same 80-wait harness, mean times regressed by 3.1% for plain values was removed despite reducing allocations. Direct-wait benchmark coverage remains available for future alternatives. -## Bundle results +## Initial PR bundle results -The final composition is **85,821 → 85,815 gzip bytes** in React Router minimal +The initial PR composition is **85,821 → 85,815 gzip bytes** in React Router minimal (**-6 B**). Initial gzip is **85,681 → 85,676 B**; raw JS is **268,520 → 268,523 B**; Brotli is **74,720 → 74,698 B**. Its two-file JS split is unchanged. @@ -107,7 +186,7 @@ changes are not additive, so independent hunk results must not be summed. | vue-start.minimal | 67241 | 67238 | -3 | 0 | +3 | -9 | | vue-start.full | 71150 | 71147 | -3 | -4 | +3 | -57 | -## Timing results and limits +## Initial PR timing results and limits All times below are milliseconds per batch of ten navigations or server loads. These are warm in-process microbenchmarks, not browser latency measurements. @@ -179,7 +258,7 @@ Timing JSON and logs are in `/tmp/router-navigation-investigation/`, with final client results named `client-final-before` / `client-final-after` and final server results `server-before` / `server-final`. -## Validation +## Initial PR validation - Before: 1,717 core tests passed, four existing expected failures; the additional queued SSR cancellation test also passed separately on the original source. diff --git a/packages/router-core/INTERNALS.md b/packages/router-core/INTERNALS.md index a2ec1c62b58..3314b5d2c3b 100644 --- a/packages/router-core/INTERNALS.md +++ b/packages/router-core/INTERNALS.md @@ -339,11 +339,10 @@ uses the active preload entry as its additional authority. `beforeLoad` context is not a cache. -Client `beforeLoad` results still pass through Promise assimilation and a -cancellable wait. A synchronous-looking result can have a custom `then` getter, -and a hook can queue a replacement navigation before its loader is planned. -Avoid probing `then` and then assimilating the same value again, or removing -that scheduling boundary without accounting for both behaviors. +Client `beforeLoad` only installs a cancellable wait for Promise results. +Synchronous context still crosses an `await` before the cancellation check: +a hook can queue a replacement navigation before its loader is planned. +Promise detection assumes ordinary Promise behavior. A completed client preload never stores reusable `beforeLoad` output. When its loader data enters the route cache, the merged context is discarded; the diff --git a/packages/router-core/src/load-client.ts b/packages/router-core/src/load-client.ts index 929f384fa5b..24a3ae55faa 100644 --- a/packages/router-core/src/load-client.ts +++ b/packages/router-core/src/load-client.ts @@ -429,15 +429,16 @@ async function contextualize( } try { setFetching(router, match, 'beforeLoad', options[0 /* controller */]) - const result = await waitFor( - beforeLoad({ - ...common, - search: match.search, - context: match.context, - ...router.options.additionalContext, - }), - signal, - ) + const value = beforeLoad({ + ...common, + search: match.search, + context: match.context, + ...router.options.additionalContext, + }) + // Keep the cancellation checkpoint without wrapping synchronous context. + const result = await (typeof value?.then === 'function' + ? waitFor(value, signal) + : value) if (signal.aborted) { return [index, CANCELED_OUTCOME] } From 46681ae9d5e26c4dbc6a0aed5135a716c545c722 Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sun, 6 Sep 2026 09:29:19 +0200 Subject: [PATCH 6/7] chore: remove navigation investigation artifacts --- RESULT-optimization-navigation-promises.md | 276 ------------------ .../tests/navigation-awaitable.bench.ts | 109 ------- .../tests/navigation-wait.bench.ts | 37 --- .../tests/server-awaitable.bench.ts | 71 ----- 4 files changed, 493 deletions(-) delete mode 100644 RESULT-optimization-navigation-promises.md delete mode 100644 packages/router-core/tests/navigation-awaitable.bench.ts delete mode 100644 packages/router-core/tests/navigation-wait.bench.ts delete mode 100644 packages/router-core/tests/server-awaitable.bench.ts diff --git a/RESULT-optimization-navigation-promises.md b/RESULT-optimization-navigation-promises.md deleted file mode 100644 index 699d29832a8..00000000000 --- a/RESULT-optimization-navigation-promises.md +++ /dev/null @@ -1,276 +0,0 @@ -# Navigation Promise overhead investigation - -Baseline production commit: `07b3bc971d`. Tests and benchmark harnesses were -committed separately in `4d5896442d` and `d65e90ec19`. -Before/after measurements use identical benchmark files and -flags, temporarily restoring only the implementation under test in this worktree. - -## Standard-Promise follow-up - -The supported hook contract assumes ordinary Promises; custom stateful `then` -getters do not constrain this optimization. Those tests were removed. Native and -foreign Promise coverage remains, including cancellation before settlement and a -late rejection. An `undefined` hook-return benchmark was added. - -Client `beforeLoad` now detects Promise results before calling `waitFor`. -Synchronous context skips the wrapper and abort listener but still crosses an -`await`, allowing queued replacement navigation to cancel stale work. Server -`beforeLoad` already awaits directly, so this follow-up changes only the client. - -Compared with the initial PR (`220dbd9505`), the same nine-match harness reports: - -| Hook distribution | Initial PR | Follow-up | Fewer Promise resources | -| ---------------------------- | ---------: | --------: | ----------------------: | -| Eight synchronous objects | 159 | 135 | 24 | -| Eight undefined returns | 159 | 135 | 24 | -| Mixed: six sync, two Promise | 159 | 141 | 18 | -| Eight Promises | 159 | 159 | 0 | - -Each synchronous hook also avoids one abort listener registration and removal. -Controller allocation remains unchanged. Against the original production -baseline, the synchronous-object case saves **52 Promise resources** per -navigation, including the earlier chunk/completion changes. - -The shared `isPromise` helper variant added 35 gzip bytes to React Router -minimal. The retained inline guard adds 13 bytes to the initial PR instead. -The complete patch is now **85,821 → 85,828 gzip bytes (+7 B)** in React Router -minimal. Across all 18 scenarios the complete patch adds **7–16 gzip bytes**. -This is a small size tradeoff for removing the synchronous hook wait scaffolding. -JS file counts are unchanged. Full measurements are in -`navigation-standard-final`; the initial-PR comparison is `navigation-standard-inline`. - -Fresh timing comparisons use the test-only commit `c069c4beca` as BEFORE and the -same source with the client guard as AFTER. Raw results are -`/tmp/router-navigation-investigation/standard-before.json` and `standard-after.json`. -Several means have high RME or large outliers, including unchanged control paths; -these timings are diagnostic only. CI CPU simulation is the performance criterion, -and CodSpeed memory results are excluded. - -| Case | Before mean ms | After mean ms | Before / after RME | Before / after median ms | -| ------------------- | -------------: | ------------: | -----------------: | -----------------------: | -| none beforeLoad | 0.2701 | 0.2573 | 3.46% / 1.32% | 0.2470 / 0.2400 | -| sync beforeLoad | 0.3343 | 0.2962 | 8.89% / 8.68% | 0.2973 / 0.2714 | -| void beforeLoad | 0.2942 | 0.2707 | 1.00% / 0.91% | 0.2795 / 0.2594 | -| async beforeLoad | 0.3404 | 0.3238 | 16.79% / 9.85% | 0.2910 / 0.2897 | -| mixed beforeLoad | 0.3039 | 0.2891 | 0.95% / 0.89% | 0.2886 / 0.2765 | -| chunks beforeLoad | 0.2725 | 0.3127 | 0.99% / 18.07% | 0.2595 / 0.2633 | -| blocking beforeLoad | 1.0469 | 0.8812 | 25.64% / 4.56% | 0.7606 / 0.7544 | - -
-Follow-up timing distribution - -| Case | Hz | SD ms | p99 ms | p999 ms | Samples | -| ---------------------------- | -----: | -----: | -----: | ------: | ------: | -| none beforeLoad (Before) | 3701.8 | 0.3556 | 0.5220 | 2.2237 | 5553 | -| none beforeLoad (After) | 3886.3 | 0.1327 | 0.8483 | 1.7761 | 5830 | -| sync beforeLoad (Before) | 2991.5 | 1.0154 | 1.1645 | 1.8701 | 4488 | -| sync beforeLoad (After) | 3375.7 | 0.9331 | 1.0310 | 1.2777 | 5064 | -| void beforeLoad (Before) | 3399.2 | 0.1067 | 1.0191 | 1.7078 | 5099 | -| void beforeLoad (After) | 3694.3 | 0.0941 | 1.0325 | 1.2072 | 5542 | -| async beforeLoad (Before) | 2937.3 | 1.9359 | 1.3031 | 1.9019 | 4406 | -| async beforeLoad (After) | 3088.3 | 1.1084 | 1.0832 | 1.8670 | 4638 | -| mixed beforeLoad (Before) | 3290.3 | 0.1033 | 1.0402 | 1.5141 | 4936 | -| mixed beforeLoad (After) | 3458.5 | 0.0948 | 1.0727 | 1.3303 | 5188 | -| chunks beforeLoad (Before) | 3669.2 | 0.1025 | 1.1019 | 1.4850 | 5504 | -| chunks beforeLoad (After) | 3197.8 | 2.0485 | 1.2917 | 2.0227 | 5051 | -| blocking beforeLoad (Before) | 955.2 | 5.1849 | 8.1289 | 10.1187 | 1433 | -| blocking beforeLoad (After) | 1134.8 | 0.8455 | 6.7124 | 8.6870 | 1703 | - -
- -Follow-up validation: 1,712 core tests passed with four existing expected -failures; core types passed on TypeScript 5.6–7, and ESLint passed with the same -26 existing warnings. All 33 Chromium redirect tests passed again. -The lower unit-test count reflects removal of the custom -thenable/getter cases and addition of two pending-Promise cancellation tests. - -## Initial PR changes (`220dbd9505`) - -- **Client chunk readiness:** combine the chunk task and its nested loader-outcome - continuations into one async task. Catch only component-loading failures; await - the loader outcome outside that catch. Publish readiness only after both are - settled, with the existing success, pending-state, and cancellation checks. -- **Client completion:** remove an empty trailing `.then()` from the transaction - completion chain. -- **Server SSR policies:** return static policy values directly. Functional - results are assimilated with `Promise.resolve`; synchronous functional throws - become rejected Promises so queued request cancellation still wins before - context/error hooks run. - -The initial PR left client `beforeLoad`, `waitFor`, controller ownership, and -public types unchanged. The standard-Promise follow-up below also optimizes -client `beforeLoad` while retaining its cancellation checkpoint. - -## Initial PR allocation results - -Counts are Promise resources reported by `async_hooks` around one operation; -absolute counts include harness awaits. Each lane has a root plus eight nested -routes. The same-harness deltas are the useful result. - -| Scenario | Before | After | Fewer Promises | -| ----------------------------------------------- | -----: | ----: | -------------: | -| Client, no hooks/chunks | 147 | 119 | 28 | -| Client, eight synchronous beforeLoad hooks | 187 | 159 | 28 | -| Client, eight Promise beforeLoad hooks | 187 | 159 | 28 | -| Client, mixed hooks | 187 | 159 | 28 | -| Client, asynchronous component preloads | 187 | 159 | 28 | -| Client, blocking loaders and component preloads | 275 | 247 | 28 | -| Server, static SSR policies | 68 | 50 | 18 | -| Server, synchronous functional policies | 84 | 74 | 10 | -| Server, Promise functional policies | 84 | 74 | 10 | -| Server, mixed policies | 72 | 56 | 16 | - -The client saving is three Promise resources per match plus one per navigation -in these cases. **Controller allocation is unchanged.** Client loader controllers -belong to shared flights and cached data. Server controllers support request -cancellation, boundary retirement, and deferred data cleanup. A synchronous -loader return does not make those controllers disposable. - -There is a separate server opportunity: fresh matching allocates controllers, -then server lane cloning allocates replacements. Removing that duplication would -need to distinguish private fresh matches from reused/exposed controllers; -blindly reusing the incoming controller is not safe for repeated loads. That -ownership change is not part of this patch. - -## Candidate attribution and rejected ideas - -| Independent client candidate | React minimal gzip delta | Decision | -| --------------------------------------------------------- | -----------------------: | ------------------------------------------------ | -| Skip awaiting synchronous beforeLoad results | +7 B | Rejected: queued replacement starts stale loader | -| Move chunk catch inside the async function only | -2 B | Superseded by consolidated readiness task | -| Consolidate chunk and loader readiness | -5 B | Retained | -| Remove completion's empty then | -5 B | Retained | -| Skip Promise.all for synchronous head/scripts | +18 B | Left out to limit bundle growth | -| Clean up abort listeners in existing settlement callbacks | +2 B | Rejected: direct waits slowed down | - -Removing the client await lets a hook that queues a replacement navigation -schedule stale loader work before cancellation. The original investigation also -flagged custom stateful `then` getters; those are outside the agreed standard- -Promise contract and are no longer a reason to reject the guard. The follow-up -keeps the await while bypassing the cancellable wrapper for synchronous results. - -The listener-cleanup candidate removed one Promise per wait but added callback -work. In the same 80-wait harness, mean times regressed by 3.1% for plain values, -6.6% for fulfilled Promises, and 5.8% for rejected Promises (RME 0.12–0.42%). It -was removed despite reducing allocations. Direct-wait benchmark coverage remains -available for future alternatives. - -## Initial PR bundle results - -The initial PR composition is **85,821 → 85,815 gzip bytes** in React Router minimal -(**-6 B**). Initial gzip is **85,681 → 85,676 B**; raw JS is **268,520 → 268,523 B**; -Brotli is **74,720 → 74,698 B**. Its two-file JS split is unchanged. - -Across all 18 scenarios, 16 shrink, one is unchanged, and one increases by 2 B. -Gzip deltas range from **-7 to +2 B**. There are no chunk-count changes. Gzip -changes are not additive, so independent hunk results must not be summed. - -| Scenario | Before gzip B | After gzip B | Gzip delta | Initial gzip delta | Raw delta | Brotli delta | -| -------------------------------- | ------------: | -----------: | ---------: | -----------------: | --------: | -----------: | -| react-router.minimal | 85821 | 85815 | -6 | -5 | +3 | -22 | -| react-router.full | 89430 | 89425 | -5 | -4 | +3 | +68 | -| solid-router.minimal | 33985 | 33982 | -3 | -3 | +3 | +47 | -| solid-router.full | 38927 | 38921 | -6 | -5 | +3 | +44 | -| vue-router.minimal | 50706 | 50700 | -6 | -5 | +3 | +13 | -| vue-router.full | 56457 | 56453 | -4 | -4 | +3 | -2 | -| react-start.minimal | 99061 | 99063 | +2 | +1 | +3 | -42 | -| react-start.query-integration | 106571 | 106569 | -2 | -3 | +3 | -66 | -| react-start.deferred-hydration | 99804 | 99797 | -7 | 0 | +3 | -77 | -| react-start.full | 102303 | 102301 | -2 | -3 | +3 | +23 | -| react-start.rsbuild.minimal | 102446 | 102444 | -2 | -2 | +2 | +50 | -| react-start.rsbuild.minimal-iife | 102856 | 102856 | 0 | 0 | +2 | -46 | -| react-start.rsbuild.full | 105840 | 105837 | -3 | -3 | +2 | -158 | -| solid-start.minimal | 47143 | 47138 | -5 | -5 | +3 | +89 | -| solid-start.deferred-hydration | 50295 | 50293 | -2 | -5 | +3 | +7 | -| solid-start.full | 52342 | 52341 | -1 | -4 | +3 | +31 | -| vue-start.minimal | 67241 | 67238 | -3 | 0 | +3 | -9 | -| vue-start.full | 71150 | 71147 | -3 | -4 | +3 | -57 | - -## Initial PR timing results and limits - -All times below are milliseconds per batch of ten navigations or server loads. -These are warm in-process microbenchmarks, not browser latency measurements. -The synchronous/mixed client and blocking-loader means have substantial outliers; -**do not interpret the mean deltas as a general navigation speedup or slowdown**. -For those cases, allocation counts are firmer evidence than timing. Static SSR -improved about 3% in the sampled runs with low within-run RME; that remains a -single-machine result. - -| Case | Before mean ms | After mean ms | Mean delta | Before / after RME | -| --------------------------- | -------------: | ------------: | ---------: | -----------------: | -| Client: none beforeLoad | 0.2718 | 0.2631 | -3.2% | 2.84% / 2.62% | -| Client: sync beforeLoad | 0.3549 | 0.3307 | -6.8% | 12.84% / 8.71% | -| Client: async beforeLoad | 0.3212 | 0.3063 | -4.6% | 1.03% / 1.02% | -| Client: mixed beforeLoad | 0.3152 | 0.3382 | 7.3% | 1.18% / 16.14% | -| Client: chunks beforeLoad | 0.3283 | 0.2749 | -16.3% | 18.30% / 0.99% | -| Client: blocking beforeLoad | 0.9235 | 0.9057 | -1.9% | 7.81% / 5.42% | -| Server: static server hooks | 0.1263 | 0.1226 | -2.9% | 0.30% / 0.34% | -| Server: sync server hooks | 0.1435 | 0.1381 | -3.7% | 0.47% / 0.37% | -| Server: async server hooks | 0.1406 | 0.1390 | -1.1% | 0.58% / 0.76% | -| Server: mixed server hooks | 0.1348 | 0.1334 | -1.1% | 1.13% / 1.30% | - -
-Sampling details (times in milliseconds) - -| Case | Hz | SD | Median | p99 | p999 | Samples | -| ------------------------------------ | -----: | -----: | -----: | -----: | ------: | ------: | -| Client: none beforeLoad (Before) | 3679.7 | 0.2921 | 0.2497 | 0.9792 | 1.8268 | 5520 | -| Client: none beforeLoad (After) | 3800.4 | 0.2658 | 0.2420 | 0.4923 | 1.9607 | 5703 | -| Client: sync beforeLoad (Before) | 2817.5 | 1.5111 | 0.3077 | 1.3887 | 3.0216 | 4227 | -| Client: sync beforeLoad (After) | 3024.1 | 0.9898 | 0.2955 | 1.1453 | 1.8660 | 4537 | -| Client: async beforeLoad (Before) | 3113.6 | 0.1151 | 0.3024 | 1.1680 | 1.5653 | 4671 | -| Client: async beforeLoad (After) | 3264.4 | 0.1116 | 0.2907 | 1.1553 | 1.6467 | 4897 | -| Client: mixed beforeLoad (Before) | 3172.3 | 0.1311 | 0.2977 | 1.3473 | 1.6404 | 4759 | -| Client: mixed beforeLoad (After) | 2956.9 | 1.8543 | 0.2916 | 1.3904 | 1.8401 | 4436 | -| Client: chunks beforeLoad (Before) | 3046.0 | 2.0724 | 0.2674 | 2.4513 | 2.8285 | 4569 | -| Client: chunks beforeLoad (After) | 3637.6 | 0.1026 | 0.2614 | 1.1515 | 1.3950 | 5457 | -| Client: blocking beforeLoad (Before) | 1082.8 | 1.4826 | 0.7633 | 6.9268 | 12.0854 | 1625 | -| Client: blocking beforeLoad (After) | 1104.1 | 1.0201 | 0.7586 | 8.3123 | 9.4379 | 1657 | -| Server: static server hooks (Before) | 7919.3 | 0.0208 | 0.1228 | 0.2827 | 0.3400 | 11879 | -| Server: static server hooks (After) | 8158.7 | 0.0235 | 0.1194 | 0.2931 | 0.3859 | 12238 | -| Server: sync server hooks (Before) | 6970.9 | 0.0353 | 0.1367 | 0.2933 | 0.5290 | 10457 | -| Server: sync server hooks (After) | 7241.7 | 0.0271 | 0.1336 | 0.2879 | 0.4732 | 10863 | -| Server: async server hooks (Before) | 7113.7 | 0.0433 | 0.1356 | 0.3400 | 0.4499 | 10671 | -| Server: async server hooks (After) | 7196.3 | 0.0559 | 0.1335 | 0.3129 | 0.5320 | 10795 | -| Server: mixed server hooks (Before) | 7416.7 | 0.0817 | 0.1270 | 0.2965 | 0.5972 | 11126 | -| Server: mixed server hooks (After) | 7497.8 | 0.0935 | 0.1236 | 0.2864 | 0.8237 | 11247 | - -
- -Each benchmark case samples for 1.5 seconds after 0.3 seconds of warmup. Client -and server cases batch ten operations; direct wait cases batch 80 waits on one -signal. Setup, assertions, and Promise instrumentation run outside timing. The -client harness asserts repeated chunk preload calls and includes a forced -blocking-loader case. Server cases isolate static and functional SSR policies -without beforeLoad work obscuring attribution. - -Run the three files independently through: - -`CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:unit --outputStyle=stream --skipRemoteCache -- bench tests/.bench.ts --run --outputJson=` - -Files: `navigation-awaitable`, `server-awaitable`, `navigation-wait`. - -Named bundle runs are under `benchmarks/bundle-size/results/runs/`: -`navigation-before` and `navigation-flat-full` hold the complete comparison; -`navigation-flat-alone` and `navigation-completion` isolate retained client -hunks. `navigation-final-before` repeats the original minimal bundle result. -Timing JSON and logs are in `/tmp/router-navigation-investigation/`, with final -client results named `client-final-before` / `client-final-after` and final -server results `server-before` / `server-final`. - -## Initial PR validation - -- Before: 1,717 core tests passed, four existing expected failures; the additional - queued SSR cancellation test also passed separately on the original source. -- After: 1,718 core tests passed, four existing expected failures. -- Router-core type checks: TypeScript 5.6, 5.7, 5.8, 5.9, 6, and 7 passed. -- Router-core ESLint: no errors; 26 existing warnings. -- React pending/presentation regression tests: 27 passed. -- React Router Chromium redirect tests: 33 passed. -- React Start Vite SSR head and hydration tests: three passed. -- Prettier and `git diff --check` passed. - -CI will provide the broader correctness and performance check: full unit and -E2E coverage, bundle-size comparisons, and CodSpeed CPU simulation benchmarks -for client navigation and SSR. CodSpeed memory benchmarks are excluded from -the performance assessment. diff --git a/packages/router-core/tests/navigation-awaitable.bench.ts b/packages/router-core/tests/navigation-awaitable.bench.ts deleted file mode 100644 index 4e6b2d88b3d..00000000000 --- a/packages/router-core/tests/navigation-awaitable.bench.ts +++ /dev/null @@ -1,109 +0,0 @@ -import { createHook } from 'node:async_hooks' -import { bench, describe, expect } from 'vitest' -import { createMemoryHistory } from '@tanstack/history' -import { BaseRootRoute, BaseRoute } from '../src' -import { createTestRouter } from './routerTestUtils' -import type { AnyRoute } from '../src' - -// Use the same cases on both implementations. Allocation counts are collected -// outside the timed loop so async-hooks instrumentation cannot bias timings. -for (const mode of [ - 'none', - 'sync', - 'void', - 'async', - 'mixed', - 'chunks', - 'blocking', -] as const) { - const root = new BaseRootRoute({}) - let chunkCalls = 0 - let parent: AnyRoute = root - for (let index = 0; index < 8; index++) { - const parentRoute = parent - const value = { [`level${index}`]: index } - const child = new BaseRoute({ - getParentRoute: () => parentRoute, - path: `level${index}`, - beforeLoad: - mode === 'none' || mode === 'chunks' || mode === 'blocking' - ? undefined - : () => - mode === 'async' || (mode === 'mixed' && index % 4 === 0) - ? Promise.resolve(value) - : mode === 'void' - ? undefined - : value, - component: - mode === 'chunks' || mode === 'blocking' - ? (Object.assign(() => null, { - preload: () => { - chunkCalls++ - return Promise.resolve() - }, - }) as any) - : undefined, - shouldReload: mode === 'blocking' ? true : undefined, - loader: - mode === 'blocking' - ? { - handler: () => Promise.resolve(value), - staleReloadMode: 'blocking', - } - : undefined, - }) - parent.addChildren([child]) - parent = child - } - const router = createTestRouter({ - routeTree: root, - history: createMemoryHistory({ - initialEntries: [ - '/level0/level1/level2/level3/level4/level5/level6/level7', - ], - }), - }) - await router.load() - const navigate = () => router.navigate({ to: '.', replace: true }) - await navigate() - expect(router.state.matches).toHaveLength(9) - expect( - router.state.matches.every((match) => match.status === 'success'), - ).toBe(true) - if (mode === 'chunks' || mode === 'blocking') { - expect(chunkCalls).toBe(16) - if (mode === 'blocking') { - expect(router.state.matches[8]!.loaderData).toEqual({ level7: 7 }) - } - } else if (mode !== 'none' && mode !== 'void') { - expect(router.state.matches[8]!.context).toMatchObject({ - level0: 0, - level7: 7, - }) - } - - let promises = 0 - const hook = createHook({ - init(_id, type) { - if (type === 'PROMISE') { - promises++ - } - }, - }) - hook.enable() - await navigate() - hook.disable() - console.info(`${mode}: ${promises} Promises per navigation`) - - describe(`${mode} beforeLoad`, () => { - bench( - '10 navigations through 8 routes', - async () => { - for (let index = 0; index < 10; index++) { - await navigate() - } - }, - { time: 1500, warmupTime: 300 }, - ) - }) -} diff --git a/packages/router-core/tests/navigation-wait.bench.ts b/packages/router-core/tests/navigation-wait.bench.ts deleted file mode 100644 index cc4fee656ad..00000000000 --- a/packages/router-core/tests/navigation-wait.bench.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { bench, describe, expect } from 'vitest' -import { waitFor } from '../src/load-client' - -const signal = new AbortController().signal -for (const mode of ['value', 'promise', 'rejection'] as const) { - const input = - mode === 'value' - ? 42 - : mode === 'promise' - ? Promise.resolve(42) - : Promise.reject(42) - // Consume the rejected input before registering timed cases. - if (mode === 'rejection') { - await expect(waitFor(input, signal)).rejects.toBe(42) - } else { - await expect(waitFor(input, signal)).resolves.toBe(42) - } - describe(`${mode} waits`, () => { - bench( - '80 waits on one signal', - async () => { - for (let index = 0; index < 80; index++) { - if (mode === 'rejection') { - try { - await waitFor(input, signal) - } catch { - // A rejected value still exercises listener cleanup. - } - } else { - await waitFor(input, signal) - } - } - }, - { time: 1500, warmupTime: 300 }, - ) - }) -} diff --git a/packages/router-core/tests/server-awaitable.bench.ts b/packages/router-core/tests/server-awaitable.bench.ts deleted file mode 100644 index b11fbf5a62e..00000000000 --- a/packages/router-core/tests/server-awaitable.bench.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { createHook } from 'node:async_hooks' -import { bench, describe, expect } from 'vitest' -import { createMemoryHistory } from '@tanstack/history' -import { BaseRootRoute, BaseRoute } from '../src' -import { loadServerRoute } from '../src/load-server' -import { createTestRouter } from './routerTestUtils' -import type { AnyRoute } from '../src' - -for (const mode of ['static', 'sync', 'async', 'mixed'] as const) { - const root = new BaseRootRoute({}) - let policyCalls = 0 - let parent: AnyRoute = root - for (let index = 0; index < 8; index++) { - const parentRoute = parent - const child = new BaseRoute({ - getParentRoute: () => parentRoute, - path: `level${index}`, - ssr: - mode === 'static' || (mode === 'mixed' && index % 4 !== 0) - ? true - : () => { - policyCalls++ - return mode === 'sync' ? true : Promise.resolve(true) - }, - }) - parent.addChildren([child]) - parent = child - } - const router = createTestRouter({ - routeTree: root, - history: createMemoryHistory({ - initialEntries: [ - '/level0/level1/level2/level3/level4/level5/level6/level7', - ], - }), - isServer: true, - }) - const load = () => loadServerRoute(router) - await load() - expect(router.state.matches).toHaveLength(9) - expect( - router.state.matches.every((match) => match.status === 'success'), - ).toBe(true) - expect(router.state.matches.every((match) => match.ssr === true)).toBe(true) - expect(policyCalls).toBe(mode === 'static' ? 0 : mode === 'mixed' ? 2 : 8) - - let promises = 0 - const hook = createHook({ - init(_id, type) { - if (type === 'PROMISE') { - promises++ - } - }, - }) - hook.enable() - await load() - hook.disable() - console.info(`${mode}: ${promises} Promises per server load`) - - describe(`${mode} server hooks`, () => { - bench( - '10 loads through 8 routes', - async () => { - for (let index = 0; index < 10; index++) { - await load() - } - }, - { time: 1500, warmupTime: 300 }, - ) - }) -} From f05bd737d609c9fef5006b09c53630693fcbd3db Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sun, 6 Sep 2026 14:25:33 +0200 Subject: [PATCH 7/7] refactor(router-core): return chunk failures directly --- packages/router-core/src/load-client.ts | 35 ++++++++++++++----------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/router-core/src/load-client.ts b/packages/router-core/src/load-client.ts index 24a3ae55faa..0d07ab18c5f 100644 --- a/packages/router-core/src/load-client.ts +++ b/packages/router-core/src/load-client.ts @@ -248,14 +248,14 @@ type CoordinatorRouter = AnyRouter & { type LoaderTask = [ index: number, outcome: Promise, - chunkFailure: Promise, + chunkFailure: Promise, candidate?: WorkMatch, ] type BackgroundLoaderTask = [ index: number, outcome: Promise, - chunkFailure: Promise, + chunkFailure: Promise, candidate: WorkMatch, ] @@ -435,7 +435,8 @@ async function contextualize( context: match.context, ...router.options.additionalContext, }) - // Keep the cancellation checkpoint without wrapping synchronous context. + // Always await to give a queued replacement navigation one microtask to + // kick in before checking cancellation, even for synchronous context. const result = await (typeof value?.then === 'function' ? waitFor(value, signal) : value) @@ -896,29 +897,32 @@ function createLoaderTask( ) // Keep thrown preloads and rejected chunks in the same task promise. - const chunkFailure = (async (): Promise => { - let failure: IndexedOutcome | undefined + const chunkFailure = (async (): Promise => { try { const chunk = loadRouteChunk(route, undefined, onLazyReady) if (chunk) { await waitFor(chunk, options[0 /* controller */].signal) } } catch (cause) { - failure = lane[1 /* matches */].some( - (candidate, candidateIndex) => - candidateIndex <= index && - (candidate.status === 'error' || - candidate.status === 'notFound' || - candidate._notFound), - ) - ? undefined - : [index, normalizeLaneError(router, lane, route, cause, options)] + if ( + !lane[1 /* matches */].some( + (candidate, candidateIndex) => + candidateIndex <= index && + (candidate.status === 'error' || + candidate.status === 'notFound' || + candidate._notFound), + ) + ) { + return [ + index, + normalizeLaneError(router, lane, route, cause, options), + ] satisfies IndexedOutcome + } } // Readiness requires both the component chunk and loader data. const result = await outcome if ( blocking && - !failure && result[0 /* kind */] === SUCCESS && match.status === 'pending' && !options[0 /* controller */].signal.aborted @@ -926,7 +930,6 @@ function createLoaderTask( match.status = 'success' onReady?.() } - return failure })() tasks.push([index, outcome, chunkFailure]) if (!background) {