From 7fe2db3e86d8145df3f2e64baa90e4e3aeff073c Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 1 May 2023 18:14:56 -0400 Subject: [PATCH] throw on non-serializable state PUSH navigation --- .changeset/non-serializable-state.md | 6 ++++++ .../TestSequences/PushStateInvalid.ts | 19 +++++++++++++++++++ packages/router/__tests__/browser-test.ts | 8 +++++++- packages/router/__tests__/hash-test.ts | 8 +++++++- packages/router/history.ts | 7 +++++++ 5 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 .changeset/non-serializable-state.md create mode 100644 packages/router/__tests__/TestSequences/PushStateInvalid.ts diff --git a/.changeset/non-serializable-state.md b/.changeset/non-serializable-state.md new file mode 100644 index 0000000000..091eb102da --- /dev/null +++ b/.changeset/non-serializable-state.md @@ -0,0 +1,6 @@ +--- +"@remix-run/router": patch +"react-router-dom": patch +--- + +Re-throw `DOMException` (`DataCloneError`) when attempting to perform a `PUSH` navigation with non-serializable state. diff --git a/packages/router/__tests__/TestSequences/PushStateInvalid.ts b/packages/router/__tests__/TestSequences/PushStateInvalid.ts new file mode 100644 index 0000000000..c683cbd2cb --- /dev/null +++ b/packages/router/__tests__/TestSequences/PushStateInvalid.ts @@ -0,0 +1,19 @@ +import type { DOMWindow } from "jsdom"; +import type { History } from "../../history"; + +export default function PushState(history: History, window: DOMWindow) { + let err = new DOMException("ERROR", "DataCloneError"); + jest.spyOn(window.history, "pushState").mockImplementation(() => { + throw err; + }); + + expect(history.location).toMatchObject({ + pathname: "/", + }); + + expect(() => + history.push("/home?the=query#the-hash", { invalid: () => {} }) + ).toThrow(err); + + expect(history.location.pathname).toBe("/"); +} diff --git a/packages/router/__tests__/browser-test.ts b/packages/router/__tests__/browser-test.ts index b699f501ae..64b736f4d9 100644 --- a/packages/router/__tests__/browser-test.ts +++ b/packages/router/__tests__/browser-test.ts @@ -10,6 +10,7 @@ import Listen from "./TestSequences/Listen"; import PushNewLocation from "./TestSequences/PushNewLocation"; import PushSamePath from "./TestSequences/PushSamePath"; import PushState from "./TestSequences/PushState"; +import PushStateInvalid from "./TestSequences/PushStateInvalid"; import PushMissingPathname from "./TestSequences/PushMissingPathname"; import PushRelativePathname from "./TestSequences/PushRelativePathname"; import ReplaceNewLocation from "./TestSequences/ReplaceNewLocation"; @@ -22,10 +23,11 @@ import ListenPopOnly from "./TestSequences/ListenPopOnly"; describe("a browser history", () => { let history: BrowserHistory; + let dom: JSDOM; beforeEach(() => { // Need to use our own custom DOM in order to get a working history - const dom = new JSDOM(`

History Example

`, { + dom = new JSDOM(`

History Example

`, { url: "https://example.org/", }); dom.window.history.replaceState(null, "", "/"); @@ -91,6 +93,10 @@ describe("a browser history", () => { it("calls change listeners with the new location", () => { PushState(history); }); + + it("re-throws when using non-serializable state", () => { + PushStateInvalid(history, dom.window); + }); }); describe("push with no pathname", () => { diff --git a/packages/router/__tests__/hash-test.ts b/packages/router/__tests__/hash-test.ts index 9a2f166f82..f4b503d9a0 100644 --- a/packages/router/__tests__/hash-test.ts +++ b/packages/router/__tests__/hash-test.ts @@ -10,6 +10,7 @@ import InitialLocationDefaultKey from "./TestSequences/InitialLocationDefaultKey import PushNewLocation from "./TestSequences/PushNewLocation"; import PushSamePath from "./TestSequences/PushSamePath"; import PushState from "./TestSequences/PushState"; +import PushStateInvalid from "./TestSequences/PushStateInvalid"; import PushMissingPathname from "./TestSequences/PushMissingPathname"; import PushRelativePathnameWarning from "./TestSequences/PushRelativePathnameWarning"; import ReplaceNewLocation from "./TestSequences/ReplaceNewLocation"; @@ -26,10 +27,11 @@ import ListenPopOnly from "./TestSequences/ListenPopOnly"; describe("a hash history", () => { let history: HashHistory; + let dom: JSDOM; beforeEach(() => { // Need to use our own custom DOM in order to get a working history - const dom = new JSDOM(`

History Example

`, { + dom = new JSDOM(`

History Example

`, { url: "https://example.org/", }); dom.window.history.replaceState(null, "", "#/"); @@ -95,6 +97,10 @@ describe("a hash history", () => { it("calls change listeners with the new location", () => { PushState(history); }); + + it("re-throws when using non-serializable state", () => { + PushStateInvalid(history, dom.window); + }); }); describe("push with no pathname", () => { diff --git a/packages/router/history.ts b/packages/router/history.ts index 9b5a01b370..f12845324a 100644 --- a/packages/router/history.ts +++ b/packages/router/history.ts @@ -634,6 +634,13 @@ function getUrlBasedHistory( try { globalHistory.pushState(historyState, "", url); } catch (error) { + // If the exception is because `state` can't be serialized, let that throw + // outwards just like a replace call would so the dev knows the cause + // https://html.spec.whatwg.org/multipage/nav-history-apis.html#shared-history-push/replace-state-steps + // https://html.spec.whatwg.org/multipage/structured-data.html#structuredserializeinternal + if (error instanceof DOMException && error.name === "DataCloneError") { + throw error; + } // They are going to lose state here, but there is no real // way to warn them about it since the page will refresh... window.location.assign(url);