throw on non-serializable state PUSH navigation (#10427)

remix-run · May 23, 2023 · 06f712f · 06f712f
1 parent 6dac8de
commit 06f712f
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 2 deletions.
diff --git a/.changeset/non-serializable-state.md 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
@@ -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
@@ -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(`<!DOCTYPE html><p>History Example</p>`, {
+    dom = new JSDOM(`<!DOCTYPE html><p>History Example</p>`, {
       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
@@ -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(`<!DOCTYPE html><p>History Example</p>`, {
+    dom = new JSDOM(`<!DOCTYPE html><p>History Example</p>`, {
       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
@@ -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);