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
23 changes: 20 additions & 3 deletions packages/vinext/src/shims/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ function withBasePath(p: string): string {
// ---------------------------------------------------------------------------

/** Maximum number of entries in the RSC prefetch cache. */
const MAX_PREFETCH_CACHE_SIZE = 50;
export const MAX_PREFETCH_CACHE_SIZE = 50;

/** TTL for prefetch cache entries in ms (matches Next.js static prefetch TTL). */
export const PREFETCH_CACHE_TTL = 30_000;
Expand Down Expand Up @@ -220,11 +220,28 @@ export function getPrefetchedUrls(): Set<string> {
*/
export function storePrefetchResponse(rscUrl: string, response: Response): void {
const cache = getPrefetchCache();
// Evict oldest entry if at capacity (Map iterates in insertion order)

// Sweep expired entries before resorting to FIFO eviction
if (cache.size >= MAX_PREFETCH_CACHE_SIZE) {
const now = Date.now();
const prefetched = getPrefetchedUrls();
for (const [key, entry] of cache) {
if (now - entry.timestamp >= PREFETCH_CACHE_TTL) {
cache.delete(key);
prefetched.delete(key);
}
}
}
Comment on lines +224 to +234
Copy link
Contributor

Choose a reason for hiding this comment

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

Since now is needed both for the sweep and for the cache.set at the end, consider hoisting it above the if:

Suggested change
// Sweep expired entries before resorting to FIFO eviction
if (cache.size >= MAX_PREFETCH_CACHE_SIZE) {
const now = Date.now();
const prefetched = getPrefetchedUrls();
for (const [key, entry] of cache) {
if (now - entry.timestamp >= PREFETCH_CACHE_TTL) {
cache.delete(key);
prefetched.delete(key);
}
}
}
const now = Date.now();
// Sweep expired entries before resorting to FIFO eviction
if (cache.size >= MAX_PREFETCH_CACHE_SIZE) {
const prefetched = getPrefetchedUrls();
for (const [key, entry] of cache) {
if (now - entry.timestamp >= PREFETCH_CACHE_TTL) {
cache.delete(key);
prefetched.delete(key);
}
}
}

Then reuse now on line 245 instead of calling Date.now() again.


// FIFO fallback if still at capacity after sweep
if (cache.size >= MAX_PREFETCH_CACHE_SIZE) {
const oldest = cache.keys().next().value;
if (oldest !== undefined) cache.delete(oldest);
if (oldest !== undefined) {
cache.delete(oldest);
getPrefetchedUrls().delete(oldest);
}
}

cache.set(rscUrl, { response, timestamp: Date.now() });
Copy link
Contributor

Choose a reason for hiding this comment

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

Date.now() is called twice per storePrefetchResponse invocation — once on line 226 for the sweep comparison, and again here for the new entry's timestamp. In production the two calls return different values (milliseconds apart), which is a minor inconsistency. Hoist now to the top of the function and reuse it:

Suggested change
cache.set(rscUrl, { response, timestamp: Date.now() });
cache.set(rscUrl, { response, timestamp: now });

This requires moving const now = Date.now() above the first if block so it's always available. Avoids the subtle issue where the sweep's "now" and the stored entry's "now" can diverge.

}

Expand Down
146 changes: 146 additions & 0 deletions tests/prefetch-cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/**
* Prefetch cache eviction tests.
*
* Verifies that storePrefetchResponse() sweeps expired entries before
* falling back to FIFO eviction, preventing expired entries from wasting
* cache slots on link-heavy pages.
*
* The navigation module computes `isServer = typeof window === "undefined"`
* at load time, so we must set globalThis.window BEFORE importing it via
* vi.resetModules() + dynamic import().
*/
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";

type Navigation = typeof import("../packages/vinext/src/shims/navigation.js");
let storePrefetchResponse: Navigation["storePrefetchResponse"];
let getPrefetchCache: Navigation["getPrefetchCache"];
let getPrefetchedUrls: Navigation["getPrefetchedUrls"];
let MAX_PREFETCH_CACHE_SIZE: Navigation["MAX_PREFETCH_CACHE_SIZE"];
let PREFETCH_CACHE_TTL: Navigation["PREFETCH_CACHE_TTL"];

beforeEach(async () => {
// Set window BEFORE importing so isServer evaluates to false
(globalThis as any).window = {
__VINEXT_RSC_PREFETCH_CACHE__: new Map(),
__VINEXT_RSC_PREFETCHED_URLS__: new Set(),
location: { pathname: "/", search: "", hash: "", href: "http://localhost/" },
addEventListener: () => {},
history: { pushState: () => {}, replaceState: () => {}, state: null },
dispatchEvent: () => {},
};
vi.resetModules();
const nav = await import("../packages/vinext/src/shims/navigation.js");
storePrefetchResponse = nav.storePrefetchResponse;
getPrefetchCache = nav.getPrefetchCache;
getPrefetchedUrls = nav.getPrefetchedUrls;
MAX_PREFETCH_CACHE_SIZE = nav.MAX_PREFETCH_CACHE_SIZE;
PREFETCH_CACHE_TTL = nav.PREFETCH_CACHE_TTL;
});

afterEach(() => {
vi.restoreAllMocks();
delete (globalThis as any).window;
});

/** Helper: fill cache with `count` entries at a given timestamp. */
function fillCache(count: number, timestamp: number, keyPrefix = "/page-"): void {
const cache = getPrefetchCache();
const prefetched = getPrefetchedUrls();
for (let i = 0; i < count; i++) {
const key = `${keyPrefix}${i}.rsc`;
cache.set(key, { response: new Response(`body-${i}`), timestamp });
prefetched.add(key);
}
}

describe("prefetch cache eviction", () => {
it("sweeps all expired entries before FIFO", () => {
const now = Date.now();
Copy link
Contributor

Choose a reason for hiding this comment

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

All four tests use Date.now() from the real clock to compute expired/now, then mock Date.now later for the storePrefetchResponse call. This works because both values are captured within the same synchronous tick, but it's subtly fragile — the test's correctness depends on the real clock and the mocked clock being close enough.

More robust: use a fixed arbitrary value throughout:

const now = 1_000_000;
const expired = now - PREFETCH_CACHE_TTL - 1_000;

This eliminates any dependency on wall-clock time and makes the tests fully deterministic.

const expired = now - PREFETCH_CACHE_TTL - 1_000; // 31s ago

fillCache(MAX_PREFETCH_CACHE_SIZE, expired);
expect(getPrefetchCache().size).toBe(MAX_PREFETCH_CACHE_SIZE);
expect(getPrefetchedUrls().size).toBe(MAX_PREFETCH_CACHE_SIZE);

vi.spyOn(Date, "now").mockReturnValue(now);
storePrefetchResponse("/new.rsc", new Response("new"));

const cache = getPrefetchCache();
expect(cache.size).toBe(1);
expect(cache.has("/new.rsc")).toBe(true);
// All evicted entries should be removed from prefetched URL set
expect(getPrefetchedUrls().size).toBe(0);
});

it("falls back to FIFO when all entries are fresh", () => {
const now = Date.now();

fillCache(MAX_PREFETCH_CACHE_SIZE, now);
expect(getPrefetchCache().size).toBe(MAX_PREFETCH_CACHE_SIZE);
expect(getPrefetchedUrls().size).toBe(MAX_PREFETCH_CACHE_SIZE);

vi.spyOn(Date, "now").mockReturnValue(now);
storePrefetchResponse("/new.rsc", new Response("new"));

const cache = getPrefetchCache();
// FIFO evicted one, new one added → still at capacity
expect(cache.size).toBe(MAX_PREFETCH_CACHE_SIZE);
expect(cache.has("/new.rsc")).toBe(true);
// First inserted entry should be evicted
expect(cache.has("/page-0.rsc")).toBe(false);
// Second entry should survive
expect(cache.has("/page-1.rsc")).toBe(true);
// FIFO-evicted entry should be removed from prefetched URL set
expect(getPrefetchedUrls().size).toBe(MAX_PREFETCH_CACHE_SIZE - 1);
expect(getPrefetchedUrls().has("/page-0.rsc")).toBe(false);
});

it("sweeps only expired entries when cache has a mix", () => {
const now = Date.now();
const expired = now - PREFETCH_CACHE_TTL - 1_000;

const half = Math.floor(MAX_PREFETCH_CACHE_SIZE / 2);
const rest = MAX_PREFETCH_CACHE_SIZE - half;

fillCache(half, expired, "/expired-");
fillCache(rest, now, "/fresh-");
expect(getPrefetchCache().size).toBe(MAX_PREFETCH_CACHE_SIZE);
expect(getPrefetchedUrls().size).toBe(MAX_PREFETCH_CACHE_SIZE);

vi.spyOn(Date, "now").mockReturnValue(now);
storePrefetchResponse("/new.rsc", new Response("new"));

const cache = getPrefetchCache();
// expired swept, fresh kept, 1 new added
expect(cache.size).toBe(rest + 1);
expect(cache.has("/new.rsc")).toBe(true);

// All expired entries should be gone
for (let i = 0; i < half; i++) {
expect(cache.has(`/expired-${i}.rsc`)).toBe(false);
}
// All fresh entries should survive
for (let i = 0; i < rest; i++) {
expect(cache.has(`/fresh-${i}.rsc`)).toBe(true);
}
// Only fresh entries remain in prefetched URL set
expect(getPrefetchedUrls().size).toBe(rest);
});

it("does not sweep when cache is below capacity", () => {
const now = Date.now();
const expired = now - PREFETCH_CACHE_TTL - 1_000;

const belowCapacity = MAX_PREFETCH_CACHE_SIZE - 1;
fillCache(belowCapacity, expired);

vi.spyOn(Date, "now").mockReturnValue(now);
storePrefetchResponse("/new.rsc", new Response("new"));

const cache = getPrefetchCache();
// Below capacity — no eviction, all entries kept + 1 new
expect(cache.size).toBe(belowCapacity + 1);
// Prefetched URL set unchanged (no eviction triggered)
expect(getPrefetchedUrls().size).toBe(belowCapacity);
Copy link
Contributor

Choose a reason for hiding this comment

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

This assertion is correct but potentially confusing to a reader: the new entry (/new.rsc) is in the cache but not in prefetchedUrls, so the size stays at belowCapacity rather than belowCapacity + 1. This is because storePrefetchResponse only manages the cache — the caller (router.prefetch()) manages prefetchedUrls separately. Worth a brief comment explaining why the count doesn't include the new entry.

});
});
Loading