Skip to content

Commit

Permalink
fix: don't reload parent routes when search params are constant (#1965)
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
jca41 committed Feb 15, 2022
1 parent 7860169 commit a762516
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 1 deletion.
1 change: 1 addition & 0 deletions contributors.yml
Expand Up @@ -185,3 +185,4 @@
- zainfathoni
- jssisodiya
- guerra08
- jca41
32 changes: 32 additions & 0 deletions packages/remix-react/__tests__/transition-test.tsx
Expand Up @@ -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();

Expand Down Expand Up @@ -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 {},
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-react/transition.ts
Expand Up @@ -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);
}
Expand Down

0 comments on commit a762516

Please sign in to comment.