Skip to content

Commit

Permalink
initially visited static pages should respect static staletime (#70640)
Browse files Browse the repository at this point in the history
This provides information to the initial SSR render that the client
leverages when seeding the prefetch cache, to determine if a page was a
fully static prerender.

The side effect of not doing this is that if we seeded a static page
during SSR, and then navigated to it, the router would make a request
for data that it didn't need. In other words, it didn't honor the static
staletime, like other navigations would.
  • Loading branch information
ztanner authored Oct 1, 2024
1 parent 1a7f8d2 commit 98ffc0e
Show file tree
Hide file tree
Showing 11 changed files with 84 additions and 20 deletions.
1 change: 1 addition & 0 deletions packages/next/src/client/app-index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ const pendingActionQueue: Promise<AppRouterActionQueue> = new Promise(
location: window.location,
couldBeIntercepted: initialRSCPayload.i,
postponed: initialRSCPayload.s,
prerendered: initialRSCPayload.S,
})
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('createInitialRouterState', () => {
location: new URL('/linking', 'https://localhost') as any,
couldBeIntercepted: false,
postponed: false,
prerendered: false,
})

const state2 = createInitialRouterState({
Expand All @@ -55,6 +56,7 @@ describe('createInitialRouterState', () => {
location: new URL('/linking', 'https://localhost') as any,
couldBeIntercepted: false,
postponed: false,
prerendered: false,
})

const expectedCache: CacheNode = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface InitialRouterStateParameters {
location: Location | null
couldBeIntercepted: boolean
postponed: boolean
prerendered: boolean
}

export function createInitialRouterState({
Expand All @@ -27,6 +28,7 @@ export function createInitialRouterState({
location,
couldBeIntercepted,
postponed,
prerendered,
}: InitialRouterStateParameters) {
// When initialized on the server, the canonical URL is provided as an array of parts.
// This is to ensure that when the RSC payload streamed to the client, crawlers don't interpret it
Expand Down Expand Up @@ -118,14 +120,13 @@ export function createInitialRouterState({
flightData: [normalizedFlightData],
canonicalUrl: undefined,
couldBeIntercepted: !!couldBeIntercepted,
// TODO: the server should probably send a value for this. Default to false for now.
isPrerender: false,
prerendered,
postponed,
},
tree: initialState.tree,
prefetchCache: initialState.prefetchCache,
nextUrl: initialState.nextUrl,
kind: PrefetchKind.AUTO,
kind: prerendered ? PrefetchKind.FULL : PrefetchKind.AUTO,
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
RSC_HEADER,
RSC_CONTENT_TYPE_HEADER,
NEXT_HMR_REFRESH_HEADER,
NEXT_IS_PRERENDER_HEADER,
NEXT_DID_POSTPONE_HEADER,
} from '../app-router-headers'
import { callServer } from '../../app-call-server'
Expand All @@ -46,7 +45,7 @@ export type FetchServerResponseResult = {
flightData: NormalizedFlightData[] | string
canonicalUrl: URL | undefined
couldBeIntercepted: boolean
isPrerender: boolean
prerendered: boolean
postponed: boolean
}

Expand All @@ -72,7 +71,7 @@ function doMpaNavigation(url: string): FetchServerResponseResult {
flightData: urlToUrlWithoutFlightMarker(url).toString(),
canonicalUrl: undefined,
couldBeIntercepted: false,
isPrerender: false,
prerendered: false,
postponed: false,
}
}
Expand Down Expand Up @@ -176,7 +175,6 @@ export async function fetchServerResponse(

const contentType = res.headers.get('content-type') || ''
const interception = !!res.headers.get('vary')?.includes(NEXT_URL)
const isPrerender = !!res.headers.get(NEXT_IS_PRERENDER_HEADER)
const postponed = !!res.headers.get(NEXT_DID_POSTPONE_HEADER)
let isFlightResponse = contentType.startsWith(RSC_CONTENT_TYPE_HEADER)

Expand Down Expand Up @@ -223,7 +221,7 @@ export async function fetchServerResponse(
flightData: normalizeFlightData(response.f),
canonicalUrl: canonicalUrl,
couldBeIntercepted: interception,
isPrerender: isPrerender,
prerendered: response.S,
postponed,
}
} catch (err) {
Expand All @@ -238,7 +236,7 @@ export async function fetchServerResponse(
flightData: url.toString(),
canonicalUrl: undefined,
couldBeIntercepted: false,
isPrerender: false,
prerendered: false,
postponed: false,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ function createLazyPrefetchEntry({
// If the prefetch was a cache hit, we want to update the existing cache entry to reflect that it was a full prefetch.
// This is because we know that a static response will contain the full RSC payload, and can be updated to respect the `static`
// staleTime.
if (prefetchResponse.isPrerender) {
if (prefetchResponse.prerendered) {
const existingCacheEntry = prefetchCache.get(
// if we prefixed the cache key due to route interception, we want to use the new key. Otherwise we use the original key
newCacheKey ?? prefetchCacheKey
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ export function serverActionReducer(
flightData,
canonicalUrl: undefined,
couldBeIntercepted: false,
isPrerender: false,
prerendered: false,
postponed: false,
},
tree: state.tree,
Expand Down
5 changes: 5 additions & 0 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ async function generateDynamicRSCPayload(
return {
b: ctx.renderOpts.buildId,
f: flightData,
S: staticGenerationStore.isStaticGeneration,
}
}

Expand Down Expand Up @@ -642,6 +643,7 @@ async function getRSCPayload(
m: missingSlots,
G: GlobalError,
s: typeof ctx.renderOpts.postponed === 'string',
S: staticGenerationStore.isStaticGeneration,
}
}

Expand Down Expand Up @@ -731,6 +733,7 @@ async function getErrorRSCPayload(
f: [[initialTree, initialSeedData, initialHead]],
G: GlobalError,
s: typeof ctx.renderOpts.postponed === 'string',
S: staticGenerationStore.isStaticGeneration,
} satisfies InitialRSCPayload
}

Expand Down Expand Up @@ -767,6 +770,7 @@ function App<T>({
location: null,
couldBeIntercepted: response.i,
postponed: response.s,
prerendered: response.S,
})

const actionQueue = createMutableActionQueue(initialState)
Expand Down Expand Up @@ -825,6 +829,7 @@ function AppWithoutContext<T>({
location: null,
couldBeIntercepted: response.i,
postponed: response.s,
prerendered: response.S,
})

const actionQueue = createMutableActionQueue(initialState)
Expand Down
4 changes: 4 additions & 0 deletions packages/next/src/server/app-render/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ export type InitialRSCPayload = {
G: React.ComponentType<any>
/** postponed */
s: boolean
/** prerendered */
S: boolean
}

// Response from `createFromFetch` for normal rendering
Expand All @@ -231,6 +233,8 @@ export type NavigationFlightResponse = {
b: string
/** flightData */
f: FlightData
/** prerendered */
S: boolean
}

// Response from `createFromFetch` for server actions. Action's flight data can be null
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ export default function HomePage() {
<Link href="/static-page" id="to-static-page">
To Static Page
</Link>
<Link href="/dynamic-page" id="to-dynamic-page-no-params">
To Dynamic Page
</Link>
<Link href="/prefetch-auto/foobar" id="to-dynamic-page">
To Dynamic Slug Page
</Link>
Expand Down
58 changes: 52 additions & 6 deletions test/e2e/app-dir/app-prefetch/prefetching.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,11 @@ describe('app dir - prefetching', () => {
beforePageLoad(page: Page) {
page.on('request', async (req: Request) => {
const url = new URL(req.url())
const headers = await req.allHeaders()
if (headers['rsc']) {
rscRequests.push(url.pathname)
if (url.pathname === '/static-page' || url.pathname === '/') {
const headers = await req.allHeaders()
if (headers['rsc']) {
rscRequests.push(url.pathname)
}
}
})
},
Expand All @@ -331,6 +333,27 @@ describe('app dir - prefetching', () => {
0
)
})

// navigate to index
await browser.elementByCss('[href="/"]').click()

// we should be on the index page
await browser.waitForElementByCss('#to-dashboard')

// navigate to the static page
await browser.elementByCss('[href="/static-page"]').click()

// we should be on the static page
await browser.waitForElementByCss('#static-page')

await browser.waitForIdleNetwork()

// We still shouldn't see any requests since it respects the static staletime (default 5m)
await retry(async () => {
expect(rscRequests.filter((req) => req === '/static-page').length).toBe(
0
)
})
})

it('should not re-fetch the initial dynamic page if the same page is prefetched with prefetch={true}', async () => {
Expand All @@ -339,9 +362,11 @@ describe('app dir - prefetching', () => {
beforePageLoad(page: Page) {
page.on('request', async (req: Request) => {
const url = new URL(req.url())
const headers = await req.allHeaders()
if (headers['rsc']) {
rscRequests.push(url.pathname)
if (url.pathname === '/dynamic-page' || url.pathname === '/') {
const headers = await req.allHeaders()
if (headers['rsc']) {
rscRequests.push(url.pathname)
}
}
})
},
Expand All @@ -362,6 +387,27 @@ describe('app dir - prefetching', () => {
rscRequests.filter((req) => req === '/dynamic-page').length
).toBe(0)
})

// navigate to index
await browser.elementByCss('[href="/"]').click()

// we should be on the index page
await browser.waitForElementByCss('#to-dashboard')

// navigate to the dynamic page
await browser.elementByCss('[href="/dynamic-page"]').click()

// we should be on the dynamic page
await browser.waitForElementByCss('#dynamic-page')

await browser.waitForIdleNetwork()

// We should see a request for the dynamic page since it respects the dynamic staletime (default 0)
await retry(async () => {
expect(
rscRequests.filter((req) => req === '/dynamic-page').length
).toBe(1)
})
})
})

Expand Down
10 changes: 7 additions & 3 deletions test/e2e/app-dir/revalidatetag-rsc/revalidatetag-rsc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,16 @@ describe('revalidateTag-rsc', () => {
await browser.refresh()
const randomNumber2 = await browser.elementById('data').text()
expect(randomNumber).toEqual(randomNumber2)

await browser.elementByCss('#revalidate-via-page').click()
await browser.waitForElementByCss('#home')
await browser.elementByCss('#home').click()
await browser.waitForElementByCss('#data')
const randomNumber3 = await browser.elementById('data').text()
expect(randomNumber3).not.toEqual(randomNumber)
await retry(async () => {
// need to refresh to evict client router cache
await browser.refresh()
await browser.waitForElementByCss('#data')
const randomNumber3 = await browser.elementById('data').text()
expect(randomNumber3).not.toEqual(randomNumber)
})
})
})

0 comments on commit 98ffc0e

Please sign in to comment.