Fix partial object (search or hash only) pathnames losing current path (

#10029) Co-authored-by: Logan McAnsh <logan@remix.run>
remix-run · Feb 2, 2023 · b620be2 · b620be2
1 parent c92a99d
commit b620be2
Show file tree

Hide file tree

Showing 4 changed files with 198 additions and 16 deletions.
diff --git a/.changeset/violet-apes-pay.md b/.changeset/violet-apes-pay.md
@@ -0,0 +1,5 @@
+---
+"react-router-dom": patch
+---
+
+Fix partial object (search or hash only) pathnames losing current path value
diff --git a/packages/react-router-dom/__tests__/link-click-test.tsx b/packages/react-router-dom/__tests__/link-click-test.tsx
@@ -61,6 +61,116 @@ describe("A <Link> click", () => {
     expect(h1?.textContent).toEqual("About");
   });
 
+  it("navigates to the new page when using an absolute URL on the same origin", () => {
+    function Home() {
+      return (
+        <div>
+          <h1>Home</h1>
+          <Link to="http://localhost/about">About</Link>
+        </div>
+      );
+    }
+
+    act(() => {
+      ReactDOM.createRoot(node).render(
+        <MemoryRouter initialEntries={["/home"]}>
+          <Routes>
+            <Route path="home" element={<Home />} />
+            <Route path="about" element={<h1>About</h1>} />
+          </Routes>
+        </MemoryRouter>
+      );
+    });
+
+    let anchor = node.querySelector("a");
+    expect(anchor).not.toBeNull();
+
+    let event: MouseEvent;
+    act(() => {
+      event = click(anchor);
+    });
+
+    expect(event.defaultPrevented).toBe(true);
+    let h1 = node.querySelector("h1");
+    expect(h1).not.toBeNull();
+    expect(h1?.textContent).toEqual("About");
+  });
+
+  describe("when an external absolute URL is specified", () => {
+    it("does not prevent default", () => {
+      function Home() {
+        return (
+          <div>
+            <h1>Home</h1>
+            <Link to="https://remix.run">About</Link>
+          </div>
+        );
+      }
+
+      act(() => {
+        ReactDOM.createRoot(node).render(
+          <MemoryRouter initialEntries={["/home"]}>
+            <Routes>
+              <Route path="home" element={<Home />} />
+              <Route path="about" element={<h1>About</h1>} />
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      let anchor = node.querySelector("a");
+      expect(anchor).not.toBeNull();
+
+      let event: MouseEvent;
+      act(() => {
+        event = click(anchor);
+      });
+
+      expect(event.defaultPrevented).toBe(false);
+    });
+
+    it("calls provided listener", () => {
+      let handlerCalled;
+      let defaultPrevented;
+
+      function Home() {
+        return (
+          <div>
+            <h1>Home</h1>
+            <Link
+              reloadDocument
+              to="https://remix.run"
+              onClick={(e) => {
+                handlerCalled = true;
+                defaultPrevented = e.defaultPrevented;
+              }}
+            >
+              About
+            </Link>
+          </div>
+        );
+      }
+
+      act(() => {
+        ReactDOM.createRoot(node).render(
+          <MemoryRouter initialEntries={["/home"]}>
+            <Routes>
+              <Route path="home" element={<Home />} />
+              <Route path="about" element={<h1>About</h1>} />
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      act(() => {
+        click(node.querySelector("a"));
+      });
+
+      expect(handlerCalled).toBe(true);
+      expect(defaultPrevented).toBe(false);
+    });
+  });
+
   describe("when reloadDocument is specified", () => {
     it("does not prevent default", () => {
       function Home() {

diff --git a/packages/react-router-dom/__tests__/link-href-test.tsx b/packages/react-router-dom/__tests__/link-href-test.tsx
@@ -172,6 +172,72 @@ describe("<Link> href", () => {
         "web+remix://somepath"
       );
     });
+
+    test('<Link to="http://localhost/inbox"> is treated as an absolute link', () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/inbox/messages"]}>
+            <Routes>
+              <Route path="inbox">
+                <Route
+                  path="messages"
+                  element={<Link to="http://localhost/inbox" />}
+                />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.root.findByType("a").props.href).toEqual(
+        "http://localhost/inbox"
+      );
+    });
+
+    test("<Link to=\"{ search: 'key=value'\"> is handled with the current pathname", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/inbox/messages"]}>
+            <Routes>
+              <Route path="inbox">
+                <Route
+                  path="messages"
+                  element={<Link to={{ search: "key=value" }} />}
+                />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.root.findByType("a").props.href).toEqual(
+        "/inbox/messages?key=value"
+      );
+    });
+
+    test("<Link to=\"{ hash: 'hash'\"> is handled with the current pathname", () => {
+      let renderer: TestRenderer.ReactTestRenderer;
+      TestRenderer.act(() => {
+        renderer = TestRenderer.create(
+          <MemoryRouter initialEntries={["/inbox/messages"]}>
+            <Routes>
+              <Route path="inbox">
+                <Route
+                  path="messages"
+                  element={<Link to={{ hash: "hash" }} />}
+                />
+              </Route>
+            </Routes>
+          </MemoryRouter>
+        );
+      });
+
+      expect(renderer.root.findByType("a").props.href).toEqual(
+        "/inbox/messages#hash"
+      );
+    });
   });
 
   describe("in a dynamic route", () => {

diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx
@@ -418,31 +418,32 @@ export const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(
     },
     ref
   ) {
-    // `location` is the unaltered href we will render in the <a> tag for absolute URLs
-    let location = typeof to === "string" ? to : createPath(to);
-    let isAbsolute = /^(?:[a-z][a-z0-9+.-]*:|\/\/)/i.test(location);
-
-    // Location to use in the click handler
-    let navigationLocation = location;
+    // Rendered into <a href> for absolute URLs
+    let absoluteHref;
     let isExternal = false;
-    if (isBrowser && isAbsolute) {
+
+    if (
+      isBrowser &&
+      typeof to === "string" &&
+      /^(?:[a-z][a-z0-9+.-]*:|\/\/)/i.test(to)
+    ) {
+      absoluteHref = to;
       let currentUrl = new URL(window.location.href);
-      let targetUrl = location.startsWith("//")
-        ? new URL(currentUrl.protocol + location)
-        : new URL(location);
+      let targetUrl = to.startsWith("//")
+        ? new URL(currentUrl.protocol + to)
+        : new URL(to);
       if (targetUrl.origin === currentUrl.origin) {
         // Strip the protocol/origin for same-origin absolute URLs
-        navigationLocation =
-          targetUrl.pathname + targetUrl.search + targetUrl.hash;
+        to = targetUrl.pathname + targetUrl.search + targetUrl.hash;
       } else {
         isExternal = true;
       }
     }
 
-    // `href` is what we render in the <a> tag for relative URLs
-    let href = useHref(navigationLocation, { relative });
+    // Rendered into <a href> for relative URLs
+    let href = useHref(to, { relative });
 
-    let internalOnClick = useLinkClickHandler(navigationLocation, {
+    let internalOnClick = useLinkClickHandler(to, {
       replace,
       state,
       target,
@@ -462,7 +463,7 @@ export const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(
       // eslint-disable-next-line jsx-a11y/anchor-has-content
       <a
         {...rest}
-        href={isAbsolute ? location : href}
+        href={absoluteHref || href}
         onClick={isExternal || reloadDocument ? onClick : handleClick}
         ref={ref}
         target={target}