Skip to content
Merged
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
10 changes: 9 additions & 1 deletion packages/next/src/client/components/segment-cache/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export type PendingSegmentCacheEntry = SegmentCacheEntryShared & {
status: EntryStatus.Pending
rsc: null
loading: null
isPartial: true
isPartial: boolean
promise: null | PromiseWithResolvers<FulfilledSegmentCacheEntry | null>
}

Expand Down Expand Up @@ -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
Expand Down
17 changes: 12 additions & 5 deletions packages/next/src/client/components/segment-cache/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -508,7 +515,7 @@ function readHeadSnapshotFromCache(
rsc = promiseForFulfilledEntry.then((entry) =>
entry !== null ? entry.rsc : null
)
isPartial = true
isPartial = segmentEntry.isPartial
break
}
case EntryStatus.Empty:
Expand Down
15 changes: 15 additions & 0 deletions test/e2e/app-dir/segment-cache/force-stale/app/dynamic/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { connection } from 'next/server'
import { Suspense } from 'react'

async function PageContent() {
await connection()
return <div id="dynamic-page-content">Dynamic page content</div>
}

export default async function Page() {
return (
<Suspense fallback={<div>Loading...</div>}>
<PageContent />
</Suspense>
)
}
11 changes: 11 additions & 0 deletions test/e2e/app-dir/segment-cache/force-stale/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export default function RootLayout({
children,
}: {
children: React.ReactNode
}) {
return (
<html lang="en">
<body>{children}</body>
</html>
)
}
9 changes: 9 additions & 0 deletions test/e2e/app-dir/segment-cache/force-stale/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { LinkAccordion } from '../components/link-accordion'

export default function Page() {
return (
<LinkAccordion href="/dynamic" prefetch={true}>
Dynamic page
</LinkAccordion>
)
}
Original file line number Diff line number Diff line change
@@ -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 (
<>
<input
type="checkbox"
checked={isVisible}
onChange={() => setIsVisible(!isVisible)}
data-link-accordion={href}
/>
{isVisible ? (
<Link href={href} prefetch={prefetch}>
{children}
</Link>
) : (
<>{children} (link is hidden)</>
)}
</>
)
}
57 changes: 57 additions & 0 deletions test/e2e/app-dir/segment-cache/force-stale/force-stale.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof createRouterAct>
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')
}
)
})
8 changes: 8 additions & 0 deletions test/e2e/app-dir/segment-cache/force-stale/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {
cacheComponents: true,
}

module.exports = nextConfig
24 changes: 23 additions & 1 deletion test/lib/router-act.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,29 @@ export function createRouterAct(
for (let attempt = 0; attempt < maxRetries; attempt++) {
try {
await page.evaluate(
() => new Promise<void>((res) => requestIdleCallback(() => res()))
() =>
new Promise<void>((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) {
Expand Down
Loading