From f8194fdb8e371b715d29d30a82e04a82a7648e9b Mon Sep 17 00:00:00 2001 From: David Evans <104063299+david-bezero@users.noreply.github.com> Date: Thu, 14 Sep 2023 14:54:55 +0100 Subject: [PATCH] Handle case when session storage is blocked (#10848) --- .changeset/soft-forks-cough.md | 5 + contributors.yml | 1 + .../__tests__/scroll-restoration-test.tsx | 148 ++++++++++++++++++ packages/react-router-dom/index.tsx | 15 +- 4 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 .changeset/soft-forks-cough.md diff --git a/.changeset/soft-forks-cough.md b/.changeset/soft-forks-cough.md new file mode 100644 index 0000000000..6cd2304adc --- /dev/null +++ b/.changeset/soft-forks-cough.md @@ -0,0 +1,5 @@ +--- +"react-router-dom": patch +--- + +Log a warning and fail gracefully in `ScrollRestoration` when `sessionStorage` is unavailable diff --git a/contributors.yml b/contributors.yml index 5cba918a57..bcd3da3977 100644 --- a/contributors.yml +++ b/contributors.yml @@ -52,6 +52,7 @@ - danielberndt - daniilguit - dauletbaev +- david-bezero - david-crespo - decadentsavant - DigitalNaut diff --git a/packages/react-router-dom/__tests__/scroll-restoration-test.tsx b/packages/react-router-dom/__tests__/scroll-restoration-test.tsx index d4309caacc..42e2231c86 100644 --- a/packages/react-router-dom/__tests__/scroll-restoration-test.tsx +++ b/packages/react-router-dom/__tests__/scroll-restoration-test.tsx @@ -13,6 +13,63 @@ import { import getHtml from "../../react-router/__tests__/utils/getHtml"; describe(`ScrollRestoration`, () => { + it("restores the scroll position for a page when re-visited", () => { + const consoleWarnMock = jest + .spyOn(console, "warn") + .mockImplementation(() => {}); + + let testWindow = getWindowImpl("/base"); + const mockScroll = jest.fn(); + window.scrollTo = mockScroll; + + let router = createBrowserRouter( + [ + { + path: "/", + Component() { + return ( + <> + + "test1-" + location.pathname} + /> + + ); + }, + children: testPages, + }, + ], + { basename: "/base", window: testWindow } + ); + let { container } = render(); + + expect(getHtml(container)).toMatch("On page 1"); + + // simulate scrolling + Object.defineProperty(window, "scrollY", { writable: true, value: 100 }); + + // leave page + window.dispatchEvent(new Event("pagehide")); + fireEvent.click(screen.getByText("Go to page 2")); + expect(getHtml(container)).toMatch("On page 2"); + + // return to page + window.dispatchEvent(new Event("pagehide")); + fireEvent.click(screen.getByText("Go to page 1")); + + expect(getHtml(container)).toMatch("On page 1"); + + // check scroll activity + expect(mockScroll.mock.calls).toEqual([ + [0, 0], + [0, 0], + [0, 100], // restored + ]); + + expect(consoleWarnMock).not.toHaveBeenCalled(); + consoleWarnMock.mockRestore(); + }); + it("removes the basename from the location provided to getKey", () => { let getKey = jest.fn(() => "mykey"); let testWindow = getWindowImpl("/base"); @@ -64,8 +121,99 @@ describe(`ScrollRestoration`, () => { // @ts-expect-error expect(getKey.mock.calls[2][0].pathname).toBe("/page"); // restore }); + + it("fails gracefully if sessionStorage is not available", () => { + const consoleWarnMock = jest + .spyOn(console, "warn") + .mockImplementation(() => {}); + + let testWindow = getWindowImpl("/base"); + const mockScroll = jest.fn(); + window.scrollTo = mockScroll; + + jest.spyOn(window, "sessionStorage", "get").mockImplementation(() => { + throw new Error("denied"); + }); + + let router = createBrowserRouter( + [ + { + path: "/", + Component() { + return ( + <> + + "test2-" + location.pathname} + /> + + ); + }, + children: testPages, + }, + ], + { basename: "/base", window: testWindow } + ); + let { container } = render(); + + expect(getHtml(container)).toMatch("On page 1"); + + // simulate scrolling + Object.defineProperty(window, "scrollY", { writable: true, value: 100 }); + + // leave page + window.dispatchEvent(new Event("pagehide")); + fireEvent.click(screen.getByText("Go to page 2")); + expect(getHtml(container)).toMatch("On page 2"); + + // return to page + window.dispatchEvent(new Event("pagehide")); + fireEvent.click(screen.getByText("Go to page 1")); + + expect(getHtml(container)).toMatch("On page 1"); + + // check scroll activity + expect(mockScroll.mock.calls).toEqual([ + [0, 0], + [0, 0], + [0, 100], // restored (still possible because the user hasn't left the page) + ]); + + expect(consoleWarnMock).toHaveBeenCalledWith( + expect.stringContaining( + "Failed to save scroll positions in sessionStorage" + ) + ); + + consoleWarnMock.mockRestore(); + }); }); +const testPages = [ + { + index: true, + Component() { + return ( +

+ On page 1
+ Go to page 2 +

+ ); + }, + }, + { + path: "page", + Component() { + return ( +

+ On page 2
+ Go to page 1 +

+ ); + }, + }, +]; + function getWindowImpl(initialUrl: string): Window { // Need to use our own custom DOM in order to get a working history const dom = new JSDOM(``, { url: "http://localhost/" }); diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index e60e754e72..08b8e4cede 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -1323,10 +1323,17 @@ function useScrollRestoration({ let key = (getKey ? getKey(location, matches) : null) || location.key; savedScrollPositions[key] = window.scrollY; } - sessionStorage.setItem( - storageKey || SCROLL_RESTORATION_STORAGE_KEY, - JSON.stringify(savedScrollPositions) - ); + try { + sessionStorage.setItem( + storageKey || SCROLL_RESTORATION_STORAGE_KEY, + JSON.stringify(savedScrollPositions) + ); + } catch (error) { + warning( + false, + `Failed to save scroll positions in sessionStorage, will not work properly (${error}).` + ); + } window.history.scrollRestoration = "auto"; }, [storageKey, getKey, navigation.state, location, matches]) );