Skip to content

Commit

Permalink
[metadata] only blocking render for html bots in ppr pages (vercel#76379
Browse files Browse the repository at this point in the history 
)

### What

In non-PRR mode, we keep the original behavior that metadata is blocking but stream the rest. Previously we also do the full dynamic rendering for both non-PPR and PPR pages, which is required for PPR pages to discard the postpone cache but not necessary for non-PPR pages.

This PR align the behavior as before.

Closes NDX-881
  • Loading branch information
@huozhi
huozhi authored Feb 23, 2025
1 parent 71e139d commit d5f596e
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 88 deletions.
22 changes: 11 additions & 11 deletions packages/next/src/server/base-server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2067,11 +2067,6 @@ export default abstract class Server<
}
}

const isHtmlBot = isHtmlBotRequest(req)
if (isHtmlBot) {
this.renderOpts.serveStreamingMetadata = false
}

if (
hasFallback ||
staticPaths?.includes(resolvedUrlPathname) ||
Expand All @@ -2082,11 +2077,6 @@ export default abstract class Server<
isSSG = true
} else if (!this.renderOpts.dev) {
isSSG ||= !!prerenderManifest.routes[toRoute(pathname)]
if (isHtmlBot) {
// When it's html limited bots request, disable SSG
// and perform the full blocking & dynamic rendering.
isSSG = false
}
}

// Toggle whether or not this is a Data request
Expand Down Expand Up @@ -2227,6 +2217,12 @@ export default abstract class Server<
'segmentPrefetchRSCRequest'
)

const isHtmlBot = isHtmlBotRequest(req)
if (isHtmlBot && isRoutePPREnabled) {
isSSG = false
this.renderOpts.serveStreamingMetadata = false
}

// we need to ensure the status code if /404 is visited directly
if (is404Page && !isNextDataRequest && !isRSCRequest) {
res.statusCode = 404
Expand Down Expand Up @@ -2483,7 +2479,11 @@ export default abstract class Server<
query: origQuery,
})

const shouldWaitOnAllReady = !supportsDynamicResponse || isHtmlBot
const shouldWaitOnAllReady =
!supportsDynamicResponse ||
// When html bots request PPR page, perform the full dynamic rendering.
(isHtmlBot && isRoutePPREnabled)

