diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cb8da2..17ced44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Abort a preload joined by navigation when the router stops so a late loader result cannot replace the next active match. +- Let a pending navigation finish when invalidate or revalidate runs without a route id, so the destination stays active instead of leaving history and router state split. - Reject loader redirect cycles and chains longer than 10 redirects during navigation and preload; even terminating chains above the limit now fail. Thanks @SebTardif. - Restore strict dependency engine checks with pnpm 11 so unsupported development installs fail early. Thanks @dependabot. - Ignore cancelled preload failures and redirects after stopping so late results cannot start new loads or remove replacement cached matches. diff --git a/src/router.ts b/src/router.ts index 353d92a..57ccd05 100644 --- a/src/router.ts +++ b/src/router.ts @@ -170,6 +170,26 @@ export function createRouter< }); }; + const cacheStoredMatch = (candidate: RouteMatch | undefined): void => { + if (!candidate) { + return; + } + // Invalidation can replace the active match while a destination loads. + // Cache the store's latest state, never the navigation's earlier snapshot. + const current = matches.getMatch(candidate.id); + if (!current || !canCacheMatch(current)) { + return; + } + matches.setCached([ + ...matches.getState().cachedMatches.filter((match) => match.id !== current.id), + current, + ]); + const route = compiled.byId.get(current.routeId); + if (route) { + loading.scheduleGc(current, route); + } + }; + const navigate = async ( routeId: TRouteId, context: TLoadContext, @@ -282,16 +302,7 @@ export function createRouter< if (activatedCachedMatch) { matches.batch(() => { - if (previous && canCacheMatch(previous)) { - matches.setCached([ - ...matches.getState().cachedMatches.filter((candidate) => candidate.id !== previous.id), - previous, - ]); - const previousRoute = compiled.byId.get(previous.routeId); - if (previousRoute) { - loading.scheduleGc(previous, previousRoute); - } - } + cacheStoredMatch(previous); matches.setActive([activatedCachedMatch]); matches.setPending([]); matches.setLocation(location, location); @@ -326,18 +337,7 @@ export function createRouter< } targetPublished = true; matches.batch(() => { - if (previous && canCacheMatch(previous)) { - matches.setCached([ - ...matches - .getState() - .cachedMatches.filter((candidate) => candidate.id !== previous.id), - previous, - ]); - const previousRoute = compiled.byId.get(previous.routeId); - if (previousRoute) { - loading.scheduleGc(previous, previousRoute); - } - } + cacheStoredMatch(previous); matches.setActive([{ ...loadedMatch, module }]); matches.setPending([]); matches.setLocation(location, location); @@ -377,12 +377,8 @@ export function createRouter< if (failedMatch) { const currentActive = targetPublished ? previous : matches.getActiveMatch(); matches.batch(() => { - if (!targetPublished && !sameMatch && currentActive && canCacheMatch(currentActive)) { - matches.setCached([...matches.getState().cachedMatches, currentActive]); - const currentRoute = compiled.byId.get(currentActive.routeId); - if (currentRoute) { - loading.scheduleGc(currentActive, currentRoute); - } + if (!targetPublished && !sameMatch) { + cacheStoredMatch(currentActive); } matches.updateMatch(match.id, (current) => ({ ...current, @@ -425,12 +421,8 @@ export function createRouter< }; const currentActive = targetPublished ? previous : matches.getActiveMatch(); matches.batch(() => { - if (!targetPublished && !sameMatch && currentActive && canCacheMatch(currentActive)) { - matches.setCached([...matches.getState().cachedMatches, currentActive]); - const currentRoute = compiled.byId.get(currentActive.routeId); - if (currentRoute) { - loading.scheduleGc(currentActive, currentRoute); - } + if (!targetPublished && !sameMatch) { + cacheStoredMatch(currentActive); } matches.setActive([resolvedMatch]); matches.setPending([]); @@ -589,6 +581,11 @@ export function createRouter< : Promise.resolve(); }; + const hasPendingNavigationToDifferentMatch = (matchId: string): boolean => { + const run = currentRun; + return run !== null && !run.controller.signal.aborted && run.matchId !== matchId; + }; + return { routes: [...compiled.byId.values()], getRoute: (routeId: TRouteId) => compiled.byId.get(routeId) ?? null, @@ -601,7 +598,8 @@ export function createRouter< if ( !active || (routeId !== undefined && active.routeId !== routeId) || - !lastContext.hasContext + !lastContext.hasContext || + (routeId === undefined && hasPendingNavigationToDifferentMatch(active.id)) ) { return Promise.resolve(); } @@ -634,15 +632,22 @@ export function createRouter< navigateLocation(location: RouteLocation, context: TLoadContext): Promise { return handleLocation(location, context); }, - revalidate(context: TLoadContext, routeId = matches.getActiveMatch()?.routeId): Promise { - if (!routeId) { + revalidate(context: TLoadContext, routeId?: TRouteId): Promise { + const active = matches.getActiveMatch(); + const targetRouteId = routeId ?? active?.routeId; + if ( + !targetRouteId || + (routeId === undefined && + active !== undefined && + hasPendingNavigationToDifferentMatch(active.id)) + ) { return Promise.resolve(); } const target = - matches.getActiveMatch()?.routeId === routeId - ? matches.getActiveMatch()?.location - : locationForPath(compiled.pathForRoute(routeId, basePath)); - return navigate(routeId, context, { history: "none", revalidate: true }, target); + active?.routeId === targetRouteId + ? active.location + : locationForPath(compiled.pathForRoute(targetRouteId, basePath)); + return navigate(targetRouteId, context, { history: "none", revalidate: true }, target); }, stop() { stopHistory?.(); diff --git a/test/router-invalidate-pending.test.ts b/test/router-invalidate-pending.test.ts new file mode 100644 index 0000000..4392ef1 --- /dev/null +++ b/test/router-invalidate-pending.test.ts @@ -0,0 +1,238 @@ +import { describe, expect, it } from "vitest"; +import { createRouter, type RouteLocation, type RouterHistory } from "../src/index"; + +type RouteId = "a" | "b"; +type HistoryOperation = { + type: "push" | "replace"; + location: RouteLocation; +}; +type MemoryHistory = RouterHistory & { + operations: HistoryOperation[]; + emit: (location: RouteLocation) => void; +}; + +function location(pathname: string, search = "", hash = ""): RouteLocation { + return { pathname, search, hash }; +} + +function createMemoryHistory(initial: RouteLocation): MemoryHistory { + let current = initial; + const listeners = new Set<(location: RouteLocation) => void>(); + const operations: HistoryOperation[] = []; + return { + operations, + location: () => current, + push(nextLocation) { + current = nextLocation; + operations.push({ type: "push", location: nextLocation }); + }, + replace(nextLocation) { + current = nextLocation; + operations.push({ type: "replace", location: nextLocation }); + }, + listen(listener) { + listeners.add(listener); + return () => listeners.delete(listener); + }, + emit(nextLocation) { + current = nextLocation; + for (const listener of listeners) { + listener(nextLocation); + } + }, + }; +} + +function createPendingDestination() { + let finishLoader!: () => void; + const loaderGate = new Promise((resolve) => { + finishLoader = resolve; + }); + let finishComponent!: () => void; + const componentGate = new Promise((resolve) => { + finishComponent = resolve; + }); + const router = createRouter({ + routes: [ + { + id: "a", + path: "/a", + component: () => "view-a", + loader: () => "data-a", + }, + { + id: "b", + path: "/b", + component: () => componentGate.then(() => "view-b"), + loader: () => loaderGate.then(() => "data-b"), + }, + ], + }); + const history = createMemoryHistory(location("/a")); + return { router, history, finishLoader, finishComponent }; +} + +function expectDestinationB( + history: MemoryHistory, + router: ReturnType["router"], +): void { + expect(history.location().pathname).toBe("/b"); + expect(router.getState().location.pathname).toBe("/b"); + expect(router.getState().matches[0]?.routeId).toBe("b"); + expect(router.getState().status).toBe("success"); + expect(router.getState().matches[0]?.data).toBe("data-b"); +} + +function createPendingMatchDestination() { + let finishLoader!: () => void; + const loaderGate = new Promise((resolve) => { + finishLoader = resolve; + }); + const router = createRouter<"chat", string, string, string>({ + routes: [ + { + id: "chat", + path: "/chat", + component: () => "view-chat", + loaderDeps: (_context, routeLocation) => routeLocation.search, + loader: (_context, options) => + options.deps === "?session=two" + ? loaderGate.then(() => `data-${options.deps}`) + : `data-${options.deps}`, + }, + ], + }); + const history = createMemoryHistory(location("/chat", "?session=one")); + return { router, history, finishLoader }; +} + +function expectSecondChatMatch( + history: MemoryHistory, + router: ReturnType["router"], + firstMatchId: string, +): void { + expect(history.location()).toEqual(location("/chat", "?session=two")); + expect(router.getState().location).toEqual(location("/chat", "?session=two")); + expect(router.getState().pendingMatches).toEqual([]); + expect(router.getState().matches[0]).toMatchObject({ + routeId: "chat", + status: "success", + data: "data-?session=two", + }); + expect(router.getState().matches[0]?.id).not.toBe(firstMatchId); +} + +describe("invalidate during a pending navigation", () => { + it("lets the pending route finish when invalidate is called without a route id", async () => { + const { router, history, finishLoader, finishComponent } = createPendingDestination(); + await router.start(history, "", "ctx"); + const navigation = router.navigate("b", "ctx", { history: "push" }); + + await router.invalidate(); + finishComponent(); + finishLoader(); + await navigation; + + expectDestinationB(history, router); + router.stop(); + }); + + it("lets the pending route finish when revalidate is called without a route id", async () => { + const { router, history, finishLoader, finishComponent } = createPendingDestination(); + await router.start(history, "", "ctx"); + const navigation = router.navigate("b", "ctx", { history: "push" }); + + await router.revalidate("ctx"); + finishComponent(); + finishLoader(); + await navigation; + + expectDestinationB(history, router); + router.stop(); + }); + + it("lets a pending match of the active route finish when invalidate has no route id", async () => { + const { router, history, finishLoader } = createPendingMatchDestination(); + await router.start(history, "", "ctx"); + const firstMatchId = router.getState().matches[0]?.id; + if (!firstMatchId) { + throw new Error("expected an active match"); + } + const navigation = router.navigate( + "chat", + "ctx", + { history: "push" }, + location("/chat", "?session=two"), + ); + + await router.invalidate(); + finishLoader(); + await navigation; + + expectSecondChatMatch(history, router, firstMatchId); + router.stop(); + }); + + it("lets a pending match of the active route finish when revalidate has no route id", async () => { + const { router, history, finishLoader } = createPendingMatchDestination(); + await router.start(history, "", "ctx"); + const firstMatchId = router.getState().matches[0]?.id; + if (!firstMatchId) { + throw new Error("expected an active match"); + } + const navigation = router.navigate( + "chat", + "ctx", + { history: "push" }, + location("/chat", "?session=two"), + ); + + await router.revalidate("ctx"); + finishLoader(); + await navigation; + + expectSecondChatMatch(history, router, firstMatchId); + router.stop(); + }); + + it("preserves invalidation when the previous active match moves into cache", async () => { + let loadACount = 0; + let finishComponentB!: () => void; + const componentBGate = new Promise((resolve) => { + finishComponentB = resolve; + }); + const router = createRouter({ + routes: [ + { + id: "a", + path: "/a", + staleTime: 60_000, + component: () => "view-a", + loader: () => `data-a-${++loadACount}`, + }, + { + id: "b", + path: "/b", + component: () => componentBGate.then(() => "view-b"), + loader: () => "data-b", + }, + ], + }); + const history = createMemoryHistory(location("/a")); + await router.start(history, "", "ctx"); + const navigation = router.navigate("b", "ctx", { history: "push" }); + + await router.invalidate(); + finishComponentB(); + await navigation; + + expectDestinationB(history, router); + await router.navigate("a", "ctx", { history: "push" }); + expect(loadACount).toBe(2); + expect(router.getState().matches[0]).toMatchObject({ + routeId: "a", + data: "data-a-2", + }); + router.stop(); + }); +});