From f969c4e87586b4b089066a2c6adf06d66960a20b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 02:12:18 +0000 Subject: [PATCH] fix: include loader identity and params in loader cache key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loader cache keys were `${entryKey}:${matchIndex}`, carrying no route identity. Whenever the same navigation entry mapped to a different route stack — a dynamic `routes` prop (feature flags, permission-dependent routes) or two instances on one page — a different route matching at the same index was served the previous route's cached loader result instead of running its own loader. Cache keys now also include the identity of the loader function (issued lazily via a WeakMap) and the matched params. The loader function is used as the identity — rather than the route definition object — because a cached result is fully determined by the loader and its args: swapping in a route variant with a different loader re-executes it, while route definitions recreated across renders keep their cached results as long as they reference the same loader function. Params are included so a shared loader reached via a different path pattern (e.g. `/:page` vs `/:section`) does not serve a result computed with different params. Fixes #207 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018waJv8AuctJWHBBWWkqTd5 --- packages/docs/src/pages/LearnLoadersPage.tsx | 15 +- .../src/__tests__/dynamicRoutes.test.tsx | 185 ++++++++++++++++++ packages/router/src/core/loaderCache.ts | 43 +++- 3 files changed, 239 insertions(+), 4 deletions(-) create mode 100644 packages/router/src/__tests__/dynamicRoutes.test.tsx diff --git a/packages/docs/src/pages/LearnLoadersPage.tsx b/packages/docs/src/pages/LearnLoadersPage.tsx index 83a4a1b..bcf4bf3 100644 --- a/packages/docs/src/pages/LearnLoadersPage.tsx +++ b/packages/docs/src/pages/LearnLoadersPage.tsx @@ -74,8 +74,9 @@ function UserDetail({ data }: { data: Promise }) {

Caching by Navigation Entry

