From 586344f0575cef71b0f631978d2eab4987c038b6 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 29 Nov 2022 11:36:36 -0500 Subject: [PATCH] Respect replace behavior on external redirects (#9654) * Respect replace behavior on external redirects * Add changeset --- .changeset/itchy-phones-rush.md | 5 ++++ packages/router/__tests__/router-test.ts | 38 +++++++++++++++++++++++- packages/router/router.ts | 6 +++- 3 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 .changeset/itchy-phones-rush.md diff --git a/.changeset/itchy-phones-rush.md b/.changeset/itchy-phones-rush.md new file mode 100644 index 0000000000..d22dafae28 --- /dev/null +++ b/.changeset/itchy-phones-rush.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +Respect `replace` behavior on external redirects diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index fbfe12e933..743b0a731d 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -5570,7 +5570,7 @@ describe("a router", () => { }); }); - it("processes external redirects if window is present", async () => { + it("processes external redirects if window is present (push)", async () => { let urls = [ "http://remix.run/blog", "https://remix.run/blog", @@ -5583,6 +5583,7 @@ describe("a router", () => { // https://stackoverflow.com/a/60697570 let oldLocation = window.location; const location = new URL(window.location.href) as unknown as Location; + location.assign = jest.fn(); location.replace = jest.fn(); delete (window as any).location; window.location = location as unknown as Location; @@ -5594,8 +5595,43 @@ describe("a router", () => { formData: createFormData({}), }); + await A.actions.child.redirectReturn(url); + expect(window.location.assign).toHaveBeenCalledWith(url); + expect(window.location.replace).not.toHaveBeenCalled(); + + window.location = oldLocation; + } + }); + + it("processes external redirects if window is present (replace)", async () => { + let urls = [ + "http://remix.run/blog", + "https://remix.run/blog", + "//remix.run/blog", + "app://whatever", + ]; + + for (let url of urls) { + // This is gross, don't blame me, blame SO :) + // https://stackoverflow.com/a/60697570 + let oldLocation = window.location; + const location = new URL(window.location.href) as unknown as Location; + location.assign = jest.fn(); + location.replace = jest.fn(); + delete (window as any).location; + window.location = location as unknown as Location; + + let t = setup({ routes: REDIRECT_ROUTES }); + + let A = await t.navigate("/parent/child", { + formMethod: "post", + formData: createFormData({}), + replace: true, + }); + await A.actions.child.redirectReturn(url); expect(window.location.replace).toHaveBeenCalledWith(url); + expect(window.location.assign).not.toHaveBeenCalled(); window.location = oldLocation; } diff --git a/packages/router/router.ts b/packages/router/router.ts index 616fd84e8d..40609b9c2d 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1604,7 +1604,11 @@ export function createRouter(init: RouterInit): Router { typeof window !== "undefined" && typeof window.location !== "undefined" ) { - window.location.replace(redirect.location); + if (replace) { + window.location.replace(redirect.location); + } else { + window.location.assign(redirect.location); + } return; }