remix-run · brophdawg11 · Jun 6, 2023 · Jun 1, 2023 · Jun 2, 2023 · Jun 2, 2023
diff --git a/.changeset/strip-basename-getkey.md b/.changeset/strip-basename-getkey.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+Strip `basename` from the `location` provided to `<ScrollRestoration getKey>` to match the `useLocation` behavior
diff --git a/package.json b/package.json
@@ -118,10 +118,10 @@
  "none": "15.8 kB"
  },
  "packages/react-router-dom/dist/react-router-dom.production.min.js": {
- "none": "12.0 kB"
+ "none": "12.1 kB"
  },
  "packages/react-router-dom/dist/umd/react-router-dom.production.min.js": {
- "none": "18.0 kB"
+ "none": "18.1 kB"
  }
  }
 }
diff --git a/packages/react-router-dom/__tests__/scroll-restoration-test.tsx b/packages/react-router-dom/__tests__/scroll-restoration-test.tsx
@@ -0,0 +1,85 @@
+import { JSDOM } from "jsdom";
+import * as React from "react";
+import { render, prettyDOM, fireEvent, screen } from "@testing-library/react";
+import "@testing-library/jest-dom";
+import {
+ Link,
+ Outlet,
+ RouterProvider,
+ ScrollRestoration,
+ createBrowserRouter,
+} from "react-router-dom";
+
+describe(`ScrollRestoration`, () => {
+ it("removes the basename from the location provided to getKey", () => {
+ let getKey = jest.fn(() => "mykey");
+ let testWindow = getWindowImpl("/base");
+ window.scrollTo = () => {};
+
+ let router = createBrowserRouter(
+ [
+ {
+ path: "/",
+ Component() {
+ return (
+ <>
+ <Outlet />
+ <ScrollRestoration getKey={getKey} />
+ </>
+ );
+ },
+ children: [
+ {
+ index: true,
+ Component() {
+ return <Link to="/page">/page</Link>;
+ },
+ },
+ {
+ path: "page",
+ Component() {
+ return <h1>Page</h1>;
+ },
+ },
+ ],
+ },
+ ],
+ { basename: "/base", window: testWindow }
+ );
+ let { container } = render(<RouterProvider router={router} />);
+
+ expect(getKey.mock.calls.length).toBe(1);
+ // @ts-expect-error
+ expect(getKey.mock.calls[0][0].pathname).toBe("/"); // restore
+
+ expect(getHtml(container)).toMatch("/page");
+ fireEvent.click(screen.getByText("/page"));
+ expect(getHtml(container)).toMatch("Page");
+
+ expect(getKey.mock.calls.length).toBe(3);
+ // @ts-expect-error
+ expect(getKey.mock.calls[1][0].pathname).toBe("/"); // save
+ // @ts-expect-error
+ expect(getKey.mock.calls[2][0].pathname).toBe("/page"); // restore
+ });
+});
+
+function getWindowImpl(initialUrl: string): 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, "", initialUrl);
+ return dom.window as unknown as Window;
+}
+
+function getHtml(container: HTMLElement) {
+ return prettyDOM(container, undefined, {
+ highlight: false,
+ theme: {
+ comment: null,
+ content: null,
+ prop: null,
+ tag: null,
+ value: null,
+ },
+ });
+}
diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx
@@ -1208,6 +1208,7 @@ function useScrollRestoration({
  let { restoreScrollPosition, preventScrollReset } = useDataRouterState(
  DataRouterStateHook.UseScrollRestoration
  );
+ let { basename } = React.useContext(NavigationContext);
  let location = useLocation();
  let matches = useMatches();
  let navigation = useNavigation();
@@ -1254,13 +1255,27 @@ function useScrollRestoration({
  // Enable scroll restoration in the router
  // eslint-disable-next-line react-hooks/rules-of-hooks
  React.useLayoutEffect(() => {
+ let getKeyWithoutBasename: GetScrollRestorationKeyFunction | undefined =
+ getKey && basename !== "/"
+ ? (location, matches) =>
+ getKey(
+ // Strip the basename to match useLocation()
+ {
+ ...location,
+ pathname:
+ stripBasename(location.pathname, basename) ||
+ location.pathname,
+ },
+ matches
+ )
+ : getKey;
  let disableScrollRestoration = router?.enableScrollRestoration(
  savedScrollPositions,
  () => window.scrollY,
- getKey
+ getKeyWithoutBasename
  );
  return () => disableScrollRestoration && disableScrollRestoration();
- }, [router, getKey]);
+ }, [router, basename, getKey]);
 
  // Restore scrolling when state.restoreScrollPosition changes
  // eslint-disable-next-line react-hooks/rules-of-hooks

diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts
@@ -6959,6 +6959,37 @@ describe("a router", () => {
  expect(t.router.state.preventScrollReset).toBe(false);
  });
 
+ it("does not strip the basename from the location provided to getKey", async () => {
+ let t = setup({
+ routes: SCROLL_ROUTES,
+ basename: "/base",
+ initialEntries: ["/base"],
+ hydrationData: {
+ loaderData: {
+ index: "INDEX_DATA",
+ },
+ },
+ });
+
+ let positions = { "/base/tasks": 100 };
+ let activeScrollPosition = 0;
+ let pathname;
+ t.router.enableScrollRestoration(
+ positions,
+ () => activeScrollPosition,
+ (l) => {
+ pathname = l.pathname;
+ return l.pathname;
+ }
+ );
+
+ let nav1 = await t.navigate("/base/tasks");
+ await nav1.loaders.tasks.resolve("TASKS");
+ expect(pathname).toBe("/base/tasks");
+ expect(t.router.state.restoreScrollPosition).toBe(100);
+ expect(t.router.state.preventScrollReset).toBe(false);
+ });
+
  it("restores scroll on GET submissions", async () => {
  let t = setup({
  routes: SCROLL_ROUTES,

diff --git a/packages/router/router.ts b/packages/router/router.ts
@@ -2444,7 +2444,7 @@ export function createRouter(init: RouterInit): Router {
  ) {
  savedScrollPositions = positions;
  getScrollPosition = getPosition;
- getScrollRestorationKey = getKey || ((location) => location.key);
+ getScrollRestorationKey = getKey || null;
 
  // Perform initial hydration scroll restoration, since we miss the boat on
  // the initial updateState() because we've not yet rendered <ScrollRestoration/>
@@ -2464,15 +2464,23 @@ export function createRouter(init: RouterInit): Router {
  };
  }
 
+ function getScrollKey(location: Location, matches: AgnosticDataRouteMatch[]) {
+ if (getScrollRestorationKey) {
+ let key = getScrollRestorationKey(
+ location,
+ matches.map((m) => createUseMatchesMatch(m, state.loaderData))
+ );
+ return key || location.key;
+ }
+ return location.key;
+ }
+
  function saveScrollPosition(
  location: Location,
  matches: AgnosticDataRouteMatch[]
  ): void {
- if (savedScrollPositions && getScrollRestorationKey && getScrollPosition) {
- let userMatches = matches.map((m) =>
- createUseMatchesMatch(m, state.loaderData)
- );
- let key = getScrollRestorationKey(location, userMatches) || location.key;
+ if (savedScrollPositions && getScrollPosition) {
+ let key = getScrollKey(location, matches);
  savedScrollPositions[key] = getScrollPosition();
  }
  }
@@ -2481,11 +2489,8 @@ export function createRouter(init: RouterInit): Router {
  location: Location,
  matches: AgnosticDataRouteMatch[]
  ): number | null {
- if (savedScrollPositions && getScrollRestorationKey && getScrollPosition) {
- let userMatches = matches.map((m) =>
- createUseMatchesMatch(m, state.loaderData)
- );
- let key = getScrollRestorationKey(location, userMatches) || location.key;
+ if (savedScrollPositions) {
+ let key = getScrollKey(location, matches);
  let y = savedScrollPositions[key];
  if (typeof y === "number") {
  return y;