Skip to content

fix(link): align app-router prefetch scheduling with intent #1149

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

Open
NathanDrake2406 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(link): align app-router prefetch scheduling with intent #1149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
97dbffa
fix(link): align app-router prefetch scheduling with intent
NathanDrake2406 May 9, 2026
d78d45a
fix(link): align app-router prefetch scheduling with intent
NathanDrake2406 May 9, 2026
2949975
test: tighten link prefetch coverage
NathanDrake2406 May 9, 2026
d7e142b
test(link): tighten prefetch decision coverage
NathanDrake2406 May 11, 2026
f0b925e
test(link): address prefetch review notes
NathanDrake2406 May 12, 2026
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
73 changes: 73 additions & 0 deletions packages/vinext/src/shims/link-prefetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { hasBasePath, stripBasePath } from "../utils/base-path.js";

export type LinkPrefetchIntent = "viewport" | "intent";
export type LinkPrefetchPriority = "low" | "high";

export type LinkPrefetchDecision =
| {
shouldPrefetch: false;
}
| {
shouldPrefetch: true;
priority: LinkPrefetchPriority;
};

export function canLinkPrefetch(input: {
nodeEnv: string | undefined;
prefetch: boolean | null | undefined;
isDangerous: boolean;
}): boolean {
return input.nodeEnv === "production" && input.prefetch !== false && !input.isDangerous;
}

export function getLinkPrefetchDecision(input: {
nodeEnv: string | undefined;
prefetch: boolean | null | undefined;
isDangerous: boolean;
intent: LinkPrefetchIntent;
}): LinkPrefetchDecision {
if (!canLinkPrefetch(input)) return { shouldPrefetch: false };

return {
shouldPrefetch: true,
priority: input.intent === "intent" ? "high" : "low",
};
}
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: getLinkPrefetchDecision is exported and tested but not used by any production code — only canLinkPrefetch and getLinkPrefetchHref are imported in link.tsx. If this is intentional as a public API for consumers or future use, that's fine, but a brief comment noting its purpose would help avoid someone removing it as dead code during cleanup.


/**
* Normalize absolute and protocol-relative Link hrefs to app-relative paths
* that are eligible for prefetching. Non-absolute relative hrefs are returned
* unchanged; callers must resolve them against the current browser URL before
* constructing a concrete fetch target.
*/
export function getLinkPrefetchHref(input: {
href: string;
basePath: string;
currentOrigin: string | undefined;
}): string | null {
const { href, basePath, currentOrigin } = input;
if (!isAbsoluteOrProtocolRelative(href)) return href;
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: for a non-absolute, non-protocol-relative href that doesn't start with / (e.g. a bare relative path like "foo" or "./bar"), this returns the raw string. That's fine because prefetchUrl and the viewport observer both run it through toBrowserNavigationHref downstream which resolves against window.location.href, but the contract of this function isn't obvious from its signature alone — it can return relative paths that aren't valid prefetch targets on their own.

A doc comment noting that callers must resolve relative results before use would help.

if (currentOrigin === undefined) return null;

try {
const current = new URL(currentOrigin);
const parsed = href.startsWith("//") ? new URL(href, current.origin) : new URL(href);
if (parsed.origin !== current.origin) return null;

if (!basePath) {
return parsed.pathname + parsed.search + parsed.hash;
}

if (!hasBasePath(parsed.pathname, basePath)) {
return null;
}

return stripBasePath(parsed.pathname, basePath) + parsed.search + parsed.hash;
} catch {
return null;
}
}

