Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Prefetching lazily generated param #73715

Merged
merged 1 commit into from
Dec 10, 2024
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
6 changes: 3 additions & 3 deletions packages/next/src/export/routes/app-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export async function exportAppPage(
postponed,
fetchTags,
fetchMetrics,
segmentFlightData,
segmentData,
} = metadata

// Ensure we don't postpone without having PPR enabled.
Expand Down Expand Up @@ -200,7 +200,7 @@ export async function exportAppPage(
flightData
)

if (segmentFlightData) {
if (segmentData) {
// Emit the per-segment prefetch data. We emit them as separate files
// so that the cache handler has the option to treat each as a
// separate entry.
Expand All @@ -210,7 +210,7 @@ export async function exportAppPage(
RSC_SEGMENTS_DIR_SUFFIX
)
const tasks = []
for (const [segmentPath, buffer] of segmentFlightData.entries()) {
for (const [segmentPath, buffer] of segmentData) {
segmentPaths.push(segmentPath)
const segmentDataFilePath =
segmentPath === '/'
Expand Down
10 changes: 5 additions & 5 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2844,7 +2844,7 @@ async function prerenderToStream(

const flightData = await streamToBuffer(reactServerResult.asStream())
metadata.flightData = flightData
metadata.segmentFlightData = await collectSegmentData(
metadata.segmentData = await collectSegmentData(
flightData,
finalRenderPrerenderStore,
ComponentMod,
Expand Down Expand Up @@ -3323,7 +3323,7 @@ async function prerenderToStream(
serverPrerenderStreamResult.asStream()
)
metadata.flightData = flightData
metadata.segmentFlightData = await collectSegmentData(
metadata.segmentData = await collectSegmentData(
flightData,
finalClientPrerenderStore,
ComponentMod,
Expand Down Expand Up @@ -3455,7 +3455,7 @@ async function prerenderToStream(

if (shouldGenerateStaticFlightData(workStore)) {
metadata.flightData = flightData
metadata.segmentFlightData = await collectSegmentData(
metadata.segmentData = await collectSegmentData(
flightData,
ssrPrerenderStore,
ComponentMod,
Expand Down Expand Up @@ -3647,7 +3647,7 @@ async function prerenderToStream(
if (shouldGenerateStaticFlightData(workStore)) {
const flightData = await streamToBuffer(reactServerResult.asStream())
metadata.flightData = flightData
metadata.segmentFlightData = await collectSegmentData(
metadata.segmentData = await collectSegmentData(
flightData,
prerenderLegacyStore,
ComponentMod,
Expand Down Expand Up @@ -3801,7 +3801,7 @@ async function prerenderToStream(
reactServerPrerenderResult.asStream()
)
metadata.flightData = flightData
metadata.segmentFlightData = await collectSegmentData(
metadata.segmentData = await collectSegmentData(
flightData,
prerenderLegacyStore,
ComponentMod,
Expand Down
7 changes: 4 additions & 3 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2749,7 +2749,7 @@ export default abstract class Server<
rscData: metadata.flightData,
postponed: metadata.postponed,
status: res.statusCode,
segmentData: undefined,
segmentData: metadata.segmentData,
} satisfies CachedAppPageValue,
revalidate: metadata.revalidate,
isFallback: !!fallbackRouteParams,
Expand Down Expand Up @@ -3057,8 +3057,9 @@ export default abstract class Server<
// it's a 404 — either the segment is fully dynamic, or an invalid segment
// path was requested.
if (cacheEntry.value.segmentData) {
const matchedSegment =
cacheEntry.value.segmentData[segmentPrefetchHeader]
const matchedSegment = cacheEntry.value.segmentData.get(
segmentPrefetchHeader
)
if (matchedSegment !== undefined) {
return {
type: 'rsc',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,14 @@ export default class FileSystemCache implements CacheHandler {
)
} catch {}

let maybeSegmentData: { [segmentPath: string]: string } | undefined
let maybeSegmentData: Map<string, Buffer> | undefined
if (meta?.segmentPaths) {
// Collect all the segment data for this page.
// TODO: To optimize file system reads, we should consider creating
// separate cache entries for each segment, rather than storing them
// all on the page's entry. Though the behavior is
// identical regardless.
const segmentData: { [segmentPath: string]: string } = {}
const segmentData: Map<string, Buffer> = new Map()
maybeSegmentData = segmentData
const segmentsDir = key + RSC_SEGMENTS_DIR_SUFFIX
await Promise.all(
Expand All @@ -213,9 +213,9 @@ export default class FileSystemCache implements CacheHandler {
IncrementalCacheKind.APP_PAGE
)
try {
segmentData[segmentPath] = await this.fs.readFile(
segmentDataFilePath,
'utf8'
segmentData.set(
segmentPath,
await this.fs.readFile(segmentDataFilePath)
)
} catch {
// This shouldn't happen, but if for some reason we fail to
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/render-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export type AppPageRenderResultMetadata = {
fetchTags?: string
fetchMetrics?: FetchMetrics

segmentFlightData?: Map<string, Buffer>
segmentData?: Map<string, Buffer>

/**
* In development, the cache is warmed up before the render. This is attached
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/server/response-cache/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export interface CachedAppPageValue {
status: number | undefined
postponed: string | undefined
headers: OutgoingHttpHeaders | undefined
segmentData: { [segmentPath: string]: string } | undefined
segmentData: Map<string, Buffer> | undefined
}

export interface CachedPageValue {
Expand Down Expand Up @@ -118,7 +118,7 @@ export interface IncrementalCachedAppPageValue {
headers: OutgoingHttpHeaders | undefined
postponed: string | undefined
status: number | undefined
segmentData: { [segmentPath: string]: string } | undefined
segmentData: Map<string, Buffer> | undefined
}

export interface IncrementalCachedPageValue {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { Suspense } from 'react'

async function Content({ params }) {
const { param } = await params
return <div id="target-page-with-lazily-generated-param">Param: {param}</div>
}

export default async function Target({ params }) {
return (
<Suspense fallback="Loading...">
<Content params={params} />
</Suspense>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import Link from 'next/link'

// TODO: Once the appropriate API exists/is implemented, configure the param to
// be statically generated on demand but not at build time (`dynamicParams =
// true` isn't supported when `dynamicIO` is enabled.) For now this test case
// seems to work without extra configuration but it might not in the future.

export default function LazilyGeneratedParamsStartPage() {
return (
<>
<p>
Demonstrates that we can prefetch param that is not generated at build
time but is lazily generated on demand
</p>
<ul>
<li>
<Link href="/lazily-generated-params/some-param-value">Target</Link>
</li>
</ul>
</>
)
}
126 changes: 112 additions & 14 deletions test/e2e/app-dir/segment-cache/basic/segment-cache-basic.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { nextTestSetup } from 'e2e-utils'
import type { Page, Route } from 'playwright'
import type * as Playwright from 'playwright'

describe('segment cache (basic tests)', () => {
const { next, isNextDev, skipped } = nextTestSetup({
Expand All @@ -19,9 +19,9 @@ describe('segment cache (basic tests)', () => {
const prefetchLock = interceptor.lockPrefetches()

const browser = await next.browser('/', {
beforePageLoad(page: Page) {
page.route('**/*', async (route: Route) => {
await interceptor.interceptRoute(route)
beforePageLoad(page: Playwright.Page) {
page.route('**/*', async (route: Playwright.Route) => {
await interceptor.interceptRoute(page, route)
})
},
})
Expand All @@ -43,9 +43,9 @@ describe('segment cache (basic tests)', () => {
it('navigate with prefetched data', async () => {
const interceptor = createRequestInterceptor()
const browser = await next.browser('/', {
beforePageLoad(page: Page) {
page.route('**/*', async (route: Route) => {
await interceptor.interceptRoute(route)
beforePageLoad(page: Playwright.Page) {
page.route('**/*', async (route: Playwright.Route) => {
await interceptor.interceptRoute(page, route)
})
},
})
Expand Down Expand Up @@ -77,12 +77,48 @@ describe('segment cache (basic tests)', () => {
)
})

it('navigate to page with lazily-generated (not at build time) static param', async () => {
const interceptor = createRequestInterceptor()
const browser = await interceptor.waitForPrefetches(async () => {
const b = await next.browser('/lazily-generated-params', {
beforePageLoad(page: Playwright.Page) {
page.route('**/*', async (route: Playwright.Route) => {
await interceptor.interceptRoute(page, route)
})
},
})
await b.elementByCss('a')
return b
})

const navigationsLock = interceptor.lockNavigations()

// Navigate to the test page
const link = await browser.elementByCss('a')
await link.click()

// We should be able to render the page with the dynamic param, because
// it is lazily generated
const target = await browser.elementById(
'target-page-with-lazily-generated-param'
)
expect(await target.innerHTML()).toMatchInlineSnapshot(
`"Param: some-param-value"`
)

await navigationsLock.release()

// TODO: Once #73540 lands we can also test that the dynamic nav was skipped
// const navigations = await navigationsLock.release()
// expect(navigations.size).toBe(0)
})

it('prefetch interception route', async () => {
const interceptor = createRequestInterceptor()
const browser = await next.browser('/interception/feed', {
beforePageLoad(page: Page) {
page.route('**/*', async (route: Route) => {
await interceptor.interceptRoute(route)
beforePageLoad(page: Playwright.Page) {
page.route('**/*', async (route: Playwright.Route) => {
await interceptor.interceptRoute(page, route)
})
},
})
Expand Down Expand Up @@ -112,8 +148,11 @@ function createRequestInterceptor() {
// implementation details as much as possible, so the only thing this does
// for now is let you block and release requests from happening based on
// their type (prefetch requests, navigation requests).
let pendingPrefetches: Set<Route> | null = null
let pendingNavigations: Set<Route> | null = null
let pendingPrefetches: Set<Playwright.Route> | null = null
let pendingNavigations: Set<Playwright.Route> | null = null

let prefetchesPromise: PromiseWithResolvers<void> = null
let lastPrefetchRequest: Playwright.Request | null = null

return {
lockNavigations() {
Expand All @@ -131,6 +170,7 @@ function createRequestInterceptor() {
for (const route of routes) {
route.continue()
}
return routes
},
}
},
Expand All @@ -150,17 +190,75 @@ function createRequestInterceptor() {
for (const route of routes) {
route.continue()
}
return routes
},
}
},

async interceptRoute(route: Route) {
const requestHeaders = await route.request().allHeaders()
/**
* Waits for the next for the next prefetch request, then keeps waiting
* until the prefetch queue is empty (to account for network throttling).
*
* If no prefetches are initiated, this will timeout.
*/
async waitForPrefetches<T>(
scope: () => Promise<T> | T = (): undefined => {}
): Promise<T> {
if (prefetchesPromise === null) {
let resolve
let reject
const promise: Promise<void> = new Promise((res, rej) => {
resolve = res
reject = rej
})
prefetchesPromise = {
resolve,
reject,
promise,
}
}
const result = await scope()
if (prefetchesPromise !== null) {
await prefetchesPromise.promise
}
return result
},

async interceptRoute(page: Playwright.Page, route: Playwright.Route) {
const request = route.request()
const requestHeaders = await request.allHeaders()

if (requestHeaders['RSC'.toLowerCase()]) {
// This is an RSC request. Check if it's a prefetch or a navigation.
if (requestHeaders['Next-Router-Prefetch'.toLowerCase()]) {
// This is a prefetch request.
if (prefetchesPromise !== null) {
// Wait for the prefetch response to finish, then wait an additional
// async task for additional prefetches to be initiated.
lastPrefetchRequest = request
const waitForMorePrefetches = async () => {
const response = await request.response()
await response.finished()
await page.evaluate(
() =>
// If the prefetch queue is network throttled, the next
// request should be issued within a microtask of the previous
// one finishing.
new Promise<void>((res) => requestIdleCallback(() => res()))
)
if (request === lastPrefetchRequest) {
// No further prefetches were initiated. Assume the prefetch
// queue is now empty.
prefetchesPromise.resolve()
prefetchesPromise = null
lastPrefetchRequest = null
}
}
waitForMorePrefetches().then(
() => {},
() => {}
)
}
if (pendingPrefetches !== null) {
pendingPrefetches.add(route)
return
Expand Down
Loading