fix: use entry.id as cache key

Author: uhyo

packages/router/src/Router.tsx

@@ -49,17 +49,18 @@ export function Router({ routes, children }: RouterProps): ReactNode {
 
   // Match current URL against routes and execute loaders
   const currentUrl = currentEntry?.url;
+  const currentEntryId = currentEntry?.id;
   const matchedRoutesWithData = useMemo(() => {
-    if (!currentUrl) return null;
+    if (!currentUrl || !currentEntryId) return null;
     const url = new URL(currentUrl);
     const matched = matchRoutes(routes, url.pathname);
     if (!matched) return null;
 
-    // Execute loaders (results are cached by URL pathname)
+    // Execute loaders (results are cached by navigation entry id)
     const request = createLoaderRequest(url);
     const signal = getIdleAbortSignal();
-    return executeLoaders(matched, url.pathname, request, signal);
-  }, [currentUrl, routes]);
+    return executeLoaders(matched, currentEntryId, request, signal);
+  }, [currentUrl, currentEntryId, routes]);
 
   const routerContextValue = useMemo(
     () => ({ currentEntry, navigate }),

packages/router/src/__tests__/loader.test.tsx

@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
-import { render, screen, act, waitFor } from "@testing-library/react";
+import { render, screen, act } from "@testing-library/react";
 import { Router } from "../Router.js";
 import { Outlet } from "../Outlet.js";
 import { route } from "../route.js";
@@ -298,7 +298,7 @@ describe("Data Loader", () => {
       expect(screen.getByText(/Count:/).textContent).toBe(firstCount);
     });
 
-    it("caches results per URL pathname", () => {
+    it("caches results per navigation entry id", () => {
       mockNavigation = setupNavigationMock("http://localhost/page1");
 
       const loaderSpy = vi.fn(({ params }: LoaderArgs) => ({
@@ -321,22 +321,60 @@ describe("Data Loader", () => {
       expect(loaderSpy).toHaveBeenCalledTimes(1);
       expect(screen.getByText("Page: page1")).toBeInTheDocument();
 
-      // Navigate to a different page
+      // Navigate to a different page (creates new entry at index 1)
       act(() => {
         mockNavigation.__simulateNavigation("http://localhost/page2");
       });
 
-      // Loader should be called again for the new URL
+      // Loader should be called again for the new entry
       expect(loaderSpy).toHaveBeenCalledTimes(2);
       expect(screen.getByText("Page: page2")).toBeInTheDocument();
 
-      // Navigate back to the first page
+      // Traverse back to the first entry (back button behavior)
       act(() => {
-        mockNavigation.__simulateNavigation("http://localhost/page1");
+        mockNavigation.__simulateTraversal(0);
       });
 
-      // Loader should NOT be called again (cache hit)
+      // Loader should NOT be called again (cache hit - same entry id)
+      expect(loaderSpy).toHaveBeenCalledTimes(2);
+      expect(screen.getByText("Page: page1")).toBeInTheDocument();
+    });
+
+    it("calls loader again for new navigation to same URL", () => {
+      mockNavigation = setupNavigationMock("http://localhost/page1");
+
+      const loaderSpy = vi.fn(({ params }: LoaderArgs) => ({
+        page: params.page,
+      }));
+
+      function Page({ data }: { data: { page: string } }) {
+        return <div>Page: {data.page}</div>;
+      }
+
+      const routes = [
+        route({
+          path: "/:page",
+          component: Page,
+          loader: loaderSpy,
+        }),
+      ];
+
+      render(<Router routes={routes} />);
+      expect(loaderSpy).toHaveBeenCalledTimes(1);
+
+      // Navigate to a different page
+      act(() => {
+        mockNavigation.__simulateNavigation("http://localhost/page2");
+      });
       expect(loaderSpy).toHaveBeenCalledTimes(2);
+
+      // Navigate forward to the same URL as the first page (creates new entry)
+      act(() => {
+        mockNavigation.__simulateNavigation("http://localhost/page1");
+      });
+
+      // Loader SHOULD be called again (new entry, different id)
+      expect(loaderSpy).toHaveBeenCalledTimes(3);
       expect(screen.getByText("Page: page1")).toBeInTheDocument();
     });
   });

packages/router/src/__tests__/setup.ts

@@ -93,6 +93,17 @@ export function createMockNavigation(initialUrl = "http://localhost/") {
       mockNavigation.navigate(url, { state });
     },
 
+    // Test helper to simulate traverse navigation (back/forward)
+    // This reuses an existing entry instead of creating a new one
+    __simulateTraversal(entryIndex: number) {
+      if (entryIndex < 0 || entryIndex >= entries.length) {
+        throw new Error(`Invalid entry index: ${entryIndex}`);
+      }
+      currentEntry = entries[entryIndex];
+      mockNavigation.currentEntry = currentEntry;
+      dispatchEvent("currententrychange", new Event("currententrychange"));
+    },
+
     // Test helper to get listeners
     __getListeners(type: string) {
       return listeners.get(type);

packages/router/src/core/loaderCache.ts

@@ -7,7 +7,7 @@ import type {
 
 /**
  * Cache for loader results.
- * Key format: `${entryKey}:${routePath}`
+ * Key format: `${entryId}:${routePath}`
  */
 const loaderCache = new Map<string, unknown>();
 
@@ -16,7 +16,7 @@ const loaderCache = new Map<string, unknown>();
  * If the result is not cached, executes the loader and caches the result.
  */
 function getOrCreateLoaderResult(
-  entryKey: string,
+  entryId: string,
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   route: RouteDefinition<any>,
   args: LoaderArgs,
@@ -25,7 +25,7 @@ function getOrCreateLoaderResult(
     return undefined;
   }
 
-  const cacheKey = `${entryKey}:${route.path}`;
+  const cacheKey = `${entryId}:${route.path}`;
 
   if (!loaderCache.has(cacheKey)) {
     loaderCache.set(cacheKey, route.loader(args));
@@ -45,18 +45,18 @@ export function createLoaderRequest(url: URL): Request {
 
 /**
  * Execute loaders for matched routes and return routes with data.
- * Results are cached by navigation entry key to prevent duplicate execution.
+ * Results are cached by navigation entry id to prevent duplicate execution.
  */
 export function executeLoaders(
   matchedRoutes: MatchedRoute[],
-  entryKey: string,
+  entryId: string,
   request: Request,
   signal: AbortSignal,
 ): MatchedRouteWithData[] {
   return matchedRoutes.map((match) => {
     const { route, params } = match;
     const args: LoaderArgs = { params, request, signal };
-    const data = getOrCreateLoaderResult(entryKey, route, args);
+    const data = getOrCreateLoaderResult(entryId, route, args);
 
     return { ...match, data };
   });

packages/router/src/core/navigation.ts

@@ -97,8 +97,15 @@ export function setupNavigationInterception(
       const request = createLoaderRequest(url);
 
       // Execute loaders immediately (before React re-renders)
-      // Results are cached, so React render will hit the cache
-      executeLoaders(matched, url.pathname, request, event.signal);
+      // Results are cached by entry id, so React render will hit the cache
+      // Note: destination.id is only available for traverse navigations (back/forward)
+      // For push/replace navigations, destination.id is empty and cache won't be shared
+      executeLoaders(
+        matched,
+        event.destination.id ?? "",
+        request,
+        event.signal,
+      );
 
       event.intercept({
         handler: async () => {