Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/stable-link-ref-callback.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

Memoize the merged `Link` ref callback to avoid unnecessary ref callback invocations on re-renders.
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@
- vladinator1000
- vonagam
- WalkAlone0325
- wataryooou
- whxhlgy
- wilcoxmd
- willemarcel
Expand Down
123 changes: 122 additions & 1 deletion packages/react-router/__tests__/dom/link-click-test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import * as React from "react";
import * as ReactDOM from "react-dom/client";
import { act } from "@testing-library/react";
import { MemoryRouter, Routes, Route, Link } from "../../index";
import {
MemoryRouter,
Routes,
Route,
Link,
UNSAFE_FrameworkContext as FrameworkContext,
} from "../../index";

function click(anchor: HTMLAnchorElement, eventInit?: MouseEventInit) {
let event = new MouseEvent("click", {
Expand Down Expand Up @@ -360,6 +366,121 @@ describe("A <Link> click", () => {
});
});

describe("when rerendering with a stable ref callback", () => {
it("does not re-invoke the ref callback", () => {
let ref = jest.fn();

function Home() {
let [count, setCount] = React.useState(0);

return (
<div>
<button onClick={() => setCount((value) => value + 1)}>Update</button>
<Link to="/home" ref={ref}>
Home {count}
</Link>
</div>
);
}

act(() => {
ReactDOM.createRoot(node).render(
<MemoryRouter initialEntries={["/home"]}>
<Routes>
<Route path="home" element={<Home />} />
</Routes>
</MemoryRouter>,
);
});

let anchor = node.querySelector("a");
let button = node.querySelector("button");
expect(anchor).not.toBeNull();
expect(button).not.toBeNull();
expect(ref).toHaveBeenCalledTimes(1);
expect(ref).toHaveBeenLastCalledWith(anchor);

act(() => {
button?.dispatchEvent(
new MouseEvent("click", {
view: window,
bubbles: true,
cancelable: true,
}),
);
});

expect(ref).toHaveBeenCalledTimes(1);
});

it("does not re-invoke the ref callback with prefetch='intent'", () => {
let ref = jest.fn();

function Home() {
return (
<div>
<Link to="/home" ref={ref} prefetch="intent">
Home
</Link>
</div>
);
}

act(() => {
ReactDOM.createRoot(node).render(
<FrameworkContext.Provider
value={
{
future: {},
manifest: { routes: {}, entry: { imports: [], module: "" } },
routeModules: {},
ssr: false,
isSpaMode: true,
routeDiscovery: { mode: "lazy", manifestPath: "" },
} as any
}
>
<MemoryRouter initialEntries={["/home"]}>
<Routes>
<Route path="home" element={<Home />} />
</Routes>
</MemoryRouter>
</FrameworkContext.Provider>,
);
});

let anchor = node.querySelector("a");
expect(anchor).not.toBeNull();
expect(ref).toHaveBeenCalledTimes(1);
expect(ref).toHaveBeenLastCalledWith(anchor);

// mouseenter triggers setMaybePrefetch(true) → re-render
act(() => {
anchor?.dispatchEvent(
new MouseEvent("mouseenter", {
view: window,
bubbles: true,
cancelable: true,
}),
);
});

// mouseleave triggers cancelIntent() → re-render
act(() => {
anchor?.dispatchEvent(
new MouseEvent("mouseleave", {
view: window,
bubbles: true,
cancelable: true,
}),
);
});

// ref should still have been called only once (on mount)
expect(ref).toHaveBeenCalledTimes(1);
});
});

describe("with a right click", () => {
it("stays on the same page", () => {
function Home() {
Expand Down
7 changes: 6 additions & 1 deletion packages/react-router/lib/dom/lib.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,11 @@ export const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(
}
}

let mergedRef = React.useMemo(
() => mergeRefs(forwardedRef, prefetchRef),
[forwardedRef, prefetchRef],
);

let isSpaLink = !(parsed.isExternal || reloadDocument);
let link = (
// eslint-disable-next-line jsx-a11y/anchor-has-content
Expand All @@ -1396,7 +1401,7 @@ export const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(
(isSpaLink ? maskedHref : undefined) || parsed.absoluteURL || href
}
onClick={isSpaLink ? handleClick : onClick}
ref={mergeRefs(forwardedRef, prefetchRef)}
ref={mergedRef}
target={target}
data-discover={
!isAbsolute && discover === "render" ? "true" : undefined
Expand Down