Skip to content

Commit

Permalink
Use provided waitUntil for pending revalidates (#74164)
Browse files Browse the repository at this point in the history
Currently we are using `pendingWaitUntil` on `renderOpts` to handle
pending revalidations (fetch updates and revalidate tag calls). This
uses our old strategy of `waitUntil` inside of `sendResponse` which just
keeps the stream open until the promise resolves. This isn't ideal if we
have a proper waitUntil strategy provided so this updates to ensure we
use that instead if available.

Also adds some debug logs so we can track when this pending revalidates
promise is resolved.
  • Loading branch information
ijjk authored Dec 20, 2024
1 parent bc69d97 commit 4e80990
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 11 deletions.
28 changes: 24 additions & 4 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1298,13 +1298,23 @@ async function renderToHTMLOrFlightImpl(
workStore.pendingRevalidateWrites ||
workStore.revalidatedTags
) {
options.waitUntil = Promise.all([
const pendingPromise = Promise.all([
workStore.incrementalCache?.revalidateTag(
workStore.revalidatedTags || []
),
...Object.values(workStore.pendingRevalidates || {}),
...(workStore.pendingRevalidateWrites || []),
])
]).finally(() => {
if (process.env.NEXT_PRIVATE_DEBUG_CACHE) {
console.log('pending revalidates promise finished for:', url)
}
})

if (renderOpts.waitUntil) {
renderOpts.waitUntil(pendingPromise)
} else {
options.waitUntil = pendingPromise
}
}

if (response.collectedTags) {
Expand Down Expand Up @@ -1455,13 +1465,23 @@ async function renderToHTMLOrFlightImpl(
workStore.pendingRevalidateWrites ||
workStore.revalidatedTags
) {
options.waitUntil = Promise.all([
const pendingPromise = Promise.all([
workStore.incrementalCache?.revalidateTag(
workStore.revalidatedTags || []
),
...Object.values(workStore.pendingRevalidates || {}),
...(workStore.pendingRevalidateWrites || []),
])
]).finally(() => {
if (process.env.NEXT_PRIVATE_DEBUG_CACHE) {
console.log('pending revalidates promise finished for:', url)
}
})

if (renderOpts.waitUntil) {
renderOpts.waitUntil(pendingPromise)
} else {
options.waitUntil = pendingPromise
}
}

// Create the new render result for the response.
Expand Down
10 changes: 10 additions & 0 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2541,6 +2541,16 @@ export default abstract class Server<

return cacheEntry
}
let pendingWaitUntil = context.renderOpts.pendingWaitUntil

// Attempt using provided waitUntil if available
// if it's not we fallback to sendResponse's handling
if (pendingWaitUntil) {
if (context.renderOpts.waitUntil) {
context.renderOpts.waitUntil(pendingWaitUntil)
pendingWaitUntil = undefined
}
}

// Send the response now that we have copied it into the cache.
await sendResponse(
Expand Down
9 changes: 8 additions & 1 deletion packages/next/src/server/route-modules/app-route/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,14 @@ export class AppRouteRouteModule extends RouteModule<
workStore.revalidatedTags || []
),
...Object.values(workStore.pendingRevalidates || {}),
])
]).finally(() => {
if (process.env.NEXT_PRIVATE_DEBUG_CACHE) {
console.log(
'pending revalidates promise finished for:',
requestStore.url
)
}
})

if (prerenderStore) {
context.renderOpts.collectedTags = prerenderStore.tags?.join(',')
Expand Down
11 changes: 5 additions & 6 deletions test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { findPort, waitFor } from 'next-test-utils'
import { findPort, retry } from 'next-test-utils'
import http from 'http'
import url from 'url'
import { outdent } from 'outdent'
Expand Down Expand Up @@ -112,11 +112,10 @@ describe('app-fetch-deduping', () => {
expect(invocation(next.cliOutput)).toBe(1)

// wait for the revalidation to finish
await waitFor(revalidate * 1000 + 1000)

await next.render('/test')

expect(invocation(next.cliOutput)).toBe(2)
await retry(async () => {
await next.render('/test')
expect(invocation(next.cliOutput)).toBe(2)
}, 10_000)
})
})
} else {
Expand Down

0 comments on commit 4e80990

Please sign in to comment.