add tests & refactor timer

ztanner · ztanner · commit dd738dbc9f7c · 2025-10-17T09:58:09.000-07:00
diff --git a/packages/next/src/client/components/segment-cache-impl/cache.ts b/packages/next/src/client/components/segment-cache-impl/cache.ts
@@ -34,6 +34,7 @@ import {
   isPrefetchTaskDirty,
   type PrefetchTask,
   type PrefetchSubtaskResult,
+  startRevalidationCooldown,
 } from './scheduler'
 import { getAppBuildId } from '../../app-build-id'
 import { createHrefFromUrl } from '../router-reducer/create-href-from-url'
@@ -315,14 +316,6 @@ export function getCurrentCacheVersion(): number {
   return currentCacheVersion
 }
 
-// Timestamp of the last cache revalidation. Used to implement a delay before
-// re-prefetching visible links, to allow CDN propagation.
-let lastRevalidationTimestamp: number | null = null
-
-export function getLastRevalidationTimestamp(): number | null {
-  return lastRevalidationTimestamp
-}
-
 /**
  * Used to clear the client prefetch cache when a server action calls
  * revalidatePath or revalidateTag. Eventually we will support only clearing the
@@ -335,9 +328,8 @@ export function revalidateEntireCache(
 ) {
   currentCacheVersion++
 
-  // Record the timestamp of this revalidation so we can delay re-prefetching
-  // to allow CDN cache propagation.
-  lastRevalidationTimestamp = Date.now()
+  // Start a cooldown before re-prefetching to allow CDN cache propagation.
+  startRevalidationCooldown()
 
   // Clearing the cache also effectively rejects any pending requests, because
   // when the response is received, it gets written into a cache entry that is
diff --git a/packages/next/src/client/components/segment-cache-impl/scheduler.ts b/packages/next/src/client/components/segment-cache-impl/scheduler.ts
@@ -26,7 +26,6 @@ import {
   resetRevalidatingSegmentEntry,
   getSegmentKeypath,
   canNewFetchStrategyProvideMoreContent,
-  getLastRevalidationTimestamp,
 } from './cache'
 import type { RouteCacheKey } from './cache-key'
 import { createCacheKey } from './cache-key'
@@ -200,8 +199,29 @@ let mostRecentlyHoveredLink: PrefetchTask | null = null
 // CDN cache propagation delay after revalidation (in milliseconds)
 const REVALIDATION_COOLDOWN_MS = 300
 
-// Flag to prevent scheduling multiple cooldown timeouts
-let didScheduleCooldownTimeout = false
+// Timeout handle for the revalidation cooldown. When non-null, prefetch
+// requests are blocked to allow CDN cache propagation.
+let revalidationCooldownTimeoutHandle: ReturnType<typeof setTimeout> | null =
+  null
+
+/**
+ * Called by the cache when revalidation occurs. Starts a cooldown period
+ * during which prefetch requests are blocked to allow CDN cache propagation.
+ */
+export function startRevalidationCooldown(): void {
+  // Clear any existing timeout in case multiple revalidations happen
+  // in quick succession.
+  if (revalidationCooldownTimeoutHandle !== null) {
+    clearTimeout(revalidationCooldownTimeoutHandle)
+  }
+
+  // Schedule the cooldown to expire after the delay.
+  revalidationCooldownTimeoutHandle = setTimeout(() => {
+    revalidationCooldownTimeoutHandle = null
+    // Retry the prefetch queue now that the cooldown has expired.
+    ensureWorkIsScheduled()
+  }, REVALIDATION_COOLDOWN_MS)
+}
 
 export type IncludeDynamicData = null | 'full' | 'dynamic'
 
@@ -350,23 +370,6 @@ function ensureWorkIsScheduled() {
   scheduleMicrotask(processQueueInMicrotask)
 }
 
-/**
- * Schedules a retry of the task queue after the revalidation cooldown expires.
- * This ensures we don't prefetch stale data from CDN caches that haven't
- * propagated yet.
- */
-function scheduleRetryAfterRevalidationCooldown(
-  remainingCooldown: number
-): void {
-  if (!didScheduleCooldownTimeout) {
-    didScheduleCooldownTimeout = true
-    setTimeout(() => {
-      didScheduleCooldownTimeout = false
-      ensureWorkIsScheduled()
-    }, remainingCooldown)
-  }
-}
-
 /**
  * Checks if we've exceeded the maximum number of concurrent prefetch requests,
  * to avoid saturating the browser's internal network queue. This is a
@@ -376,18 +379,13 @@ function scheduleRetryAfterRevalidationCooldown(
  * Also checks if we're within the revalidation cooldown window, during which
  * prefetch requests are delayed to allow CDN cache propagation.
  */
