diff --git a/packages/next/src/client/components/segment-cache/cache.ts b/packages/next/src/client/components/segment-cache/cache.ts index e053fd7c09aec..bfee5ce19f756 100644 --- a/packages/next/src/client/components/segment-cache/cache.ts +++ b/packages/next/src/client/components/segment-cache/cache.ts @@ -241,7 +241,7 @@ export type PendingSegmentCacheEntry = SegmentCacheEntryShared & { status: EntryStatus.Pending rsc: null loading: null - isPartial: true + isPartial: boolean promise: null | PromiseWithResolvers } @@ -847,6 +847,14 @@ export function upgradeToPendingSegment( const pendingEntry: PendingSegmentCacheEntry = emptyEntry as any pendingEntry.status = EntryStatus.Pending pendingEntry.fetchStrategy = fetchStrategy + + if (fetchStrategy === FetchStrategy.Full) { + // We can assume the response will contain the full segment data. Set this + // to false so we know it's OK to omit this segment from any navigation + // requests that may happen while the data is still pending. + pendingEntry.isPartial = false + } + // Set the version here, since this is right before the request is initiated. // The next time the global cache version is incremented, the entry will // effectively be evicted. This happens before initiating the request, rather diff --git a/packages/next/src/client/components/segment-cache/navigation.ts b/packages/next/src/client/components/segment-cache/navigation.ts index fd7c2d41245cc..c941d2badcf3f 100644 --- a/packages/next/src/client/components/segment-cache/navigation.ts +++ b/packages/next/src/client/components/segment-cache/navigation.ts @@ -444,10 +444,17 @@ function readRenderSnapshotFromCache( loading = promiseForFulfilledEntry.then((entry) => entry !== null ? entry.loading : null ) - // Since we don't know yet whether the segment is partial or fully - // static, we must assume it's partial; we can't skip the - // dynamic request. - isPartial = true + // Because the request is still pending, we typically don't know yet + // whether the response will be partial. We shouldn't skip this segment + // during the dynamic navigation request. Otherwise, we might need to + // do yet another request to fill in the remaining data, creating + // a waterfall. + // + // The one exception is if this segment is being fetched with via + // prefetch={true} (i.e. the "force stale" or "full" strategy). If so, + // we can assume the response will be full. This field is set to `false` + // for such segments. + isPartial = segmentEntry.isPartial break } case EntryStatus.Empty: @@ -508,7 +515,7 @@ function readHeadSnapshotFromCache( rsc = promiseForFulfilledEntry.then((entry) => entry !== null ? entry.rsc : null ) - isPartial = true + isPartial = segmentEntry.isPartial break } case EntryStatus.Empty: diff --git a/test/e2e/app-dir/segment-cache/force-stale/app/dynamic/page.tsx b/test/e2e/app-dir/segment-cache/force-stale/app/dynamic/page.tsx new file mode 100644 index 0000000000000..990c798b5213d --- /dev/null +++ b/test/e2e/app-dir/segment-cache/force-stale/app/dynamic/page.tsx @@ -0,0 +1,15 @@ +import { connection } from 'next/server' +import { Suspense } from 'react' + +async function PageContent() { + await connection() + return
Dynamic page content
+} + +export default async function Page() { + return ( + Loading...}> + + + ) +} diff --git a/test/e2e/app-dir/segment-cache/force-stale/app/layout.tsx b/test/e2e/app-dir/segment-cache/force-stale/app/layout.tsx new file mode 100644 index 0000000000000..dbce4ea8e3aeb --- /dev/null +++ b/test/e2e/app-dir/segment-cache/force-stale/app/layout.tsx @@ -0,0 +1,11 @@ +export default function RootLayout({ + children, +}: { + children: React.ReactNode +}) { + return ( + + {children} + + ) +} diff --git a/test/e2e/app-dir/segment-cache/force-stale/app/page.tsx b/test/e2e/app-dir/segment-cache/force-stale/app/page.tsx new file mode 100644 index 0000000000000..7fdcafe0eec2d --- /dev/null +++ b/test/e2e/app-dir/segment-cache/force-stale/app/page.tsx @@ -0,0 +1,9 @@ +import { LinkAccordion } from '../components/link-accordion' + +export default function Page() { + return ( + + Dynamic page + + ) +} diff --git a/test/e2e/app-dir/segment-cache/force-stale/components/link-accordion.tsx b/test/e2e/app-dir/segment-cache/force-stale/components/link-accordion.tsx new file mode 100644 index 0000000000000..c6848d479aef8 --- /dev/null +++ b/test/e2e/app-dir/segment-cache/force-stale/components/link-accordion.tsx @@ -0,0 +1,33 @@ +'use client' + +import Link, { type LinkProps } from 'next/link' +import { useState } from 'react' + +export function LinkAccordion({ + href, + children, + prefetch, +}: { + href: string + children: React.ReactNode + prefetch?: LinkProps['prefetch'] +}) { + const [isVisible, setIsVisible] = useState(false) + return ( + <> + setIsVisible(!isVisible)} + data-link-accordion={href} + /> + {isVisible ? ( + + {children} + + ) : ( + <>{children} (link is hidden) + )} + + ) +} diff --git a/test/e2e/app-dir/segment-cache/force-stale/force-stale.test.ts b/test/e2e/app-dir/segment-cache/force-stale/force-stale.test.ts new file mode 100644 index 0000000000000..447516b69f5fd --- /dev/null +++ b/test/e2e/app-dir/segment-cache/force-stale/force-stale.test.ts @@ -0,0 +1,57 @@ +import { nextTestSetup } from 'e2e-utils' +import type * as Playwright from 'playwright' +import { createRouterAct } from 'router-act' + +describe('force stale', () => { + const { next, isNextDev } = nextTestSetup({ + files: __dirname, + }) + if (isNextDev) { + test('prefetching is disabled in dev', () => {}) + return + } + + it( + 'during a navigation, don\'t request segments that have a pending "full" ' + + 'prefetch already in progress', + async () => { + let act: ReturnType + const browser = await next.browser('/', { + beforePageLoad(p: Playwright.Page) { + act = createRouterAct(p) + }, + }) + + await act( + async () => { + // Reveal a link to a dynamic page. The Link has prefetch={true}, so the + // full page data is prefetched, including dynamic content. + const toggleLinkVisibility = await browser.elementByCss( + 'input[data-link-accordion="/dynamic"]' + ) + await act(async () => await toggleLinkVisibility.click(), { + includes: 'Dynamic page content', + // Block the data from loading into the client so we can test what + // happens if we request the same segment again during a navigation. + block: true, + }) + + // Initiate a navigation to the dynamic page. Even though the dynamic + // content from the prefetch hasn't loaded yet, the router should not + // request the same segment again, because it knows the data it + // receives from the prefetch will be complete. This assumption is + // _only_ correct for "full" prefetches, because we explicitly instruct + // the server not to omit any dynamic or runtime data. + const link = await browser.elementByCss('a[href="/dynamic"]') + await link.click() + }, + // There should have been no additional requests upon navigation + 'no-requests' + ) + + // The data succesfully streams in. + const content = await browser.elementById('dynamic-page-content') + expect(await content.text()).toBe('Dynamic page content') + } + ) +}) diff --git a/test/e2e/app-dir/segment-cache/force-stale/next.config.js b/test/e2e/app-dir/segment-cache/force-stale/next.config.js new file mode 100644 index 0000000000000..e64bae22d6580 --- /dev/null +++ b/test/e2e/app-dir/segment-cache/force-stale/next.config.js @@ -0,0 +1,8 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = { + cacheComponents: true, +} + +module.exports = nextConfig diff --git a/test/lib/router-act.ts b/test/lib/router-act.ts index cefe17bad31c7..204890c917c90 100644 --- a/test/lib/router-act.ts +++ b/test/lib/router-act.ts @@ -77,7 +77,29 @@ export function createRouterAct( for (let attempt = 0; attempt < maxRetries; attempt++) { try { await page.evaluate( - () => new Promise((res) => requestIdleCallback(() => res())) + () => + new Promise((res) => + requestIdleCallback(() => res(), { + // Add a timeout option to prevents the callback from being + // backgrounded indefinitely. Not sure why this happens but + // without it, the callback will never fire. + // + // Note that this does not delay the callback from firing. + // It should still fire pretty much "immediately". It's just a + // safeguard in case the idle callback queue is not fired within + // a reasonable amount of time. It really shouldn't + // be necessary. + // + // TODO: I'm getting increasingly frustrated by how flaky + // Playwright's APIs are. At this point I'm convinced we should + // rewrite the whole router-act module to an equivalent + // implementation that runs directly in the browser, by + // injecting a script into the page. Since we only use it for + // our own contrived e2e test apps, we can just import the + // script into each test app that needs it. + timeout: 100, + }) + ) ) return } catch (err) {