Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

throw on non-serializable state PUSH navigation #10427

Merged
merged 1 commit into from May 23, 2023
Merged

throw on non-serializable state PUSH navigation #10427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/non-serializable-state.md
View file
Open in desktop
@@ -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.
19 changes: 19 additions & 0 deletions packages/router/__tests__/TestSequences/PushStateInvalid.ts
View file
Open in desktop
@@ -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("/");
}
8 changes: 7 additions & 1 deletion packages/router/__tests__/browser-test.ts
View file
Open in desktop
Expand Up @@ -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";
Expand All @@ -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, "", "/");
Expand Down Expand Up @@ -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", () => {
Expand Down
8 changes: 7 additions & 1 deletion packages/router/__tests__/hash-test.ts
View file
Open in desktop
Expand Up @@ -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";
Expand All @@ -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, "", "#/");
Expand Down Expand Up @@ -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", () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/router/history.ts
View file
Open in desktop
Expand Up @@ -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);
Expand Down