Skip to content

Commit 08e2f76

Browse files
brophdawg11brophdawg11
authored
Strip basename from location provided to getKey (#10550)
1 parent 8972f30 commit 08e2f76

File tree

6 files changed

+156
-15
lines changed

6 files changed

+156
-15
lines changed

‎.changeset/strip-basename-getkey.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@remix-run/router": patch
3+
---
4+
5+
Strip `basename` from the `location` provided to `<ScrollRestoration getKey>` to match the `useLocation` behavior

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,10 @@
118118
"none": "15.8 kB"
119119
},
120120
"packages/react-router-dom/dist/react-router-dom.production.min.js": {
121-
"none": "12.0 kB"
121+
"none": "12.1 kB"
122122
},
123123
"packages/react-router-dom/dist/umd/react-router-dom.production.min.js": {
124-
"none": "18.0 kB"
124+
"none": "18.1 kB"
125125
}
126126
}
127127
}

‎packages/react-router-dom/__tests__/scroll-restoration-test.tsx‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { JSDOM } from "jsdom";
2+
import * as React from "react";
3+
import { render, prettyDOM, fireEvent, screen } from "@testing-library/react";
4+
import "@testing-library/jest-dom";
5+
import {
6+
Link,
7+
Outlet,
8+
RouterProvider,
9+
ScrollRestoration,
10+
createBrowserRouter,
11+
} from "react-router-dom";
12+
13+
describe(`ScrollRestoration`, () => {
14+
it("removes the basename from the location provided to getKey", () => {
15+
let getKey = jest.fn(() => "mykey");
16+
let testWindow = getWindowImpl("/base");
17+
window.scrollTo = () => {};
18+
19+
let router = createBrowserRouter(
20+
[
21+
{
22+
path: "/",
23+
Component() {
24+
return (
25+
<>
26+
<Outlet />
27+
<ScrollRestoration getKey={getKey} />
28+
</>
29+
);
30+
},
31+
children: [
32+
{
33+
index: true,
34+
Component() {
35+
return <Link to="/page">/page</Link>;
36+
},
37+
},
38+
{
39+
path: "page",
40+
Component() {
41+
return <h1>Page</h1>;
42+
},
43+
},
44+
],
45+
},
46+
],
47+
{ basename: "/base", window: testWindow }
48+
);
49+
let { container } = render(<RouterProvider router={router} />);
50+
51+
expect(getKey.mock.calls.length).toBe(1);
52+
// @ts-expect-error
53+
expect(getKey.mock.calls[0][0].pathname).toBe("/"); // restore
54+
55+
expect(getHtml(container)).toMatch("/page");
56+
fireEvent.click(screen.getByText("/page"));
57+
expect(getHtml(container)).toMatch("Page");
58+
59+
expect(getKey.mock.calls.length).toBe(3);
60+
// @ts-expect-error
61+
expect(getKey.mock.calls[1][0].pathname).toBe("/"); // save
62+
// @ts-expect-error
63+
expect(getKey.mock.calls[2][0].pathname).toBe("/page"); // restore
64+
});
65+
});
66+
67+
function getWindowImpl(initialUrl: string): Window {
68+
// Need to use our own custom DOM in order to get a working history
69+
const dom = new JSDOM(`<!DOCTYPE html>`, { url: "http://localhost/" });
70+
dom.window.history.replaceState(null, "", initialUrl);
71+
return dom.window as unknown as Window;
72+
}
73+
74+
function getHtml(container: HTMLElement) {
75+
return prettyDOM(container, undefined, {
76+
highlight: false,
77+
theme: {
78+
comment: null,
79+
content: null,
80+
prop: null,
81+
tag: null,
82+
value: null,
83+
},
84+
});
85+
}

