Skip to content

Commit

Permalink
fix: fixed headers returned with PPR and fixed force-dynamic static d…
Browse files Browse the repository at this point in the history
…etection
  • Loading branch information
wyattjoh committed Dec 11, 2023
1 parent a79e9a4 commit 97d6669
Show file tree
Hide file tree
Showing 13 changed files with 318 additions and 191 deletions.
35 changes: 25 additions & 10 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ import { formatManifest } from './manifests/formatter/format-manifest'
import { getStartServerInfo, logStartInfo } from '../server/lib/app-info-log'
import type { NextEnabledDirectories } from '../server/base-server'
import { RouteKind } from '../server/future/route-kind'
import createDebug from 'next/dist/compiled/debug'

const debug = createDebug('next:build')

interface ExperimentalBypassForInfo {
experimentalBypassFor?: RouteHas[]
Expand Down Expand Up @@ -1627,6 +1630,8 @@ export default async function build(
}
)

debug(`page ${page} isPageStatic:`, workerResult)

if (pageType === 'app' && originalAppPath) {
appNormalizedPaths.set(originalAppPath, page)
// TODO-APP: handle prerendering with edge
Expand Down Expand Up @@ -1667,7 +1672,10 @@ export default async function build(
}

const appConfig = workerResult.appConfig || {}
if (appConfig.revalidate !== 0) {

// Configure the page to be static if the revalidation
// time isn't zero or the page has PPR enabled.
if (appConfig.revalidate !== 0 || isPPR) {
const isDynamic = isDynamicRoute(page)
const hasGenerateStaticParams =
!!workerResult.prerenderRoutes?.length
Expand Down Expand Up @@ -2267,10 +2275,11 @@ export default async function build(
)
}

if (hasDynamicData && pageInfo.isStatic) {
// If the page was marked as being static, but it contains dynamic
// data (ie, in the case of a static generation bailout), then it
// should be marked dynamic.
// If the page was marked as being static, but it contains dynamic
// data (ie, in the case of a static generation bailout), then it
// should be marked dynamic. If PPR is enabled for this route
// though, it should continue to be marked as static.
if (hasDynamicData && pageInfo.isStatic && !pageInfo.isPPR) {
pageInfo.isStatic = false
pageInfo.isSSG = false
}
Expand Down Expand Up @@ -2374,14 +2383,20 @@ export default async function build(
}
} else {
hasDynamicData = true
// we might have determined during prerendering that this page
// used dynamic data
pageInfo.isStatic = false
pageInfo.isSSG = false

// We might have determined during prerendering that this page
// used dynamic data. If PPR is enabled, it should be marked
// as static.
if (!pageInfo.isPPR) {
pageInfo.isStatic = false
pageInfo.isSSG = false
}
}
})

