Respect replace behavior on external redirects (#9654)

* Respect replace behavior on external redirects * Add changeset
remix-run · Nov 29, 2022 · 586344f · 586344f
1 parent 83b4758
commit 586344f
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 2 deletions.
diff --git a/.changeset/itchy-phones-rush.md 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
@@ -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
@@ -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;
     }