‎packages/react-router-dom/index.tsx‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,6 +1208,7 @@ function useScrollRestoration({
12081208
let { restoreScrollPosition, preventScrollReset } = useDataRouterState(
12091209
DataRouterStateHook.UseScrollRestoration
12101210
);
1211+
let { basename } = React.useContext(NavigationContext);
12111212
let location = useLocation();
12121213
let matches = useMatches();
12131214
let navigation = useNavigation();
@@ -1254,13 +1255,27 @@ function useScrollRestoration({
12541255
// Enable scroll restoration in the router
12551256
// eslint-disable-next-line react-hooks/rules-of-hooks
12561257
React.useLayoutEffect(() => {
1258+
let getKeyWithoutBasename: GetScrollRestorationKeyFunction | undefined =
1259+
getKey && basename !== "/"
1260+
? (location, matches) =>
1261+
getKey(
1262+
// Strip the basename to match useLocation()
1263+
{
1264+
...location,
1265+
pathname:
1266+
stripBasename(location.pathname, basename) ||
1267+
location.pathname,
1268+
},
1269+
matches
1270+
)
1271+
: getKey;
12571272
let disableScrollRestoration = router?.enableScrollRestoration(
12581273
savedScrollPositions,
12591274
() => window.scrollY,
1260-
getKey
1275+
getKeyWithoutBasename
12611276
);
12621277
return () => disableScrollRestoration && disableScrollRestoration();
1263-
}, [router, getKey]);
1278+
}, [router, basename, getKey]);
12641279

12651280
// Restore scrolling when state.restoreScrollPosition changes
12661281
// eslint-disable-next-line react-hooks/rules-of-hooks

‎packages/router/__tests__/router-test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6959,6 +6959,37 @@ describe("a router", () => {
69596959
expect(t.router.state.preventScrollReset).toBe(false);
69606960
});
69616961

6962+
it("does not strip the basename from the location provided to getKey", async () => {
6963+
let t = setup({
6964+
routes: SCROLL_ROUTES,
6965+
basename: "/base",
6966+
initialEntries: ["/base"],
6967+
hydrationData: {
6968+
loaderData: {
6969+
index: "INDEX_DATA",
6970+
},
6971+
},
6972+
});
6973+
6974+
let positions = { "/base/tasks": 100 };
6975+
let activeScrollPosition = 0;
6976+
let pathname;
6977+
t.router.enableScrollRestoration(
6978+
positions,
6979+
() => activeScrollPosition,
6980+
(l) => {
6981+
pathname = l.pathname;
6982+
return l.pathname;
6983+
}
6984+
);
6985+
6986+
let nav1 = await t.navigate("/base/tasks");
6987+
await nav1.loaders.tasks.resolve("TASKS");
6988+
expect(pathname).toBe("/base/tasks");
6989+
expect(t.router.state.restoreScrollPosition).toBe(100);
6990+
expect(t.router.state.preventScrollReset).toBe(false);
6991+
});
6992+
69626993
it("restores scroll on GET submissions", async () => {
69636994
let t = setup({
69646995
routes: SCROLL_ROUTES,

‎packages/router/router.ts‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2444,7 +2444,7 @@ export function createRouter(init: RouterInit): Router {
24442444
) {
24452445
savedScrollPositions = positions;
24462446
getScrollPosition = getPosition;
2447-
getScrollRestorationKey = getKey || ((location) => location.key);
2447+
getScrollRestorationKey = getKey || null;
24482448

24492449
// Perform initial hydration scroll restoration, since we miss the boat on
24502450
// the initial updateState() because we've not yet rendered <ScrollRestoration/>
@@ -2464,15 +2464,23 @@ export function createRouter(init: RouterInit): Router {
24642464
};
24652465
}
24662466

2467+
function getScrollKey(location: Location, matches: AgnosticDataRouteMatch[]) {
2468+
if (getScrollRestorationKey) {
2469+
let key = getScrollRestorationKey(
2470+
location,
2471+
matches.map((m) => createUseMatchesMatch(m, state.loaderData))
2472+
);
2473+
return key || location.key;
2474+
}
2475+
return location.key;
2476+
}
2477+
24672478
function saveScrollPosition(
24682479
location: Location,
24692480
matches: AgnosticDataRouteMatch[]
24702481
): void {
2471-
if (savedScrollPositions && getScrollRestorationKey && getScrollPosition) {
2472-
let userMatches = matches.map((m) =>
2473-
createUseMatchesMatch(m, state.loaderData)
2474-
);
2475-
let key = getScrollRestorationKey(location, userMatches) || location.key;
2482+
if (savedScrollPositions && getScrollPosition) {
2483+
let key = getScrollKey(location, matches);
24762484
savedScrollPositions[key] = getScrollPosition();
24772485
}
24782486
}
@@ -2481,11 +2489,8 @@ export function createRouter(init: RouterInit): Router {
24812489
location: Location,
24822490
matches: AgnosticDataRouteMatch[]
24832491
): number | null {
2484-
if (savedScrollPositions && getScrollRestorationKey && getScrollPosition) {
2485-
let userMatches = matches.map((m) =>
2486-
createUseMatchesMatch(m, state.loaderData)
2487-
);
2488-
let key = getScrollRestorationKey(location, userMatches) || location.key;
2492+
if (savedScrollPositions) {
2493+
let key = getScrollKey(location, matches);
24892494
let y = savedScrollPositions[key];
24902495
if (typeof y === "number") {
24912496
return y;

0 commit comments

Comments
 (0)