From a7625169f5ff392a2c36053c49c79ff583d49cf6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Caf=C3=A9?= <jpgacafe@gmail.com>
Date: Tue, 15 Feb 2022 15:13:34 +0000
Subject: [PATCH] fix: don't reload parent routes when search params are
 constant (#1965)

`location.search` starts with `?` while `searchParams.toString()` does not have a `?` in it, so this check was not behaving any differently than just checking if `location.search` wasn't empty.

We might be tempted to sort these before comparing, but technically application code can behave differently depending on the order of the params, so we shouldn't do that.
---
 contributors.yml                              |  1 +
 .../remix-react/__tests__/transition-test.tsx | 32 +++++++++++++++++++
 packages/remix-react/transition.ts            |  2 +-
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/contributors.yml b/contributors.yml
index 50605b154c2..20f1898aca6 100644
--- a/contributors.yml
+++ b/contributors.yml
@@ -185,3 +185,4 @@
 - zainfathoni
 - jssisodiya
 - guerra08
+- jca41
diff --git a/packages/remix-react/__tests__/transition-test.tsx b/packages/remix-react/__tests__/transition-test.tsx
index 7debd56e11d..e3f0c2233a9 100644
--- a/packages/remix-react/__tests__/transition-test.tsx
+++ b/packages/remix-react/__tests__/transition-test.tsx
@@ -118,6 +118,20 @@ describe("normal navigation", () => {
     expect(t.getState().loaderData.foo).toBe("2");
   });
 
+  it("does not reload all routes when search does not change", async () => {
+    let t = setup();
+    let A = t.navigate.get("/foo?q=1");
+    await A.loader.resolve("1");
+    expect(t.rootLoaderMock.calls.length).toBe(1);
+    expect(t.getState().loaderData.foo).toBe("1");
+
+    let B = t.navigate.get("/foo/bar?q=1");
+    await B.loader.resolve("2");
+    expect(t.rootLoaderMock.calls.length).toBe(1);
+    
+    expect(t.getState().loaderData.foobar).toBe("2");
+  });
+
   it("reloads only routes with changed params", async () => {
     let t = setup();
 
@@ -314,6 +328,15 @@ describe("no route match", () => {
                 "module": "",
                 "path": "/foo",
               },
+              Object {
+                "action": [MockFunction],
+                "element": Object {},
+                "hasLoader": true,
+                "id": "foobar",
+                "loader": [MockFunction],
+                "module": "",
+                "path": "/foo/bar",
+              },
               Object {
                 "action": [MockFunction],
                 "element": Object {},
@@ -1892,6 +1915,15 @@ let setup = ({ url } = { url: "/" }) => {
           element: {},
           module: ""
         },
+        {
+          path: "/foo/bar",
+          id: "foobar",
+          hasLoader: true,
+          loader: createLoader(),
+          action: createAction(),
+          element: {},
+          module: ""
+        },
         {
           path: "/bar",
           id: "bar",
diff --git a/packages/remix-react/transition.ts b/packages/remix-react/transition.ts
index e2190f91e20..bb2f34a0e13 100644
--- a/packages/remix-react/transition.ts
+++ b/packages/remix-react/transition.ts
@@ -1395,7 +1395,7 @@ function filterMatchesToLoad(
     // clicked the same link, resubmitted a GET form
     createHref(url) === createHref(state.location) ||
     // search affects all loaders
-    url.searchParams.toString() !== state.location.search
+    url.searchParams.toString() !== state.location.search.substring(1)
   ) {
     return matches.filter(filterByRouteProps);
   }