-function hasNetworkBandwidth(now: number, task: PrefetchTask): boolean {
+function hasNetworkBandwidth(task: PrefetchTask): boolean {
   // Check if we're within the revalidation cooldown window
-  const lastRevalidation = getLastRevalidationTimestamp()
-  if (lastRevalidation !== null) {
-    const timeSinceRevalidation = now - lastRevalidation
-    if (timeSinceRevalidation < REVALIDATION_COOLDOWN_MS) {
-      // We're within the cooldown window. Schedule a retry after the cooldown
-      // expires, then return false to prevent prefetching.
-      const remainingCooldown = REVALIDATION_COOLDOWN_MS - timeSinceRevalidation
-      scheduleRetryAfterRevalidationCooldown(remainingCooldown)
-      return false
-    }
+  if (revalidationCooldownTimeoutHandle !== null) {
+    // We're within the cooldown window. Return false to prevent prefetching.
+    // When the cooldown expires, the timeout will call ensureWorkIsScheduled()
+    // to retry the queue.
+    return false
   }
 
   // TODO: Also check if there's an in-progress navigation. We should never
@@ -477,7 +475,7 @@ function processQueueInMicrotask() {
 
   // Process the task queue until we run out of network bandwidth.
   let task = heapPeek(taskHeap)
-  while (task !== null && hasNetworkBandwidth(now, task)) {
+  while (task !== null && hasNetworkBandwidth(task)) {
     task.cacheVersion = getCurrentCacheVersion()
 
     const exitStatus = pingRoute(now, task)
@@ -650,7 +648,7 @@ function pingRootRouteTree(
         return PrefetchTaskExitStatus.Done
       }
       // Recursively fill in the segment tree.
-      if (!hasNetworkBandwidth(now, task)) {
+      if (!hasNetworkBandwidth(task)) {
         // Stop prefetching segments until there's more bandwidth.
         return PrefetchTaskExitStatus.InProgress
       }
@@ -834,7 +832,7 @@ function pingSharedPartOfCacheComponentsTree(
   const newTreeChildren = newTree.slots
   if (newTreeChildren !== null) {
     for (const parallelRouteKey in newTreeChildren) {
-      if (!hasNetworkBandwidth(now, task)) {
+      if (!hasNetworkBandwidth(task)) {
         // Stop prefetching segments until there's more bandwidth.
         return PrefetchTaskExitStatus.InProgress
       }
@@ -930,7 +928,7 @@ function pingNewPartOfCacheComponentsTree(
   )
   pingStaticSegmentData(now, task, route, segment, task.key, tree)
   if (tree.slots !== null) {
-    if (!hasNetworkBandwidth(now, task)) {
+    if (!hasNetworkBandwidth(task)) {
       // Stop prefetching segments until there's more bandwidth.
       return PrefetchTaskExitStatus.InProgress
     }
diff --git a/test/e2e/app-dir/segment-cache/revalidation/segment-cache-revalidation.test.ts b/test/e2e/app-dir/segment-cache/revalidation/segment-cache-revalidation.test.ts
@@ -314,4 +314,54 @@ describe('segment cache (revalidation)', () => {
       'no-requests'
     )
   })
+
+  it('delay re-prefetch after revalidation to allow CDN propagation', async () => {
+    let act: ReturnType<typeof createRouterAct>
+    const browser = await next.browser('/', {
+      beforePageLoad(page) {
+        act = createRouterAct(page)
+      },
+    })
+
+    const linkVisibilityToggle = await browser.elementByCss(
+      'input[data-link-accordion="/greeting"]'
+    )
+
+    // Reveal the link the target page to trigger a prefetch
+    await act(
+      async () => {
+        await linkVisibilityToggle.click()
+      },
+      {
+        includes: 'random-greeting',
+      }
+    )
+
+    // Perform an action that calls revalidatePath. This triggers a 300ms
+    // cooldown before any new prefetch requests can be made.
+    const revalidateByPath = await browser.elementById('revalidate-by-path')
+    await revalidateByPath.click()
+
+    // Immediately after revalidation, no prefetch should have occurred yet
+    await new Promise((resolve) => setTimeout(resolve, 50))
+    TestLog.assert([])
+
+    // Halfway through cooldown (150ms), still no prefetch
+    await new Promise((resolve) => setTimeout(resolve, 100))
+    TestLog.assert([])
+
+    // After cooldown expires (300ms + buffer), prefetch should have occurred
+    await new Promise((resolve) => setTimeout(resolve, 200))
+    TestLog.assert(['REQUEST: random-greeting'])
+
+    // Navigate to the target page.
+    await act(async () => {
+      const link = await browser.elementByCss('a[href="/greeting"]')
+      await link.click()
+      // Navigation should finish immediately because the page is
+      // fully prefetched.
+      const greeting = await browser.elementById('greeting')
+      expect(await greeting.innerHTML()).toBe('random-greeting [1]')
+    }, 'no-requests')
+  })
 })
diff --git a/test/lib/router-act.ts b/test/lib/router-act.ts
@@ -309,7 +309,32 @@ export function createRouterAct(
       const remaining = new Set<PendingRSCRequest>()
       let actualResponses: Array<ExpectedResponseConfig> = []
       let alreadyMatched = new Map<string, string>()
-      while (batch.pendingRequests.size > 0) {
+
+      // Track when the queue was last empty to implement a settling period
+      let queueEmptyStartTime: number | null = null
+      const SETTLING_PERIOD_MS = 500 // Wait 500ms after queue empties
+
+      while (
+        batch.pendingRequests.size > 0 ||
+        queueEmptyStartTime === null ||
+        Date.now() - queueEmptyStartTime < SETTLING_PERIOD_MS
+      ) {
+        if (batch.pendingRequests.size > 0) {
+          // Queue has requests, reset settling timer
+          queueEmptyStartTime = null
+        } else if (queueEmptyStartTime === null) {
+          // Queue just became empty, start settling timer
+          queueEmptyStartTime = Date.now()
+        }
+
+        if (batch.pendingRequests.size === 0) {
+          // Queue is empty during settling period, wait a bit and check again
+          await new Promise((resolve) => setTimeout(resolve, 50))
+          await waitForIdleCallback()
+          await waitForPendingRequestChecks()
+          continue
+        }
+
         const pending = batch.pendingRequests
         batch.pendingRequests = new Set()
         for (const item of pending) {