Loader results are cached using the navigation entry ID from the - Navigation API. Each time you navigate to a new URL, the browser creates a new navigation - entry with a unique ID. The Router uses this ID as the cache key, so: + Navigation API, combined with the identity of the loader function and the + matched params. Each time you navigate to a new URL, the browser creates + a new navigation entry with a unique ID, so:

  • @@ -91,6 +92,16 @@ function UserDetail({ data }: { data: Promise }) { This design ensures that loaders run exactly once per navigation while preventing unnecessary re-fetches during React re-renders.

    + +

    Changing the Routes Prop

    +

    + Because the loader function is part of the cache key, the routes prop can + change dynamically — for example when routes depend on feature flags or user + permissions. When a route with a different loader matches the current URL + after a routes change, that loader executes fresh instead of being served another + loader’s cached data. Routes that keep the same loader function keep their cached + results, even if the route definition objects themselves are recreated. +

    diff --git a/packages/router/src/__tests__/dynamicRoutes.test.tsx b/packages/router/src/__tests__/dynamicRoutes.test.tsx new file mode 100644 index 0000000..793f07e --- /dev/null +++ b/packages/router/src/__tests__/dynamicRoutes.test.tsx @@ -0,0 +1,185 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { Router } from "../Router/index.js"; +import { Outlet } from "../Outlet.js"; +import { route } from "../route.js"; +import { setupNavigationMock, cleanupNavigationMock } from "./setup.js"; +import { clearLoaderCache } from "../core/loaderCache.js"; + +describe("Dynamic routes prop", () => { + beforeEach(() => { + setupNavigationMock("http://localhost/"); + clearLoaderCache(); + }); + + afterEach(() => { + cleanupNavigationMock(); + vi.restoreAllMocks(); + }); + + function Page({ data }: { data: { label: string } }) { + return
    Label: {data.label}
    ; + } + + it("re-executes loaders when the routes prop changes on the same entry", () => { + const loaderA = vi.fn(() => ({ label: "variant A" })); + const loaderB = vi.fn(() => ({ label: "variant B" })); + + const routesA = [route({ path: "/", component: Page, loader: loaderA })]; + const routesB = [route({ path: "/", component: Page, loader: loaderB })]; + + const { rerender } = render(); + expect(screen.getByText("Label: variant A")).toBeInTheDocument(); + + // Same URL, same navigation entry — only the routes prop changes, + // as with feature flags or permission-dependent routes. + rerender(); + expect(screen.getByText("Label: variant B")).toBeInTheDocument(); + expect(loaderB).toHaveBeenCalledTimes(1); + }); + + it("serves cached results again when switching back to previous routes", () => { + const loaderA = vi.fn(() => ({ label: "variant A" })); + const loaderB = vi.fn(() => ({ label: "variant B" })); + + const routesA = [route({ path: "/", component: Page, loader: loaderA })]; + const routesB = [route({ path: "/", component: Page, loader: loaderB })]; + + const { rerender } = render(); + rerender(); + rerender(); + + expect(screen.getByText("Label: variant A")).toBeInTheDocument(); + // Both variants' results stay cached for this entry, so flipping the + // flag back does not refetch. + expect(loaderA).toHaveBeenCalledTimes(1); + expect(loaderB).toHaveBeenCalledTimes(1); + }); + + it("does not re-execute loaders when the same routes array is re-rendered", () => { + const loader = vi.fn(() => ({ label: "stable" })); + const routes = [route({ path: "/", component: Page, loader })]; + + const { rerender } = render(); + rerender(); + rerender(); + + expect(screen.getByText("Label: stable")).toBeInTheDocument(); + expect(loader).toHaveBeenCalledTimes(1); + }); + + it("does not re-execute loaders when route objects are recreated with the same loader", () => { + const loader = vi.fn(() => ({ label: "stable loader" })); + const makeRoutes = () => [route({ path: "/", component: Page, loader })]; + + const { rerender } = render(); + // Route objects are recreated, but they reference the same loader + // function, so the cached result is reused. + rerender(); + + expect(screen.getByText("Label: stable loader")).toBeInTheDocument(); + expect(loader).toHaveBeenCalledTimes(1); + }); + + it("re-executes a shared loader when a routes change gives it different params", () => { + cleanupNavigationMock(); + setupNavigationMock("http://localhost/home"); + clearLoaderCache(); + + const loader = vi.fn(({ params }: { params: Record }) => ({ + label: Object.entries(params) + .map(([k, v]) => `${k}=${v}`) + .join(","), + })); + + const routesA = [route({ path: "/:page", component: Page, loader })]; + const routesB = [route({ path: "/:section", component: Page, loader })]; + + const { rerender } = render(); + expect(screen.getByText("Label: page=home")).toBeInTheDocument(); + + // Same loader function, but the new pattern extracts different params, + // so the cached result must not be served. + rerender(); + expect(screen.getByText("Label: section=home")).toBeInTheDocument(); + expect(loader).toHaveBeenCalledTimes(2); + }); + + it("does not serve one Router's loader data to another Router on the same page", () => { + const loaderA = vi.fn(() => ({ label: "router A" })); + const loaderB = vi.fn(() => ({ label: "router B" })); + + const routesA = [route({ path: "/", component: Page, loader: loaderA })]; + const routesB = [route({ path: "/", component: Page, loader: loaderB })]; + + // Both Routers observe the same navigation entry, so without route + // identity in the cache key their loader results would collide. + render( + <> + + + , + ); + + expect(screen.getByText("Label: router A")).toBeInTheDocument(); + expect(screen.getByText("Label: router B")).toBeInTheDocument(); + expect(loaderA).toHaveBeenCalledTimes(1); + expect(loaderB).toHaveBeenCalledTimes(1); + }); + + it("keeps cached results of route objects shared between route trees", () => { + const parentLoaderA = vi.fn(() => ({ label: "layout A" })); + const parentLoaderB = vi.fn(() => ({ label: "layout B" })); + const childLoader = vi.fn(() => ({ label: "dashboard" })); + + cleanupNavigationMock(); + setupNavigationMock("http://localhost/dashboard"); + clearLoaderCache(); + + function Layout({ data }: { data: { label: string } }) { + return ( +
    +
    Layout: {data.label}
    + +
    + ); + } + + // The same child route object is shared by both route trees. + const child = route({ + path: "dashboard", + component: Page, + loader: childLoader, + }); + const routesA = [ + route({ + path: "/", + component: Layout, + loader: parentLoaderA, + children: [child], + }), + ]; + const routesB = [ + route({ + path: "/", + component: Layout, + loader: parentLoaderB, + children: [child], + }), + ]; + + const { rerender } = render(); + expect(screen.getByText("Layout: layout A")).toBeInTheDocument(); + expect(screen.getByText("Label: dashboard")).toBeInTheDocument(); + + rerender(); + expect(screen.getByText("Layout: layout B")).toBeInTheDocument(); + expect(screen.getByText("Label: dashboard")).toBeInTheDocument(); + + // Only the parent route changed identity; the shared child route keeps + // its cached result and does not re-execute. + expect(parentLoaderA).toHaveBeenCalledTimes(1); + expect(parentLoaderB).toHaveBeenCalledTimes(1); + expect(childLoader).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/router/src/core/loaderCache.ts b/packages/router/src/core/loaderCache.ts index da29d8a..efa6513 100644 --- a/packages/router/src/core/loaderCache.ts +++ b/packages/router/src/core/loaderCache.ts @@ -13,10 +13,49 @@ export class LoaderError { /** * Cache for loader results. - * Key format: `${entryId}:${matchIndex}` + * Key format: `${entryId}:${matchIndex}:${loaderId}:${paramsKey}` */ const loaderCache = new Map(); +/** + * Identity of each loader function, issued lazily. + * + * Included in cache keys so that a cached result is only ever served for the + * loader that produced it. Without it, swapping the `routes` prop (feature + * flags, permission-dependent routes) or rendering two ``s would let + * a different route matching at the same index pick up another route's + * cached data. + * + * The loader function is used as the identity — rather than the route + * definition object — because a cached result is fully determined by the + * loader and its args. Route definitions recreated across renders keep their + * cached results as long as they reference the same loader function. + */ +type LoaderFunction = NonNullable; + +const loaderIds = new WeakMap(); +let nextLoaderId = 0; + +function getLoaderId(loader: LoaderFunction): number { + let id = loaderIds.get(loader); + if (id === undefined) { + id = nextLoaderId++; + loaderIds.set(loader, id); + } + return id; +} + +/** + * Serialize matched params into a stable cache key component. + * + * Params are part of the loader's input: the same loader function reached + * via a different path pattern (e.g. `/:page` vs `/:section`) receives + * different params for the same URL and must not share a cached result. + */ +function getParamsKey(params: Record): string { + return JSON.stringify(Object.entries(params).sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0))); +} + /** * Get or create a loader result from cache. * If the result is not cached, executes the loader and caches the result. @@ -32,7 +71,7 @@ function getOrCreateLoaderResult( return undefined; } - const cacheKey = `${entryId}:${matchIndex}`; + const cacheKey = `${entryId}:${matchIndex}:${getLoaderId(route.loader)}:${getParamsKey(args.params)}`; if (!cache.has(cacheKey)) { try {