Fix hash issue for matchRoutes optimization

remix-run · Jan 31, 2025 · 5fc88dd · 5fc88dd
1 parent 980add6
commit 5fc88dd
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 16 deletions.
diff --git a/.changeset/witty-birds-fetch.md b/.changeset/witty-birds-fetch.md
@@ -0,0 +1,5 @@
+---
+"react-router": patch
+---
+
+Fix regression introduced in `7.1.4` via [#12800](https://github.com/remix-run/react-router/pull/12800) that caused issues navigating to hash routes inside splat routes for applications using Lazy Route Discovery (`patchRoutesOnNavigation`)
diff --git a/packages/react-router/__tests__/router/navigation-test.ts b/packages/react-router/__tests__/router/navigation-test.ts
@@ -1,3 +1,5 @@
+import { JSDOM } from "jsdom";
+import { createBrowserRouter } from "../../lib/dom/lib";
 import type { HydrationState } from "../../lib/router/router";
 import { cleanup, setup } from "./utils/data-router-setup";
 import { createFormData } from "./utils/utils";
@@ -60,6 +62,13 @@ function initializeTest(init?: {
   });
 }
 
+function getWindowImpl(initialUrl: string, isHash = false): Window {
+  // Need to use our own custom DOM in order to get a working history
+  const dom = new JSDOM(`<!DOCTYPE html>`, { url: "http://localhost/" });
+  dom.window.history.replaceState(null, "", (isHash ? "#" : "") + initialUrl);
+  return dom.window as unknown as Window;
+}
+
 describe("navigations", () => {
   afterEach(() => cleanup());
 
@@ -431,6 +440,49 @@ describe("navigations", () => {
       });
     });
 
+    it("does not use fog of war partial matches for hash change only navigations", async () => {
+      let router = createBrowserRouter(
+        [
+          {
+            path: "/",
+            children: [
+              {
+                path: "*",
+              },
+            ],
+          },
+        ],
+        {
+          window: getWindowImpl("/"),
+          // This is what enables the partialMatches logic
+          patchRoutesOnNavigation: () => {},
+        }
+      );
+      expect(router.state.location).toMatchObject({
+        pathname: "/",
+        hash: "",
+      });
+      expect(router.state.matches).toMatchObject([{ route: { path: "/" } }]);
+      await router.navigate("/foo");
+      expect(router.state.location).toMatchObject({
+        pathname: "/foo",
+        hash: "",
+      });
+      expect(router.state.matches).toMatchObject([
+        { route: { path: "/" } },
+        { route: { path: "*" } },
+      ]);
+      await router.navigate("/foo#bar");
+      expect(router.state.location).toMatchObject({
+        pathname: "/foo",
+        hash: "#bar",
+      });
+      expect(router.state.matches).toMatchObject([
+        { route: { path: "/" } },
+        { route: { path: "*" } },
+      ]);
+    });
+
     it("redirects from loaders (throw)", async () => {
       let t = initializeTest();
 

diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts
@@ -1533,6 +1533,23 @@ export function createRouter(init: RouterInit): Router {
         : matchRoutes(routesToUse, location, basename);
     let flushSync = (opts && opts.flushSync) === true;
 
+    // Short circuit if it's only a hash change and not a revalidation or
+    // mutation submission.
+    //
+    // Ignore on initial page loads because since the initial hydration will always
+    // be "same hash".  For example, on /page#hash and submit a <Form method="post">
+    // which will default to a navigation to /page
+    if (
+      matches &&
+      state.initialized &&
+      !isRevalidationRequired &&
+      isHashChangeOnly(state.location, location) &&
+      !(opts && opts.submission && isMutationMethod(opts.submission.formMethod))
+    ) {
+      completeNavigation(location, { matches }, { flushSync });
+      return;
+    }
+
     let fogOfWar = checkFogOfWar(matches, routesToUse, location.pathname);
     if (fogOfWar.active && fogOfWar.matches) {
       matches = fogOfWar.matches;
@@ -1557,22 +1574,6 @@ export function createRouter(init: RouterInit): Router {
       return;
     }
 
-    // Short circuit if it's only a hash change and not a revalidation or
-    // mutation submission.
-    //
-    // Ignore on initial page loads because since the initial hydration will always
-    // be "same hash".  For example, on /page#hash and submit a <Form method="post">
-    // which will default to a navigation to /page
-    if (
-      state.initialized &&
-      !isRevalidationRequired &&
-      isHashChangeOnly(state.location, location) &&
-      !(opts && opts.submission && isMutationMethod(opts.submission.formMethod))
-    ) {
-      completeNavigation(location, { matches }, { flushSync });
-      return;
-    }
-
     // Create a controller/Request for this navigation
     pendingNavigationController = new AbortController();
     let request = createClientSideRequest(