diff --git a/docs/router/framework/react/api/router/RouterStateType.md b/docs/router/framework/react/api/router/RouterStateType.md index 6c4d2aca934..a77f4e775f2 100644 --- a/docs/router/framework/react/api/router/RouterStateType.md +++ b/docs/router/framework/react/api/router/RouterStateType.md @@ -11,7 +11,6 @@ type RouterState = { isLoading: boolean isTransitioning: boolean matches: Array - pendingMatches: Array location: ParsedLocation resolvedLocation: ParsedLocation } @@ -41,11 +40,6 @@ The `RouterState` type contains all of the properties that are available on the - Type: [`Array`](./RouteMatchType.md) - An array of all of the route matches that have been resolved and are currently active. -### `pendingMatches` property - -- Type: [`Array`](./RouteMatchType.md) -- An array of all of the route matches that are currently pending. - ### `location` property - Type: [`ParsedLocation`](./ParsedLocationType.md) diff --git a/docs/router/framework/react/api/router/useChildMatchesHook.md b/docs/router/framework/react/api/router/useChildMatchesHook.md index cf31d210160..93b6a5fc490 100644 --- a/docs/router/framework/react/api/router/useChildMatchesHook.md +++ b/docs/router/framework/react/api/router/useChildMatchesHook.md @@ -5,9 +5,6 @@ title: useChildMatches hook The `useChildMatches` hook returns all of the child [`RouteMatch`](./RouteMatchType.md) objects from the closest match down to the leaf-most match. **It does not include the current match, which can be obtained using the `useMatch` hook.** -> [!IMPORTANT] -> If the router has pending matches and they are showing their pending component fallbacks, `router.state.pendingMatches` will used instead of `router.state.matches`. - ## useChildMatches options The `useChildMatches` hook accepts a single _optional_ argument, an `options` object. diff --git a/docs/router/framework/react/api/router/useParentMatchesHook.md b/docs/router/framework/react/api/router/useParentMatchesHook.md index fe4068f3972..d22063b3ca3 100644 --- a/docs/router/framework/react/api/router/useParentMatchesHook.md +++ b/docs/router/framework/react/api/router/useParentMatchesHook.md @@ -5,9 +5,6 @@ title: useParentMatches hook The `useParentMatches` hook returns all of the parent [`RouteMatch`](./RouteMatchType.md) objects from the root down to the immediate parent of the current match in context. **It does not include the current match, which can be obtained using the `useMatch` hook.** -> [!IMPORTANT] -> If the router has pending matches and they are showing their pending component fallbacks, `router.state.pendingMatches` will used instead of `router.state.matches`. - ## useParentMatches options The `useParentMatches` hook accepts an optional `options` object. diff --git a/packages/react-router/tests/store-updates-during-navigation.test.tsx b/packages/react-router/tests/store-updates-during-navigation.test.tsx index dfb06f80d03..3a24b92eb9b 100644 --- a/packages/react-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/react-router/tests/store-updates-during-navigation.test.tsx @@ -136,8 +136,8 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBeGreaterThanOrEqual(10) // WARN: this is flaky, and sometimes (rarely) is 12 - expect(updates).toBeLessThanOrEqual(13) + expect(updates).toBeGreaterThanOrEqual(6) // WARN: this is flaky + expect(updates).toBeLessThanOrEqual(9) }) test('redirection in preload', async () => { @@ -155,7 +155,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(4) + expect(updates).toBe(3) }) test('sync beforeLoad', async () => { @@ -171,8 +171,8 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBeGreaterThanOrEqual(9) // WARN: this is flaky - expect(updates).toBeLessThanOrEqual(12) + expect(updates).toBeGreaterThanOrEqual(4) // WARN: this is flaky + expect(updates).toBeLessThanOrEqual(7) }) test('nothing', async () => { @@ -183,8 +183,8 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBeGreaterThanOrEqual(6) // WARN: this is flaky, and sometimes (rarely) is 9 - expect(updates).toBeLessThanOrEqual(9) + expect(updates).toBeGreaterThanOrEqual(2) // WARN: this is flaky + expect(updates).toBeLessThanOrEqual(5) }) test('not found in beforeLoad', async () => { @@ -199,7 +199,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(7) + expect(updates).toBe(2) }) test('hover preload, then navigate, w/ async loaders', async () => { @@ -225,7 +225,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(16) + expect(updates).toBe(8) }) test('navigate, w/ preloaded & async loaders', async () => { @@ -241,8 +241,8 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBeGreaterThanOrEqual(7) - expect(updates).toBeLessThanOrEqual(8) + expect(updates).toBeGreaterThanOrEqual(2) // WARN: this is flaky + expect(updates).toBeLessThanOrEqual(5) }) test('navigate, w/ preloaded & sync loaders', async () => { @@ -258,7 +258,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(6) + expect(updates).toBe(3) }) test('navigate, w/ previous navigation & async loader', async () => { @@ -274,7 +274,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(5) + expect(updates).toBe(3) }) test('preload a preloaded route w/ async loader', async () => { diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index 3efe8ebfee0..ac0feba5817 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -54,6 +54,12 @@ const resolvePreload = (inner: InnerLoadContext, matchId: string): boolean => { /** * Builds the accumulated context from router options and all matches up to (and optionally including) the given index. * Merges __routeContext and __beforeLoadContext from each match. + * + * Note: We use inner.matches directly (which is __pendingMatches during loading) + * rather than looking up via router.getMatch() to ensure we always get the latest + * updates from updateMatch during the loading phase. This is critical because + * __matchesById might be rebuilt by other operations (e.g., onReady commit) + * which could cause timing issues in SPA mode. */ const buildMatchContext = ( inner: InnerLoadContext, @@ -65,11 +71,9 @@ const buildMatchContext = ( } const end = includeCurrentMatch ? index : index - 1 for (let i = 0; i <= end; i++) { - const innerMatch = inner.matches[i] - if (!innerMatch) continue - const m = inner.router.getMatch(innerMatch.id) - if (!m) continue - Object.assign(context, m.__routeContext, m.__beforeLoadContext) + const match = inner.matches[i] + if (!match) continue + Object.assign(context, match.__routeContext, match.__beforeLoadContext) } return context } @@ -473,16 +477,16 @@ const executeBeforeLoad = ( handleSerialError(inner, index, beforeLoadContext, 'BEFORE_LOAD') } - batch(() => { - pending() - // Only store __beforeLoadContext here, don't update context yet - // Context will be updated in loadRouteMatch after loader completes - inner.updateMatch(matchId, (prev) => ({ - ...prev, - __beforeLoadContext: beforeLoadContext, - })) - resolve() - }) + // Execute the update synchronously to ensure the match is updated + // before the next beforeLoad runs. We avoid batch() here because + // batching reactive notifications can cause timing issues where + // the child route's beforeLoad runs before the parent's context is available. + pending() + inner.updateMatch(matchId, (prev) => ({ + ...prev, + __beforeLoadContext: beforeLoadContext, + })) + resolve() } let beforeLoadContext @@ -586,8 +590,10 @@ const getLoaderContext = ( route: AnyRoute, ): LoaderFnContext => { const parentMatchPromise = inner.matchPromises[index - 1] as any - const { params, loaderDeps, abortController, cause } = - inner.router.getMatch(matchId)! + // Use inner.matches[index] directly to ensure we get the latest updates + // from the pending matches array during loading + const match = inner.matches[index]! + const { params, loaderDeps, abortController, cause } = match const context = buildMatchContext(inner, index) @@ -866,8 +872,36 @@ export async function loadMatches(arg: { updateMatch: UpdateMatchFn sync?: boolean }): Promise> { + // Create a local matches index for O(1) lookup during loading + // This is necessary because the router's __pendingMatchesIndex may be cleared + // by other operations (like transitions or navigation) during async loading + const matchesIndex = new Map() + arg.matches.forEach((match, i) => { + matchesIndex.set(match.id, i) + }) + + // Save the original updateMatch before wrapping + const originalUpdateMatch = arg.updateMatch + + // Wrap updateMatch to update the local matches array directly + // This ensures updates are applied even if __pendingMatches is cleared + const localUpdateMatch: UpdateMatchFn = (id, updater) => { + const index = matchesIndex.get(id) + if (index !== undefined) { + const prev = arg.matches[index]! + const next = updater(prev) + if (next !== prev) { + arg.matches[index] = next + } + } + // Also call the original updateMatch to keep __matchesById in sync + // and handle any other side effects + originalUpdateMatch(id, updater) + } + const inner: InnerLoadContext = Object.assign(arg, { matchPromises: [], + updateMatch: localUpdateMatch, }) // make sure the pending component is immediately rendered when hydrating a match that is not SSRed diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index ac3c4eefefa..23995007623 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -521,7 +521,6 @@ export interface RouterState< isLoading: boolean isTransitioning: boolean matches: Array - pendingMatches?: Array cachedMatches: Array location: ParsedLocation> resolvedLocation?: ParsedLocation> @@ -935,6 +934,24 @@ export class RouterCore< viewTransitionPromise?: ControlledPromise isScrollRestoring = false isScrollRestorationSetup = false + private __pendingMatches?: Array + private __pendingMatchesIndex?: Map + /** + * Index map for committed matches (state.matches) for O(1) lookup. + */ + private __matchesIndex = new Map() + /** + * Index map for cached matches (state.cachedMatches) for O(1) lookup. + */ + private __cachedMatchesIndex = new Map() + /** + * Unified map of all matches by ID for O(1) lookup in getMatch. + * Kept in sync with __pendingMatches, state.matches, and state.cachedMatches. + */ + private __matchesById = new Map() + private __queuedMatchUpdates = new Map() + private __queuedCachedMatchUpdates = new Map() + private __matchFlushScheduled = false // Must build in constructor __store!: Store> @@ -1743,7 +1760,7 @@ export class RouterCore< (match) => match.isFetching === 'loader', ) const matchesToCancelArray = new Set([ - ...(this.state.pendingMatches ?? []), + ...(this.__pendingMatches ?? []), ...currentPendingMatches, ...currentLoadingMatches, ]) @@ -2320,6 +2337,8 @@ export class RouterCore< // Match the routes const pendingMatches = this.matchRoutes(this.latestLocation) + this.__pendingMatches = pendingMatches + this.rebuildPendingMatchesIndex(pendingMatches) // Ingest the new matches this.__store.setState((s) => ({ @@ -2328,7 +2347,6 @@ export class RouterCore< statusCode: 200, isLoading: true, location: this.latestLocation, - pendingMatches, // If a cached moved to pendingMatches, remove it from cachedMatches cachedMatches: s.cachedMatches.filter( (d) => !pendingMatches.some((e) => e.id === d.id), @@ -2370,14 +2388,13 @@ export class RouterCore< await loadMatches({ router: this, sync: opts?.sync, - matches: this.state.pendingMatches as Array, + matches: this.__pendingMatches ?? [], location: next, updateMatch: this.updateMatch, - // eslint-disable-next-line @typescript-eslint/require-await - onReady: async () => { + onReady: () => { // Wrap batch in framework-specific transition wrapper (e.g., Solid's startTransition) this.startTransition(() => { - this.startViewTransition(async () => { + this.startViewTransition(() => { // this.viewTransitionPromise = createControlledPromise() // Commit the pending matches. If a previous match was @@ -2389,7 +2406,10 @@ export class RouterCore< batch(() => { this.__store.setState((s) => { const previousMatches = s.matches - const newMatches = s.pendingMatches || s.matches + const newMatches = + this.__pendingMatches && this.__pendingMatches.length + ? this.__pendingMatches + : s.matches exitingMatches = previousMatches.filter( (match) => !newMatches.some((d) => d.id === match.id), @@ -2407,7 +2427,6 @@ export class RouterCore< isLoading: false, loadedAt: Date.now(), matches: newMatches, - pendingMatches: undefined, /** * When committing new matches, cache any exiting matches that are still usable. * Routes that resolved with `status: 'error'` or `status: 'notFound'` are @@ -2425,6 +2444,9 @@ export class RouterCore< }) this.clearExpiredCache() }) + this.__pendingMatches = undefined + this.rebuildPendingMatchesIndex(undefined) + this.rebuildMatchesById() // ;( @@ -2440,8 +2462,10 @@ export class RouterCore< ) }) }) + return Promise.resolve() }) }) + return Promise.resolve() }, }) } catch (err) { @@ -2562,34 +2586,182 @@ export class RouterCore< } } - updateMatch: UpdateMatchFn = (id, updater) => { - this.startTransition(() => { - const matchesKey = this.state.pendingMatches?.some((d) => d.id === id) - ? 'pendingMatches' - : this.state.matches.some((d) => d.id === id) - ? 'matches' - : this.state.cachedMatches.some((d) => d.id === id) - ? 'cachedMatches' - : '' - - if (matchesKey) { - this.__store.setState((s) => ({ - ...s, - [matchesKey]: s[matchesKey]?.map((d) => - d.id === id ? updater(d) : d, - ), - })) + private rebuildPendingMatchesIndex = ( + matches: Array | undefined, + ) => { + if (!matches) { + this.__pendingMatchesIndex = undefined + return + } + const index = new Map() + matches.forEach((match, i) => { + index.set(match.id, i) + // Also add to unified lookup map + this.__matchesById.set(match.id, match) + }) + this.__pendingMatchesIndex = index + } + + /** + * Rebuild the unified __matchesById map and index maps from all match sources. + * Called when state.matches or state.cachedMatches changes. + */ + private rebuildMatchesById = () => { + this.__matchesById.clear() + this.__matchesIndex.clear() + this.__cachedMatchesIndex.clear() + // Add in reverse priority order so higher priority sources overwrite + // Priority: cachedMatches < matches < pendingMatches + for (let i = 0; i < this.state.cachedMatches.length; i++) { + const match = this.state.cachedMatches[i]! + this.__matchesById.set(match.id, match) + this.__cachedMatchesIndex.set(match.id, i) + } + for (let i = 0; i < this.state.matches.length; i++) { + const match = this.state.matches[i]! + this.__matchesById.set(match.id, match) + this.__matchesIndex.set(match.id, i) + } + if (this.__pendingMatches) { + for (const match of this.__pendingMatches) { + this.__matchesById.set(match.id, match) + } + } + } + + private queueMatchUpdate = ( + kind: 'matches' | 'cachedMatches', + id: string, + match: AnyRouteMatch, + ) => { + if (kind === 'matches') { + this.__queuedMatchUpdates.set(id, match) + } else { + this.__queuedCachedMatchUpdates.set(id, match) + } + + if (this.__matchFlushScheduled) return + this.__matchFlushScheduled = true + Promise.resolve().then(() => { + this.__matchFlushScheduled = false + if ( + this.__queuedMatchUpdates.size === 0 && + this.__queuedCachedMatchUpdates.size === 0 + ) { + return } + const queuedMatches = new Map(this.__queuedMatchUpdates) + const queuedCached = new Map(this.__queuedCachedMatchUpdates) + this.__queuedMatchUpdates.clear() + this.__queuedCachedMatchUpdates.clear() + this.startTransition(() => { + this.__store.setState((s) => { + let nextMatches = s.matches + let nextCachedMatches = s.cachedMatches + + if (queuedMatches.size) { + nextMatches = s.matches.slice() + for (let i = 0; i < nextMatches.length; i++) { + const updated = queuedMatches.get(nextMatches[i]!.id) + if (updated) { + nextMatches[i] = updated + } + } + } + + if (queuedCached.size) { + nextCachedMatches = s.cachedMatches.slice() + for (let i = 0; i < nextCachedMatches.length; i++) { + const updated = queuedCached.get(nextCachedMatches[i]!.id) + if (updated) { + nextCachedMatches[i] = updated + } + } + } + + if ( + nextMatches === s.matches && + nextCachedMatches === s.cachedMatches + ) { + return s + } + + return { + ...s, + matches: nextMatches, + cachedMatches: nextCachedMatches, + } + }) + // Rebuild only indices - __matchesById is already updated in updateMatchInternal + this.rebuildIndices() + }) }) } + /** + * Rebuild only the index maps (not __matchesById which is already up-to-date). + * Called after flushMatchUpdates to sync indices with the new state arrays. + */ + private rebuildIndices = () => { + this.__matchesIndex.clear() + this.__cachedMatchesIndex.clear() + for (let i = 0; i < this.state.matches.length; i++) { + this.__matchesIndex.set(this.state.matches[i]!.id, i) + } + for (let i = 0; i < this.state.cachedMatches.length; i++) { + this.__cachedMatchesIndex.set(this.state.cachedMatches[i]!.id, i) + } + } + + private updateMatchInternal = (id: string, updater: (prev: any) => any) => { + // Check pending matches first (highest priority) - O(1) lookup + const pendingIndex = this.__pendingMatchesIndex?.get(id) + if (pendingIndex !== undefined && this.__pendingMatches) { + const prev = this.__pendingMatches[pendingIndex]! + const next = updater(prev) + if (next !== prev) { + this.__pendingMatches[pendingIndex] = next + this.__matchesById.set(id, next) + } + return + } + + // Check committed matches using O(1) index lookup + const matchesIndex = this.__matchesIndex.get(id) + if (matchesIndex !== undefined) { + // Use any pending queued update as the base, otherwise use state.matches + // This ensures multiple updates accumulate correctly before flush + const prev = + this.__queuedMatchUpdates.get(id) ?? this.state.matches[matchesIndex]! + const next = updater(prev) + if (next !== prev) { + this.__matchesById.set(id, next) + this.queueMatchUpdate('matches', id, next) + } + return + } + + // Check cached matches using O(1) index lookup + const cachedIndex = this.__cachedMatchesIndex.get(id) + if (cachedIndex !== undefined) { + // Use any pending queued update as the base, otherwise use state.cachedMatches + const prev = + this.__queuedCachedMatchUpdates.get(id) ?? + this.state.cachedMatches[cachedIndex]! + const next = updater(prev) + if (next !== prev) { + this.__matchesById.set(id, next) + this.queueMatchUpdate('cachedMatches', id, next) + } + } + } + + updateMatch: UpdateMatchFn = (id, updater) => { + this.updateMatchInternal(id, updater) + } + getMatch: GetMatchFn = (matchId: string): AnyRouteMatch | undefined => { - const findFn = (d: { id: string }) => d.id === matchId - return ( - this.state.cachedMatches.find(findFn) ?? - this.state.pendingMatches?.find(findFn) ?? - this.state.matches.find(findFn) - ) + return this.__matchesById.get(matchId) } /** @@ -2624,12 +2796,17 @@ export class RouterCore< return d } + if (this.__pendingMatches) { + this.__pendingMatches = this.__pendingMatches.map(invalidate) + this.rebuildPendingMatchesIndex(this.__pendingMatches) + } + this.__store.setState((s) => ({ ...s, matches: s.matches.map(invalidate), cachedMatches: s.cachedMatches.map(invalidate), - pendingMatches: s.pendingMatches?.map(invalidate), })) + this.rebuildMatchesById() this.shouldViewTransition = false return this.load({ sync: opts?.sync }) @@ -2689,6 +2866,7 @@ export class RouterCore< } }) } + this.rebuildMatchesById() } clearExpiredCache = () => { @@ -2734,7 +2912,7 @@ export class RouterCore< }) const activeMatchIds = new Set( - [...this.state.matches, ...(this.state.pendingMatches ?? [])].map( + [...this.state.matches, ...(this.__pendingMatches ?? [])].map( (d) => d.id, ), ) @@ -2745,6 +2923,7 @@ export class RouterCore< ]) // If the matches are already loaded, we need to add them to the cachedMatches + let addedToCache = false batch(() => { matches.forEach((match) => { if (!loadedMatchIds.has(match.id)) { @@ -2752,9 +2931,13 @@ export class RouterCore< ...s, cachedMatches: [...(s.cachedMatches as any), match], })) + addedToCache = true } }) }) + if (addedToCache) { + this.rebuildMatchesById() + } try { matches = await loadMatches({ @@ -2902,7 +3085,6 @@ export function getInitialRouterState( resolvedLocation: undefined, location, matches: [], - pendingMatches: [], cachedMatches: [], statusCode: 200, } diff --git a/packages/router-core/src/ssr/ssr-client.ts b/packages/router-core/src/ssr/ssr-client.ts index 2afbcbd17b2..14f46c24cf0 100644 --- a/packages/router-core/src/ssr/ssr-client.ts +++ b/packages/router-core/src/ssr/ssr-client.ts @@ -142,6 +142,8 @@ export async function hydrate(router: AnyRouter): Promise { matches, } }) + // Rebuild internal match lookup maps after directly setting matches + ;(router as any).rebuildMatchesById() // Allow the user to handle custom hydration data await router.options.hydrate?.(dehydratedData) diff --git a/packages/router-core/tests/load.test.ts b/packages/router-core/tests/load.test.ts index 3a3d8078007..71df6ab41b8 100644 --- a/packages/router-core/tests/load.test.ts +++ b/packages/router-core/tests/load.test.ts @@ -83,9 +83,8 @@ describe('beforeLoad skip or exec', () => { const router = setup({ beforeLoad }) const navigation = router.navigate({ to: '/foo' }) expect(beforeLoad).toHaveBeenCalledTimes(1) - expect(router.state.pendingMatches).toEqual( - expect.arrayContaining([expect.objectContaining({ id: '/foo/foo' })]), - ) + expect(router.state.status).toBe('pending') + expect(router.getMatch('/foo/foo')).toBeDefined() await navigation expect(router.state.location.pathname).toBe('/foo') expect(router.state.matches).toEqual( @@ -265,9 +264,8 @@ describe('loader skip or exec', () => { const router = setup({ loader }) const navigation = router.navigate({ to: '/foo' }) expect(loader).toHaveBeenCalledTimes(1) - expect(router.state.pendingMatches).toEqual( - expect.arrayContaining([expect.objectContaining({ id: '/foo/foo' })]), - ) + expect(router.state.status).toBe('pending') + expect(router.getMatch('/foo/foo')).toBeDefined() await navigation expect(router.state.location.pathname).toBe('/foo') expect(router.state.matches).toEqual( @@ -548,6 +546,7 @@ describe('params.parse notFound', () => { const testRoute = new BaseRoute({ getParentRoute: () => rootRoute, path: '/test/$id', + notFoundComponent: () => 'Not Found' as any, params: { parse: ({ id }: { id: string }) => { const parsed = parseInt(id, 10) @@ -566,9 +565,7 @@ describe('params.parse notFound', () => { await router.load() - const match = router.state.pendingMatches?.find( - (m) => m.routeId === testRoute.id, - ) + const match = router.state.matches.find((m) => m.routeId === testRoute.id) expect(match?.status).toBe('notFound') }) diff --git a/packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx b/packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx index c306d82d809..4e8700ff7cc 100644 --- a/packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx +++ b/packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx @@ -140,9 +140,7 @@ function RouteComp({ setActiveId: (id: string) => void }) { const styles = useStyles() - const matches = createMemo( - () => routerState().pendingMatches || routerState().matches, - ) + const matches = createMemo(() => routerState().matches) const match = createMemo(() => routerState().matches.find((d) => d.routeId === route.id), ) @@ -308,11 +306,7 @@ export const BaseTanStackRouterDevtoolsPanel = }) const activeMatch = createMemo(() => { - const matches = [ - ...(routerState().pendingMatches ?? []), - ...routerState().matches, - ...routerState().cachedMatches, - ] + const matches = [...routerState().matches, ...routerState().cachedMatches] return matches.find( (d) => d.routeId === activeId() || d.id === activeId(), ) @@ -521,10 +515,7 @@ export const BaseTanStackRouterDevtoolsPanel =
- {(routerState().pendingMatches?.length - ? routerState().pendingMatches - : routerState().matches - )?.map((match: any, _i: any) => { + {routerState().matches.map((match: any, _i: any) => { return (
State:
- {routerState().pendingMatches?.find( + {routerState().cachedMatches.find( (d: any) => d.id === activeMatch()?.id, ) - ? 'Pending' - : routerState().matches.find( - (d: any) => d.id === activeMatch()?.id, - ) - ? 'Active' - : 'Cached'} + ? 'Cached' + : 'Active'}
diff --git a/packages/router-plugin/src/core/route-hmr-statement.ts b/packages/router-plugin/src/core/route-hmr-statement.ts index b910c1340b4..867a5f6a452 100644 --- a/packages/router-plugin/src/core/route-hmr-statement.ts +++ b/packages/router-plugin/src/core/route-hmr-statement.ts @@ -29,10 +29,7 @@ function handleRouteUpdate( // TODO: how to rebuild the tree if we add a new route? walkReplaceSegmentTree(newRoute, router.processedTree.segmentTree) const filter = (m: AnyRouteMatch) => m.routeId === oldRoute.id - if ( - router.state.matches.find(filter) || - router.state.pendingMatches?.find(filter) - ) { + if (router.state.matches.find(filter)) { router.invalidate({ filter }) } function walkReplaceSegmentTree( diff --git a/packages/router-plugin/tests/add-hmr/snapshots/react/arrow-function@true.tsx b/packages/router-plugin/tests/add-hmr/snapshots/react/arrow-function@true.tsx index 5a3588a7dc8..b47916429ba 100644 --- a/packages/router-plugin/tests/add-hmr/snapshots/react/arrow-function@true.tsx +++ b/packages/router-plugin/tests/add-hmr/snapshots/react/arrow-function@true.tsx @@ -26,7 +26,7 @@ if (import.meta.hot) { router.resolvePathCache.clear(); walkReplaceSegmentTree(newRoute, router.processedTree.segmentTree); const filter = m => m.routeId === oldRoute.id; - if (router.state.matches.find(filter) || router.state.pendingMatches?.find(filter)) { + if (router.state.matches.find(filter)) { router.invalidate({ filter }); diff --git a/packages/router-plugin/tests/add-hmr/snapshots/react/function-declaration@true.tsx b/packages/router-plugin/tests/add-hmr/snapshots/react/function-declaration@true.tsx index bee310532ff..3c60039d55e 100644 --- a/packages/router-plugin/tests/add-hmr/snapshots/react/function-declaration@true.tsx +++ b/packages/router-plugin/tests/add-hmr/snapshots/react/function-declaration@true.tsx @@ -26,7 +26,7 @@ if (import.meta.hot) { router.resolvePathCache.clear(); walkReplaceSegmentTree(newRoute, router.processedTree.segmentTree); const filter = m => m.routeId === oldRoute.id; - if (router.state.matches.find(filter) || router.state.pendingMatches?.find(filter)) { + if (router.state.matches.find(filter)) { router.invalidate({ filter }); diff --git a/packages/router-plugin/tests/add-hmr/snapshots/solid/arrow-function@true.tsx b/packages/router-plugin/tests/add-hmr/snapshots/solid/arrow-function@true.tsx index 3ad31b8eb2b..bcdbb5b60b0 100644 --- a/packages/router-plugin/tests/add-hmr/snapshots/solid/arrow-function@true.tsx +++ b/packages/router-plugin/tests/add-hmr/snapshots/solid/arrow-function@true.tsx @@ -25,7 +25,7 @@ if (import.meta.hot) { router.resolvePathCache.clear(); walkReplaceSegmentTree(newRoute, router.processedTree.segmentTree); const filter = m => m.routeId === oldRoute.id; - if (router.state.matches.find(filter) || router.state.pendingMatches?.find(filter)) { + if (router.state.matches.find(filter)) { router.invalidate({ filter }); diff --git a/packages/solid-router/src/useMatch.tsx b/packages/solid-router/src/useMatch.tsx index 40a99e1487e..e4f4671eb7f 100644 --- a/packages/solid-router/src/useMatch.tsx +++ b/packages/solid-router/src/useMatch.tsx @@ -84,14 +84,11 @@ export function useMatch< ) if (match === undefined) { - // During navigation transitions, check if the match exists in pendingMatches - const pendingMatch = state.pendingMatches?.find((d: any) => - opts.from ? opts.from === d.routeId : d.id === nearestMatchId(), - ) - // Determine if we should throw an error const shouldThrowError = - !pendingMatch && !state.isTransitioning && (opts.shouldThrow ?? true) + !state.isLoading && + !state.isTransitioning && + (opts.shouldThrow ?? true) return { match: undefined, shouldThrowError } } diff --git a/packages/solid-router/tests/index.test.tsx b/packages/solid-router/tests/index.test.tsx index 0d4a837531b..cc643a6a7b2 100644 --- a/packages/solid-router/tests/index.test.tsx +++ b/packages/solid-router/tests/index.test.tsx @@ -69,7 +69,6 @@ test('index true=true', () => { // }) // const promise = router.mount() -// expect(router.store.pendingMatches[0].id).toBe('/') // await promise // expect(router.state.matches[0].id).toBe('/') @@ -92,7 +91,6 @@ test('index true=true', () => { // let promise = router.mount() -// expect(router.store.pendingMatches[0].id).toBe('/a') // await promise // expect(router.state.matches[0].id).toBe('/a') // }) @@ -122,7 +120,6 @@ test('index true=true', () => { // let promise = router.mount() -// expect(router.store.pendingMatches[1].id).toBe('/a/b') // await promise // expect(router.state.matches[1].id).toBe('/a/b') // }) @@ -144,14 +141,11 @@ test('index true=true', () => { // let promise = router.mount() -// expect(router.store.pendingMatches[0].id).toBe('/') - // await promise // expect(router.state.matches[0].id).toBe('/') // promise = router.navigate({ to: 'a' }) // expect(router.state.matches[0].id).toBe('/') -// expect(router.store.pendingMatches[0].id).toBe('a') // await promise // expect(router.state.matches[0].id).toBe('a') diff --git a/packages/solid-router/tests/store-updates-during-navigation.test.tsx b/packages/solid-router/tests/store-updates-during-navigation.test.tsx index 4534853c70d..e80079af644 100644 --- a/packages/solid-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/solid-router/tests/store-updates-during-navigation.test.tsx @@ -136,8 +136,8 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBeGreaterThanOrEqual(10) // WARN: this is flaky, and sometimes (rarely) is 12 - expect(updates).toBeLessThanOrEqual(13) + expect(updates).toBeGreaterThanOrEqual(6) // WARN: this is flaky + expect(updates).toBeLessThanOrEqual(9) }) test('redirection in preload', async () => { @@ -156,7 +156,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Solid has different update counts than React due to different reactivity - expect(updates).toBe(6) + expect(updates).toBe(4) }) test('sync beforeLoad', async () => { @@ -173,7 +173,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Solid has different update counts than React due to different reactivity - expect(updates).toBe(8) + expect(updates).toBe(4) }) test('nothing', async () => { @@ -184,8 +184,8 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBeGreaterThanOrEqual(6) // WARN: this is flaky - expect(updates).toBeLessThanOrEqual(10) + expect(updates).toBeGreaterThanOrEqual(2) // WARN: this is flaky + expect(updates).toBeLessThanOrEqual(5) }) test('not found in beforeLoad', async () => { @@ -200,7 +200,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(7) + expect(updates).toBe(2) }) test('hover preload, then navigate, w/ async loaders', async () => { @@ -226,7 +226,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(16) + expect(updates).toBe(7) }) test('navigate, w/ preloaded & async loaders', async () => { @@ -242,8 +242,8 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBeGreaterThanOrEqual(9) // WARN: this is flaky, and sometimes (rarely) is 12 - expect(updates).toBeLessThanOrEqual(13) + expect(updates).toBeGreaterThanOrEqual(2) // WARN: this is flaky + expect(updates).toBeLessThanOrEqual(5) }) test('navigate, w/ preloaded & sync loaders', async () => { @@ -260,7 +260,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Solid has one fewer update than React due to different reactivity - expect(updates).toBe(7) + expect(updates).toBe(3) }) test('navigate, w/ previous navigation & async loader', async () => { @@ -276,7 +276,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(5) + expect(updates).toBe(3) }) test('preload a preloaded route w/ async loader', async () => { @@ -294,6 +294,6 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(2) + expect(updates).toBe(1) }) }) diff --git a/packages/vue-router/src/useMatch.tsx b/packages/vue-router/src/useMatch.tsx index 0c527f66816..ba21186e085 100644 --- a/packages/vue-router/src/useMatch.tsx +++ b/packages/vue-router/src/useMatch.tsx @@ -70,57 +70,46 @@ export function useMatch< > { const nearestMatchId = opts.from ? injectDummyMatch() : injectMatch() - // Store to track pending error for deferred throwing - const pendingError = Vue.ref(null) - - // Select the match from router state - const matchSelection = useRouterState({ + // Select the match from router state, returning match and whether to throw + const matchState = useRouterState({ select: (state: any) => { const match = state.matches.find((d: any) => opts.from ? opts.from === d.routeId : d.id === nearestMatchId.value, ) if (match === undefined) { - // During navigation transitions, check if the match exists in pendingMatches - const pendingMatch = state.pendingMatches?.find((d: any) => - opts.from ? opts.from === d.routeId : d.id === nearestMatchId.value, - ) - - // If there's a pending match or we're transitioning, return undefined without throwing - if (pendingMatch || state.isTransitioning) { - pendingError.value = null - return undefined - } - - // Store the error to throw later if shouldThrow is enabled - if (opts.shouldThrow ?? true) { - pendingError.value = new Error( - `Invariant failed: Could not find ${opts.from ? `an active match from "${opts.from}"` : 'a nearest match!'}`, - ) - } + // Determine if we should throw an error + const shouldThrowError = + !state.isLoading && + !state.isTransitioning && + (opts.shouldThrow ?? true) - return undefined + return { match: undefined, shouldThrowError } } - pendingError.value = null - return opts.select ? opts.select(match) : match + return { + match: opts.select ? opts.select(match) : match, + shouldThrowError: false, + } }, - } as any) + }) - // Throw the error if we have one - this happens after the selector runs - // Using a computed so the error is thrown when the return value is accessed + // Computed that throws when appropriate const result = Vue.computed(() => { - // Check for pending error first - if (pendingError.value) { - throw pendingError.value + const state = matchState.value + if (state.shouldThrowError) { + throw new Error( + `Invariant failed: Could not find ${opts.from ? `an active match from "${opts.from}"` : 'a nearest match!'}`, + ) } - return matchSelection.value + return state.match }) - // Also immediately throw if there's already an error from initial render - // This ensures errors are thrown even if the returned ref is never accessed - if (pendingError.value) { - throw pendingError.value + // Also immediately check for initial render throw + if (matchState.value.shouldThrowError) { + throw new Error( + `Invariant failed: Could not find ${opts.from ? `an active match from "${opts.from}"` : 'a nearest match!'}`, + ) } return result as any diff --git a/packages/vue-router/tests/index.test.tsx b/packages/vue-router/tests/index.test.tsx index edaf1f2fcc0..0d40b5f082a 100644 --- a/packages/vue-router/tests/index.test.tsx +++ b/packages/vue-router/tests/index.test.tsx @@ -69,7 +69,6 @@ test('index true=true', () => { // }) // const promise = router.mount() -// expect(router.store.pendingMatches[0].id).toBe('/') // await promise // expect(router.state.matches[0].id).toBe('/') @@ -92,7 +91,6 @@ test('index true=true', () => { // let promise = router.mount() -// expect(router.store.pendingMatches[0].id).toBe('/a') // await promise // expect(router.state.matches[0].id).toBe('/a') // }) @@ -122,7 +120,6 @@ test('index true=true', () => { // let promise = router.mount() -// expect(router.store.pendingMatches[1].id).toBe('/a/b') // await promise // expect(router.state.matches[1].id).toBe('/a/b') // }) @@ -144,14 +141,11 @@ test('index true=true', () => { // let promise = router.mount() -// expect(router.store.pendingMatches[0].id).toBe('/') - // await promise // expect(router.state.matches[0].id).toBe('/') // promise = router.navigate({ to: 'a' }) // expect(router.state.matches[0].id).toBe('/') -// expect(router.store.pendingMatches[0].id).toBe('a') // await promise // expect(router.state.matches[0].id).toBe('a') diff --git a/packages/vue-router/tests/store-updates-during-navigation.test.tsx b/packages/vue-router/tests/store-updates-during-navigation.test.tsx index 6cf44c37257..c33598c1e25 100644 --- a/packages/vue-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/vue-router/tests/store-updates-during-navigation.test.tsx @@ -138,7 +138,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(27) + expect(updates).toBe(18) }) test('redirection in preload', async () => { @@ -157,7 +157,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(10) + expect(updates).toBe(6) }) test('sync beforeLoad', async () => { @@ -174,7 +174,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(25) + expect(updates).toBe(12) }) test('nothing', async () => { @@ -187,8 +187,8 @@ describe("Store doesn't update *too many* times during navigation", () => { // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity // Vue's reactivity model may cause slightly more updates due to computed refs - expect(updates).toBeGreaterThanOrEqual(14) // WARN: this is flaky - expect(updates).toBeLessThanOrEqual(26) + expect(updates).toBeGreaterThanOrEqual(6) // WARN: this is flaky + expect(updates).toBeLessThanOrEqual(14) }) test('not found in beforeLoad', async () => { @@ -204,7 +204,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(22) + expect(updates).toBe(7) }) test('hover preload, then navigate, w/ async loaders', async () => { @@ -231,7 +231,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(38) + expect(updates).toBe(17) }) test('navigate, w/ preloaded & async loaders', async () => { @@ -248,7 +248,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(18) + expect(updates).toBe(6) }) test('navigate, w/ preloaded & sync loaders', async () => { @@ -265,7 +265,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(16) + expect(updates).toBe(6) }) test('navigate, w/ previous navigation & async loader', async () => { @@ -282,7 +282,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(12) + expect(updates).toBe(6) }) test('preload a preloaded route w/ async loader', async () => {