const renderOpts: LoadedRenderOpts = {
...components,
...opts,
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/app-dir/metadata-streaming-static-generation/app/suspenseful/dynamic/page.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ import { connection } from 'next/server'

export default async function Page() {
await connection()
await new Promise((resolve) => setTimeout(resolve, 500))
return <p>suspenseful - dynamic</p>
}

export async function generateMetadata() {
await connection()
await new Promise((resolve) => setTimeout(resolve, 1 * 1000))
await new Promise((resolve) => setTimeout(resolve, 200))
return {
title: 'suspenseful page - dynamic',
}
Expand Down
158 changes: 85 additions & 73 deletions ...app-dir/metadata-streaming-static-generation/metadata-streaming-static-generation.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,94 +2,106 @@ import { nextTestSetup } from 'e2e-utils'

const isPPREnabled = process.env.__NEXT_EXPERIMENTAL_PPR === 'true'

describe('app-dir - metadata-streaming-static-generation', () => {
const { next, isNextDev, isNextStart } = nextTestSetup({
files: __dirname,
})

if (isNextStart && !isPPREnabled) {
// Precondition for the following tests in build mode.
// This test is only useful for non-PPR mode as in PPR mode those routes
// are all listed in the prerender manifest.
it('should generate all pages static', async () => {
const prerenderManifest = JSON.parse(
await next.readFile('.next/prerender-manifest.json')
)
const staticRoutes = prerenderManifest.routes
expect(Object.keys(staticRoutes).sort()).toEqual([
'/',
'/slow/static',
'/suspenseful/static',
])
// Skip PPR test as it's covered in test/e2e/app-dir/ppr-metadata-streaming/ppr-metadata-streaming.test.ts
;(isPPREnabled ? describe.skip : describe)(
'app-dir - metadata-streaming-static-generation',
() => {
const { next, isNextDev, isNextStart } = nextTestSetup({
files: __dirname,
})
}

if (isNextDev) {
// In development it's still dynamic rendering that metadata will be inserted into body
describe('static pages (development)', () => {
it('should contain async generated metadata in body for simple static page', async () => {
const $ = await next.render$('/')
expect($('body title').text()).toBe('index page')
if (isNextStart) {
// Precondition for the following tests in build mode.
// This test is only useful for non-PPR mode as in PPR mode those routes
// are all listed in the prerender manifest.
it('should generate all pages static', async () => {
const prerenderManifest = JSON.parse(
await next.readFile('.next/prerender-manifest.json')
)
const staticRoutes = prerenderManifest.routes
expect(Object.keys(staticRoutes).sort()).toEqual([
'/',
'/slow/static',
'/suspenseful/static',
])
})
}

it('should contain async generated metadata in body for slow static page', async () => {
const $ = await next.render$('/slow/static')
expect($('body title').text()).toBe('slow page - static')
})
if (isNextDev) {
// In development it's still dynamic rendering that metadata will be inserted into body
describe('static pages (development)', () => {
it('should contain async generated metadata in body for simple static page', async () => {
const $ = await next.render$('/')
expect($('body title').text()).toBe('index page')
})

it('should contain async generated metadata in body static page with suspenseful content', async () => {
const $ = await next.render$('/suspenseful/static')
expect($('body title').text()).toBe('suspenseful page - static')
})
})
} else {
describe('static pages (production)', () => {
it('should contain async generated metadata in head for simple static page', async () => {
const $ = await next.render$('/')
expect($('head title').text()).toBe('index page')
it('should contain async generated metadata in body for slow static page', async () => {
const $ = await next.render$('/slow/static')
expect($('body title').text()).toBe('slow page - static')
})

it('should contain async generated metadata in body static page with suspenseful content', async () => {
const $ = await next.render$('/suspenseful/static')
expect($('body title').text()).toBe('suspenseful page - static')
})
})
} else {
describe('static pages (production)', () => {
it('should contain async generated metadata in head for simple static page', async () => {
const $ = await next.render$('/')
expect($('head title').text()).toBe('index page')
})

it('should contain async generated metadata in head for slow static page', async () => {
const $ = await next.render$('/slow/static')
expect($('head title').text()).toBe('slow page - static')
})

it('should contain async generated metadata in head for slow static page', async () => {
const $ = await next.render$('/slow/static')
expect($('head title').text()).toBe('slow page - static')
it('should contain async generated metadata in head static page with suspenseful content', async () => {
const $ = await next.render$('/suspenseful/static')
expect($('head title').text()).toBe('suspenseful page - static')
})
})
}

it('should contain async generated metadata in head static page with suspenseful content', async () => {
const $ = await next.render$('/suspenseful/static')
expect($('head title').text()).toBe('suspenseful page - static')
describe('dynamic pages', () => {
it('should contain async generated metadata in body for simple dynamics page', async () => {
const $ = await next.render$('/suspenseful/dynamic')
expect($('body title').text()).toBe('suspenseful page - dynamic')
})
})
}

describe('dynamic pages', () => {
it('should contain async generated metadata in body for simple dynamics page', async () => {
const $ = await next.render$('/suspenseful/dynamic')
expect($('body title').text()).toBe('suspenseful page - dynamic')
it('should contain async generated metadata in body for suspenseful dynamic page', async () => {
const $ = await next.render$('/slow/dynamic')
expect($('body title').text()).toBe('slow page - dynamic')
})
})

it('should contain async generated metadata in body for suspenseful dynamic page', async () => {
const $ = await next.render$('/slow/dynamic')
expect($('body title').text()).toBe('slow page - dynamic')
})
})
describe('dynamic pages with html limited bots', () => {
it('should contain stream metadata in head for suspenseful dynamic page', async () => {
const $ = await next.render$('/suspenseful/dynamic', undefined, {
headers: {
'User-Agent': 'Discordbot/2.0;',
},
})
expect($('head title').text()).toBe('suspenseful page - dynamic')
// Ensure it's suspenseful content
expect($('.suspenseful-layout').text()).toBe('')

describe('dynamic pages with html limited bots', () => {
it('should contain async generated metadata in head for simple dynamics page', async () => {
const $ = await next.render$('/suspenseful/dynamic', undefined, {
headers: {
'User-Agent': 'Discordbot/2.0;',
},
// Can still render the suspenseful content with browser
const browser = await next.browser('/suspenseful/dynamic')
expect(await browser.elementByCss('.suspenseful-layout').text()).toBe(
'suspenseful - dynamic'
)
})
expect($('head title').text()).toBe('suspenseful page - dynamic')
})

it('should contain async generated metadata in head for suspenseful dynamic page', async () => {
const $ = await next.render$('/slow/dynamic', undefined, {
headers: {
'User-Agent': 'Discordbot/2.0;',
},
it('should contain async generated metadata in head for simple dynamic page', async () => {
const $ = await next.render$('/slow/dynamic', undefined, {
headers: {
'User-Agent': 'Discordbot/2.0;',
},
})
expect($('head title').text()).toBe('slow page - dynamic')
})
expect($('head title').text()).toBe('slow page - dynamic')
})
})
})
}
)
2 changes: 1 addition & 1 deletion test/e2e/app-dir/ppr-metadata-streaming/app/dynamic-page/partial/page.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ async function SuspendedComponent() {
await new Promise((resolve) => setTimeout(resolve, 500))
return (
<div>
<div>suspended component</div>
<div>outer suspended component</div>
<NestedSuspendedComponent />
</div>
)
Expand Down
9 changes: 7 additions & 2 deletions test/e2e/app-dir/ppr-metadata-streaming/ppr-metadata-streaming.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe('ppr-metadata-streaming', () => {
if (!isNextDev && !isNextDeploy) {
// This test is only relevant in production mode, as it's testing PPR results
describe('html limited bots', () => {
it('should serve partial static shell when normal UA requests the page', async () => {
it('should serve partial static shell when normal UA requests the PPR page', async () => {
const res1 = await next.fetch('/dynamic-page/partial')
const res2 = await next.fetch('/dynamic-page/partial')

Expand All @@ -123,7 +123,7 @@ describe('ppr-metadata-streaming', () => {
expect(headers.get('x-nextjs-postponed')).toBe('1')
})

it('should not serve partial static shell when html limited bots requests the page', async () => {
it('should perform blocking and dynamic rendering when html limited bots requests the PPR page', async () => {
const htmlLimitedBotUA = 'Discordbot'
const res1 = await next.fetch('/dynamic-page/partial', {
headers: {
Expand All @@ -150,6 +150,11 @@ describe('ppr-metadata-streaming', () => {
// Two requests are dynamic and should not have the same data-date attribute
expect(attribute2).toBeGreaterThan(attribute1)
expect(attribute1).toBeTruthy()

// Should contain resolved suspense content
const bodyHtml = $1('body').html()
expect(bodyHtml).toContain('outer suspended component')
expect(bodyHtml).toContain('nested suspended component')
})
})
}
Expand Down

0 comments on commit d5f596e

Please sign in to comment.