Fix up same-hash link short circuiting logic (#10408)

Author: brophdawg11

.changeset/same-hash-links.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+"Same hash" navigations no longer re-run loaders to match browser behavior (i.e. `/path#hash -> /path#hash`)

packages/router/__tests__/router-test.ts

@@ -1476,14 +1476,46 @@ describe("a router", () => {
       });
     });
 
-    it("does not load anything on hash change only <Link> navigations", async () => {
+    it("does not run loaders on hash change only navigations", async () => {
       let t = initializeTmTest();
       expect(t.router.state.loaderData).toMatchObject({ root: "ROOT" });
       let A = await t.navigate("/#bar");
       expect(A.loaders.root.stub.mock.calls.length).toBe(0);
       expect(t.router.state.loaderData).toMatchObject({ root: "ROOT" });
     });
 
+    it("does not run loaders on same-hash navigations", async () => {
+      let t = initializeTmTest({ url: "/#bar" });
+      expect(t.router.state.loaderData).toMatchObject({ root: "ROOT" });
+      let A = await t.navigate("/#bar");
+      expect(A.loaders.root.stub.mock.calls.length).toBe(0);
+      expect(A.loaders.index.stub.mock.calls.length).toBe(0);
+    });
+
+    it("runs loaders on same-hash navigations to new paths", async () => {
+      let t = initializeTmTest({ url: "/#bar" });
+      expect(t.router.state.loaderData).toMatchObject({ root: "ROOT" });
+      let A = await t.navigate("/foo#bar");
+      expect(A.loaders.root.stub.mock.calls.length).toBe(0);
+      expect(A.loaders.foo.stub.mock.calls.length).toBe(1);
+    });
+
+    it("runs loaders on hash removal navigations (same path)", async () => {
+      let t = initializeTmTest({ url: "/#bar" });
+      expect(t.router.state.loaderData).toMatchObject({ root: "ROOT" });
+      let A = await t.navigate("/");
+      expect(A.loaders.root.stub.mock.calls.length).toBe(1);
+      expect(A.loaders.index.stub.mock.calls.length).toBe(1);
+    });
+
+    it("runs loaders on hash removal navigations (nested path)", async () => {
+      let t = initializeTmTest({ url: "/#bar" });
+      expect(t.router.state.loaderData).toMatchObject({ root: "ROOT" });
+      let A = await t.navigate("/foo");
+      expect(A.loaders.root.stub.mock.calls.length).toBe(0);
+      expect(A.loaders.foo.stub.mock.calls.length).toBe(1);
+    });
+
     it('does not load anything on hash change only empty <Form method="get"> navigations', async () => {
       let t = initializeTmTest();
       expect(t.router.state.loaderData).toMatchObject({ root: "ROOT" });

packages/router/router.ts

@@ -3266,7 +3266,8 @@ function getMatchesToLoad(
         // Forced revalidation due to submission, useRevalidator, or X-Remix-Revalidate
         isRevalidationRequired ||
         // Clicked the same link, resubmitted a GET form
-        currentUrl.toString() === nextUrl.toString() ||
+        currentUrl.pathname + currentUrl.search ===
+          nextUrl.pathname + nextUrl.search ||
         // Search params affect all loaders
         currentUrl.search !== nextUrl.search ||
         isNewRouteInstance(currentRouteMatch, nextRouteMatch),
@@ -4003,9 +4004,22 @@ function stripHashFromPath(path: To) {
 }
 
 function isHashChangeOnly(a: Location, b: Location): boolean {
-  return (
-    a.pathname === b.pathname && a.search === b.search && a.hash !== b.hash
-  );
+  if (a.pathname !== b.pathname || a.search !== b.search) {
+    return false;
+  }
+
+  if (a.hash === "") {
+    // No hash -> hash
+    return b.hash !== "";
+  } else if (a.hash === b.hash) {
+    // current hash -> same hash
+    return true;
+  } else if (b.hash !== "") {
+    // current hash -> new hash
+    return true;
+  }
+
+  return false;
 }
 
 function isDeferredResult(result: DataResult): result is DeferredResult {