diff --git a/packages/next/src/client/app-dir/link.tsx b/packages/next/src/client/app-dir/link.tsx index 183ca919dba179..adf90964deb755 100644 --- a/packages/next/src/client/app-dir/link.tsx +++ b/packages/next/src/client/app-dir/link.tsx @@ -17,6 +17,7 @@ import { schedulePrefetchTask as scheduleSegmentPrefetchTask, cancelPrefetchTask, bumpPrefetchTask, + PrefetchPriority, } from '../components/segment-cache/scheduler' import { getCurrentAppRouterState } from '../../shared/lib/router/action-queue' import { createCacheKey } from '../components/segment-cache/cache-key' @@ -125,6 +126,7 @@ type LinkInstance = { prefetchHref: string isVisible: boolean + wasHoveredOrTouched: boolean // The most recently initiated prefetch task. It may or may not have // already completed. The same prefetch task object can be reused across @@ -181,6 +183,7 @@ function mountLinkInstance( router, kind, isVisible: false, + wasHoveredOrTouched: false, prefetchTask: null, } const existingInstance = links.get(element) @@ -247,6 +250,7 @@ function onNavigationIntent(element: HTMLAnchorElement | SVGAElement) { } // Prefetch the link on hover/touchstart. if (instance !== undefined) { + instance.wasHoveredOrTouched = true rescheduleLinkPrefetch(instance) } } @@ -274,10 +278,15 @@ function rescheduleLinkPrefetch(instance: LinkInstance) { return } - // In the Segment Cache implementation, we increase the relative priority of - // links whenever they re-enter the viewport, as if they were being scheduled - // for the first time. - // TODO: Prioritize links that are hovered. + // In the Segment Cache implementation, we assign a higher priority level to + // links that were at one point hovered or touched. Since the queue is last- + // in-first-out, the highest priority Link is whichever one was hovered last. + // + // We also increase the relative priority of links whenever they re-enter the + // viewport, as if they were being scheduled for the first time. + const priority = instance.wasHoveredOrTouched + ? PrefetchPriority.Intent + : PrefetchPriority.Default if (existingPrefetchTask === null) { // Initiate a prefetch task. const appRouterState = getCurrentAppRouterState() @@ -288,13 +297,14 @@ function rescheduleLinkPrefetch(instance: LinkInstance) { instance.prefetchTask = scheduleSegmentPrefetchTask( cacheKey, treeAtTimeOfPrefetch, - instance.kind === PrefetchKind.FULL + instance.kind === PrefetchKind.FULL, + priority ) } } else { // We already have an old task object that we can reschedule. This is // effectively the same as canceling the old task and creating a new one. - bumpPrefetchTask(existingPrefetchTask) + bumpPrefetchTask(existingPrefetchTask, priority) } } diff --git a/packages/next/src/client/components/segment-cache/prefetch.ts b/packages/next/src/client/components/segment-cache/prefetch.ts index 7f7fa0c2607e3f..00d1bd7ca33cff 100644 --- a/packages/next/src/client/components/segment-cache/prefetch.ts +++ b/packages/next/src/client/components/segment-cache/prefetch.ts @@ -1,7 +1,7 @@ import type { FlightRouterState } from '../../../server/app-render/types' import { createPrefetchURL } from '../../components/app-router' import { createCacheKey } from './cache-key' -import { schedulePrefetchTask } from './scheduler' +import { schedulePrefetchTask, PrefetchPriority } from './scheduler' /** * Entrypoint for prefetching a URL into the Segment Cache. @@ -26,5 +26,10 @@ export function prefetch( return } const cacheKey = createCacheKey(url.href, nextUrl) - schedulePrefetchTask(cacheKey, treeAtTimeOfPrefetch, includeDynamicData) + schedulePrefetchTask( + cacheKey, + treeAtTimeOfPrefetch, + includeDynamicData, + PrefetchPriority.Default + ) } diff --git a/packages/next/src/client/components/segment-cache/scheduler.ts b/packages/next/src/client/components/segment-cache/scheduler.ts index e784b609651556..3c3ea868c6e833 100644 --- a/packages/next/src/client/components/segment-cache/scheduler.ts +++ b/packages/next/src/client/components/segment-cache/scheduler.ts @@ -131,8 +131,21 @@ const enum PrefetchTaskExitStatus { /** * The priority of the prefetch task. Higher numbers are higher priority. */ -const enum PrefetchPriority { +export const enum PrefetchPriority { + /** + * Assigned to any visible link that was hovered/touched at some point. This + * is not removed on mouse exit, because a link that was momentarily + * hovered is more likely to to be interacted with than one that was not. + */ + Intent = 2, + /** + * The default priority for prefetch tasks. + */ Default = 1, + /** + * Assigned to tasks when they spawn non-blocking background work, like + * revalidating a partially cached entry to see if more data is available. + */ Background = 0, } @@ -169,13 +182,14 @@ let didScheduleMicrotask = false export function schedulePrefetchTask( key: RouteCacheKey, treeAtTimeOfPrefetch: FlightRouterState, - includeDynamicData: boolean + includeDynamicData: boolean, + priority: PrefetchPriority ): PrefetchTask { // Spawn a new prefetch task const task: PrefetchTask = { key, treeAtTimeOfPrefetch, - priority: PrefetchPriority.Default, + priority, hasBackgroundWork: false, includeDynamicData, sortId: sortIdCounter++, @@ -206,19 +220,25 @@ export function cancelPrefetchTask(task: PrefetchTask): void { heapDelete(taskHeap, task) } -export function bumpPrefetchTask(task: PrefetchTask): void { +export function bumpPrefetchTask( + task: PrefetchTask, + priority: PrefetchPriority +): void { // Bump the prefetch task to the top of the queue, as if it were a fresh // task. This is essentially the same as canceling the task and scheduling // a new one, except it reuses the original object. // - // The primary use case is to increase the relative priority of a Link- - // initated prefetch on hover. + // The primary use case is to increase the priority of a Link-initated + // prefetch on hover. // Un-cancel the task, in case it was previously canceled. task.isCanceled = false - // Assign a new sort ID. Higher sort IDs are higher priority. + // Assign a new sort ID to move it ahead of all other tasks at the same + // priority level. (Higher sort IDs are processed first.) task.sortId = sortIdCounter++ + task.priority = priority + if (task._heapIndex !== -1) { // The task is already in the queue. heapResift(taskHeap, task) diff --git a/test/e2e/app-dir/segment-cache/prefetch-scheduling/prefetch-scheduling.test.ts b/test/e2e/app-dir/segment-cache/prefetch-scheduling/prefetch-scheduling.test.ts index ad69feb831cd31..20ec7449acde6b 100644 --- a/test/e2e/app-dir/segment-cache/prefetch-scheduling/prefetch-scheduling.test.ts +++ b/test/e2e/app-dir/segment-cache/prefetch-scheduling/prefetch-scheduling.test.ts @@ -13,13 +13,6 @@ describe('segment cache prefetch scheduling', () => { } it('increases the priority of a viewport-initiated prefetch on hover', async () => { - // TODO: This works because we bump the prefetch task to the front of the - // queue on mouseenter. But there's a flaw: if another link enters the - // viewport while the first link is still being hovered, the second link - // will go ahead of it in the queue. In other words, we currently don't - // treat mouseenter as a higher priority signal than "viewport enter". To - // fix this, we need distinct priority levels for hover and viewport; the - // last-in-first-out strategy is not sufficient for the desired behavior. let act: ReturnType const browser = await next.browser('/cancellation', { beforePageLoad(p: Playwright.Page) { @@ -59,6 +52,59 @@ describe('segment cache prefetch scheduling', () => { ) }) + it( + 'even on mouseexit, any link that was previously hovered is prioritized ' + + 'over links that were never hovered at all', + async () => { + let act: ReturnType + const browser = await next.browser('/cancellation', { + beforePageLoad(p: Playwright.Page) { + act = createRouterAct(p) + }, + }) + + const checkbox = await browser.elementByCss('input[type="checkbox"]') + await act( + async () => { + // Reveal the links to start prefetching, but block the responses from + // reaching the client. This will initiate prefetches for the route + // trees, but it won't start prefetching any segment data yet until the + // trees have loaded. + await act(async () => { + await checkbox.click() + }, 'block') + + // Hover over a link to increase its relative priority. + const link2 = await browser.elementByCss('a[href="/cancellation/2"]') + await link2.hover() + + // Hover over a different link to increase its relative priority. + const link5 = await browser.elementByCss('a[href="/cancellation/5"]') + await link5.hover() + + // Click on the "Show More Links" button to reveal additional links. + // Even though these links are newer than the ones we hovered over, + // the hovered links should be prefetched first. + const showMoreLinksButton = + await browser.elementById('show-more-links') + await showMoreLinksButton.click() + }, + // Assert that the segment data is prefetched in the expected order. + [ + // The last link we hovered over should be the first to prefetch. + { includes: 'Content of page 5' }, + // The second-to-last link we hovered over should come next. + { includes: 'Content of page 2' }, + // Then assert on one of the links that were revealed when we click + // the "Show More Links" button + { includes: 'Content of page 10' }, + // Then assert on one of the other links that were revealed originally + { includes: 'Content of page 4' }, + ] + ) + } + ) + it( 'cancels a viewport-initiated prefetch if the link leaves the viewport ' + 'before it finishes',