if (!hasDynamicData && isDynamic) {
// Add the route as a supported dynamic route if it has no data
// requirements or if PPR is enabled.
if ((!hasDynamicData || pageInfo.isPPR) && isDynamic) {
let dataRoute: string | null = null
let prefetchDataRoute: string | undefined

Expand Down
28 changes: 12 additions & 16 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,7 @@ export async function isPageStatic({
let encodedPrerenderRoutes: Array<string> | undefined
let prerenderFallback: boolean | 'blocking' | undefined
let appConfig: AppConfig = {}
let isPPR = false
let isClientComponent: boolean = false
const pathIsEdgeRuntime = isEdgeRuntime(pageRuntime)

Expand Down Expand Up @@ -1594,16 +1595,19 @@ export async function isPageStatic({
{}
)

if (ppr && routeModule.definition.kind === RouteKind.APP_PAGE) {
isPPR = true
}

if (appConfig.dynamic === 'force-static' && pathIsEdgeRuntime) {
Log.warn(
`Page "${page}" is using runtime = 'edge' which is currently incompatible with dynamic = 'force-static'. Please remove either "runtime" or "force-static" for correct behavior`
)
}

// If force dynamic was set and we don't have PPR enabled, then set the
// revalidate to 0.
// TODO: (PPR) remove this once PPR is enabled by default
if (appConfig.dynamic === 'force-dynamic' && !ppr) {
// When `dynamic = "force-dynamic"`, we should not retain the value in
// the cache.
if (appConfig.dynamic === 'force-dynamic') {
appConfig.revalidate = 0
}

Expand Down Expand Up @@ -1693,18 +1697,10 @@ export async function isPageStatic({
)
}

let isStatic = false
if (!hasStaticProps && !hasGetInitialProps && !hasServerProps) {
isStatic = true
}
const hasDynamicData =
hasStaticProps || hasGetInitialProps || hasServerProps

// When PPR is enabled, any route may contain or be completely static, so
// mark this route as static.
let isPPR = false
if (ppr && routeModule.definition.kind === RouteKind.APP_PAGE) {
isPPR = true
isStatic = true
}
const isStatic = isPPR || !hasDynamicData

return {
isStatic,
Expand Down Expand Up @@ -2098,7 +2094,7 @@ export function getPossibleInstrumentationHookFilenames(
folder: string,
extensions: string[]
) {
const files = []
const files: string[] = []
for (const extension of extensions) {
files.push(
path.join(folder, `${INSTRUMENTATION_HOOK_FILENAME}.${extension}`),
Expand Down
27 changes: 23 additions & 4 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1294,7 +1294,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}

res.statusCode = Number(req.headers['x-invoke-status'])
let err = null
let err: Error | null = null

if (typeof req.headers['x-invoke-error'] === 'string') {
const invokeError = JSON.parse(
Expand Down Expand Up @@ -2601,7 +2601,18 @@ export default abstract class Server<ServerOptions extends Options = Options> {
return null
}

if (isSSG && !this.minimalMode) {
const didPostpone =
cacheEntry.value?.kind === 'PAGE' && !!cacheEntry.value.postponed

if (
isSSG &&
!this.minimalMode &&
// We don't want to send a cache header for requests that contain dynamic
// data. If this is a Dynamic RSC request or wasn't a Prefetch RSC
// request, then we should set the cache header.
!isDynamicRSCRequest &&
(!didPostpone || isPrefetchRSCRequest)
) {
// set x-nextjs-cache header to match the header
// we set for the image-optimizer
res.setHeader(
Expand Down Expand Up @@ -2813,7 +2824,12 @@ export default abstract class Server<ServerOptions extends Options = Options> {
return {
type: 'rsc',
body: cachedData.html,
revalidate: cacheEntry.revalidate,
// Dynamic RSC responses cannot be cached, even if they're
// configured with `force-static` because we have no way of
// distinguishing between `force-static` and pages that have no
// postponed state.
// TODO: distinguish `force-static` from pages with no postponed state (static)
revalidate: 0,
}
}

Expand Down Expand Up @@ -2881,7 +2897,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
return {
type: 'html',
body,
revalidate: cacheEntry.revalidate,
// We don't want to cache the response if it has postponed data because
// the response being sent to the client it's dynamic parts are streamed
// to the client on the same request.
revalidate: 0,
}
} else if (isDataReq) {
return {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React, { Suspense } from 'react'
import { Dynamic } from '../../../../../components/dynamic'

export const dynamic = 'force-dynamic'

export function generateStaticParams() {
return []
}

export default ({ params: { slug } }) => {
return (
<Suspense
fallback={
<Dynamic pathname={`/dynamic/force-dynamic/nested/${slug}`} fallback />
}
>
<Dynamic pathname={`/dynamic/force-dynamic/nested/${slug}`} />
</Suspense>
)
}
10 changes: 3 additions & 7 deletions test/e2e/app-dir/ppr-full/app/dynamic/force-dynamic/page.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@ import { Dynamic } from '../../../components/dynamic'

export const dynamic = 'force-dynamic'

export default ({ params: { slug } }) => {
export default () => {
return (
<Suspense
fallback={
<Dynamic pathname={`/dynamic/force-dynamic/${slug}`} fallback />
}
>
<Dynamic pathname={`/dynamic/force-dynamic/${slug}`} />
<Suspense fallback={<Dynamic pathname="/dynamic/force-dynamic" fallback />}>
<Dynamic pathname="/dynamic/force-dynamic" />
</Suspense>
)
}
8 changes: 3 additions & 5 deletions test/e2e/app-dir/ppr-full/app/dynamic/force-static/page.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ import { Dynamic } from '../../../components/dynamic'
export const dynamic = 'force-static'
export const revalidate = 60

export default ({ params: { slug } }) => {
export default () => {
return (
<Suspense
fallback={<Dynamic pathname={`/dynamic/force-static/${slug}`} fallback />}
>
<Dynamic pathname={`/dynamic/force-static/${slug}`} />
<Suspense fallback={<Dynamic pathname="/dynamic/force-static" fallback />}>
<Dynamic pathname="/dynamic/force-static" />
</Suspense>
)
}
5 changes: 1 addition & 4 deletions test/e2e/app-dir/ppr-full/app/static/page.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
import React from 'react'
import { Dynamic } from '../../components/dynamic'

export default () => {
return <Dynamic pathname="/static" fallback />
return 'Static'
}
4 changes: 2 additions & 2 deletions test/e2e/app-dir/ppr-full/components/dynamic.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { headers } from 'next/headers'

export const Dynamic = ({ pathname, fallback }) => {
if (fallback) {
return <div>Loading...</div>
return <div>Dynamic Loading...</div>
}

const messages = []
Expand All @@ -28,7 +28,7 @@ export const Dynamic = ({ pathname, fallback }) => {
<dt>
Header: <code>{name}</code>
</dt>
<dd>{value ?? 'null'}</dd>
<dd>{value ?? `MISSING:${name.toUpperCase()}`}</dd>
</React.Fragment>
))}
</dl>
Expand Down

0 comments on commit 97d6669

Please sign in to comment.