function isAbsoluteOrProtocolRelative(href: string): boolean {
return href.startsWith("http://") || href.startsWith("https://") || href.startsWith("//");
}
75 changes: 52 additions & 23 deletions packages/vinext/src/shims/link.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import React, {
useState,
type AnchorHTMLAttributes,
type MouseEvent,
type TouchEvent,
} from "react";
// Import shared RSC prefetch utilities from navigation shim (relative path
// so this resolves both via the Vite plugin and in direct vitest imports)
Expand All @@ -34,6 +35,7 @@ import {
VINEXT_RSC_MOUNTED_SLOTS_HEADER,
} from "../server/app-rsc-cache-busting.js";
import { isDangerousScheme } from "./url-safety.js";
import { canLinkPrefetch, getLinkPrefetchHref } from "./link-prefetch.js";
import {
resolveRelativeHref,
toBrowserNavigationHref,
Expand Down Expand Up @@ -118,16 +120,15 @@ function resolveHref(href: LinkProps["href"]): string {
* Uses `requestIdleCallback` (or `setTimeout` fallback) to avoid blocking
* the main thread during initial page load.
*/
function prefetchUrl(href: string): void {
function prefetchUrl(href: string, priority: "low" | "high" = "low"): void {
if (typeof window === "undefined") return;

// Normalize same-origin absolute URLs to local paths before prefetching
let prefetchHref = href;
if (href.startsWith("http://") || href.startsWith("https://") || href.startsWith("//")) {
const localPath = toSameOriginAppPath(href, __basePath);
if (localPath == null) return; // truly external — don't prefetch
prefetchHref = localPath;
}
const prefetchHref = getLinkPrefetchHref({
href,
basePath: __basePath,
currentOrigin: window.location.origin,
});
if (prefetchHref == null) return;

const fullHref = toBrowserNavigationHref(prefetchHref, window.location.href, __basePath);

Expand All @@ -154,7 +155,7 @@ function prefetchUrl(href: string): void {
fetch(rscUrl, {
headers,
credentials: "include",
priority: "low" as const,
priority,
// @ts-expect-error — purpose is a valid fetch option in some browsers
purpose: "prefetch",
}),
Expand Down Expand Up @@ -282,6 +283,8 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link(
scroll = true,
children,
onClick,
onMouseEnter,
onTouchStart,
onNavigate,
...rest
},
Expand Down Expand Up @@ -315,7 +318,11 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link(
// Prefetching: observe the element when it enters the viewport.
// prefetch={false} disables, prefetch={true} or undefined/null (default) enables.
const internalRef = useRef<HTMLAnchorElement | null>(null);
const shouldPrefetch = prefetchProp !== false && !isDangerous;
const shouldPrefetch = canLinkPrefetch({
nodeEnv: process.env.NODE_ENV,
prefetch: prefetchProp,
isDangerous,
});

const setRefs = useCallback(
(node: HTMLAnchorElement | null) => {
Expand All @@ -332,22 +339,17 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link(
const node = internalRef.current;
if (!node) return;

// Normalize same-origin absolute URLs; skip truly external ones
let hrefToPrefetch = localizedHref;
if (
localizedHref.startsWith("http://") ||
localizedHref.startsWith("https://") ||
localizedHref.startsWith("//")
) {
const localPath = toSameOriginAppPath(localizedHref, __basePath);
if (localPath == null) return; // truly external
hrefToPrefetch = localPath;
}
const hrefToPrefetch = getLinkPrefetchHref({
href: localizedHref,
basePath: __basePath,
currentOrigin: window.location.origin,
});
if (hrefToPrefetch == null) return;

const observer = getSharedObserver();
if (!observer) return;

observerCallbacks.set(node, () => prefetchUrl(hrefToPrefetch));
observerCallbacks.set(node, () => prefetchUrl(hrefToPrefetch, "low"));
observer.observe(node);

return () => {
Expand All @@ -356,6 +358,27 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link(
};
}, [shouldPrefetch, localizedHref]);

const prefetchOnIntent = useCallback(() => {
if (!shouldPrefetch) return;
prefetchUrl(localizedHref, "high");
}, [shouldPrefetch, localizedHref]);

const handleMouseEnter = useCallback(
(e: MouseEvent<HTMLAnchorElement>) => {
onMouseEnter?.(e);
prefetchOnIntent();
},
[onMouseEnter, prefetchOnIntent],
);

const handleTouchStart = useCallback(
(e: TouchEvent<HTMLAnchorElement>) => {
onTouchStart?.(e);
prefetchOnIntent();
},
[onTouchStart, prefetchOnIntent],
);

const handleClick = async (e: MouseEvent<HTMLAnchorElement>) => {
if (onClick) onClick(e);
if (e.defaultPrevented) return;
Expand Down Expand Up @@ -471,7 +494,11 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link(
if (process.env.NODE_ENV !== "production") {
console.warn(`<Link> blocked dangerous href: ${resolvedHref}`);
}
return <a {...anchorProps}>{children}</a>;
return (
<a {...anchorProps} onMouseEnter={handleMouseEnter} onTouchStart={handleTouchStart}>
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good: unlike the previous review's claim, the dangerous branch correctly uses handleMouseEnter/handleTouchStart (the wrapped handlers), not the raw props. This means both branches go through the same handler path, and prefetch is suppressed via shouldPrefetch === false in prefetchOnIntent. Clean.

{children}
</a>
);
}

return (
Expand All @@ -482,6 +509,8 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link(
onClick={(event) => {
void handleClick(event);
}}
onMouseEnter={handleMouseEnter}
onTouchStart={handleTouchStart}
{...anchorProps}
>
{children}
Expand Down
33 changes: 0 additions & 33 deletions tests/e2e/app-router/advanced.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,6 @@ async function waitForAppRouterHydration(page: Page) {
});
}

function normalizePrefetchCacheKey(key: string): string {
const [url = "", context] = key.split("\0");
const normalizedUrl = url.replace(/[?&]_rsc(?:=[^&]*)?/, "");
return context === undefined ? normalizedUrl : `${normalizedUrl}\0${context}`;
}

async function readPhotoPrefetchCacheKeys(page: Page): Promise<string[]> {
const keys = await page.evaluate(() =>
Array.from(window.__VINEXT_RSC_PREFETCH_CACHE__?.keys() ?? []).filter((key) =>
key.includes("/photos/42.rsc"),
),
);

return keys.map(normalizePrefetchCacheKey).sort();
}

test.describe("Parallel Routes", () => {
test("dashboard renders all parallel slot content", async ({ page }) => {
await page.goto(`${BASE}/dashboard`);
Expand Down Expand Up @@ -286,23 +270,6 @@ test.describe("Intercepting Routes", () => {
await expect(page.locator('[data-testid="feed-page"]')).toBeVisible();
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removed test was the only E2E-level test for interception-context cache-key separation (feed vs gallery producing separate prefetch cache entries). The unit test in tests/prefetch-cache.test.ts (line 174) still covers the cache-key logic directly, so the behavior is still protected. Worth a follow-up to restore this at the integration level once production-mode E2E testing is feasible.

await expect(page.locator('[data-testid="photo-page"]')).not.toBeVisible();
});

test("prefetches keep separate cache entries for feed and gallery interception contexts", async ({
page,
}) => {
await page.goto(`${BASE}/feed`);
await waitForAppRouterHydration(page);
await expect
.poll(async () => readPhotoPrefetchCacheKeys(page))
.toEqual(["/photos/42.rsc\u0000/feed"]);

await page.click("#gallery-link");
await page.waitForURL(`${BASE}/gallery`);
await waitForAppRouterHydration(page);
await expect
.poll(async () => readPhotoPrefetchCacheKeys(page))
.toEqual(["/photos/42.rsc\u0000/feed", "/photos/42.rsc\u0000/gallery"]);
});
});

test.describe("Route Segment Config", () => {
Expand Down
Loading
Loading