Skip to content

Commit

Permalink
disable static generation on interception routes (vercel#61004)
Browse files Browse the repository at this point in the history
### What & Why?
Interception routes depend on contextual information that are provided
via request headers. Specifically it needs to know about the
`Next-Router-State-Tree` when generating the interception route RSC
data, which isn't available at build time. This doesn't currently cause
any usage issues, but it erroneously emits static files & RSC payloads
that the client router won't be able to use and will instead fallback to
a dynamic request.

I removed some special case in an existing test since this fix also
resolves a discrepancy in behavior when PPR is turned on

### How?
This excludes interception routes from `appStaticPaths` at builds which
currently determines which pages should be statically generated.

Closes NEXT-2190
  • Loading branch information
ztanner authored Jan 23, 2024
1 parent 2e977f1 commit cf0f090
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 46 deletions.
51 changes: 29 additions & 22 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ import { hasCustomExportOutput } from '../export/utils'
import { interopDefault } from '../lib/interop-default'
import { formatDynamicImportPath } from '../lib/format-dynamic-import-path'
import { isDefaultRoute } from '../lib/is-default-route'
import { isInterceptionRouteAppPath } from '../server/future/helpers/interception-routes'

interface ExperimentalBypassForInfo {
experimentalBypassFor?: RouteHas[]
Expand Down Expand Up @@ -1857,7 +1858,7 @@ export default async function build(
)
} else {
// If this route can be partially pre-rendered, then
// mark it as such and mark it that it can be
// mark it as such and mark that it can be
// generated server-side.
if (workerResult.isPPR) {
isPPR = workerResult.isPPR
Expand Down Expand Up @@ -1885,6 +1886,8 @@ export default async function build(
}

const appConfig = workerResult.appConfig || {}
const isInterceptionRoute =
isInterceptionRouteAppPath(page)
if (appConfig.revalidate !== 0) {
const isDynamic = isDynamicRoute(page)
const hasGenerateStaticParams =
Expand All @@ -1900,26 +1903,29 @@ export default async function build(
)
}

if (
// Mark the app as static if:
// - It has no dynamic param
// - It doesn't have generateStaticParams but `dynamic` is set to
// `error` or `force-static`
!isDynamic
) {
appStaticPaths.set(originalAppPath, [page])
appStaticPathsEncoded.set(originalAppPath, [page])
isStatic = true
} else if (
isDynamic &&
!hasGenerateStaticParams &&
(appConfig.dynamic === 'error' ||
appConfig.dynamic === 'force-static')
) {
appStaticPaths.set(originalAppPath, [])
appStaticPathsEncoded.set(originalAppPath, [])
isStatic = true
isPPR = false
// Mark the app as static if:
// - It's not an interception route (these currently depend on request headers and cannot be computed at build)
// - It has no dynamic param
// - It doesn't have generateStaticParams but `dynamic` is set to
// `error` or `force-static`
if (!isInterceptionRoute) {
if (!isDynamic) {
appStaticPaths.set(originalAppPath, [page])
appStaticPathsEncoded.set(originalAppPath, [
page,
])
isStatic = true
} else if (
isDynamic &&
!hasGenerateStaticParams &&
(appConfig.dynamic === 'error' ||
appConfig.dynamic === 'force-static')
) {
appStaticPaths.set(originalAppPath, [])
appStaticPathsEncoded.set(originalAppPath, [])
isStatic = true
isPPR = false
}
}
}

Expand All @@ -1936,7 +1942,8 @@ export default async function build(
!isStatic &&
!isAppRouteRoute(originalAppPath) &&
!isDynamicRoute(originalAppPath) &&
!isPPR
!isPPR &&
!isInterceptionRoute
) {
appPrefetchPaths.set(originalAppPath, page)
}
Expand Down
9 changes: 8 additions & 1 deletion packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ import { isAppRouteRouteModule } from '../server/future/route-modules/checks'
import { interopDefault } from '../lib/interop-default'
import type { PageExtensions } from './page-extensions-type'
import { formatDynamicImportPath } from '../lib/format-dynamic-import-path'
import { isInterceptionRouteAppPath } from '../server/future/helpers/interception-routes'

export type ROUTER_TYPE = 'pages' | 'app'

Expand Down Expand Up @@ -1744,14 +1745,20 @@ export async function isPageStatic({
isStatic = true
}

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

// interception routes depend on `Next-URL` and `Next-Router-State-Tree` request headers and thus cannot be prerendered
if (isInterceptionRouteAppPath(page)) {
isStatic = false
isPPR = false
}

return {
isStatic,
isPPR,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,27 +127,12 @@ createNextDescribe(
app: new FileRef(path.join(__dirname, 'app')),
},
},
({ next, isNextStart }) => {
if (process.env.__NEXT_EXPERIMENTAL_PPR === 'true' && isNextStart) {
// The PPR prefetch will 404 since it'll request the full page (which won't exist, since the intercepted route
// has no default). The default router behavior if a prefetch fails is to trigger an MPA navigation
it('should render the non-intercepted page on navigation', async () => {
const browser = await next.browser('/')

await browser.elementByCss('[href="/photos/1"]').click()
await check(() => browser.elementById('slot').text(), /@slot default/)
await check(
() => browser.elementById('children').text(),
/Photo Page \(non-intercepted\) 1/
)
})
} else {
it('should use the default fallback (a 404) if there is no custom default page', async () => {
const browser = await next.browser('/')

await browser.elementByCss('[href="/photos/1"]').click()
await check(() => browser.elementByCss('body').text(), /404/)
})
}
({ next }) => {
it('should use the default fallback (a 404) if there is no custom default page', async () => {
const browser = await next.browser('/')

await browser.elementByCss('[href="/photos/1"]').click()
await check(() => browser.elementByCss('body').text(), /404/)
})
}
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>Interception Modal</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default function Layout(props) {
return (
<>
<div>{props.children}</div>
<div>{props.modal}</div>
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>Modal Page</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Link from 'next/link'

export default function Page() {
return (
<div>
<Link href="/baz/modal">Open Interception Modal</Link>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'
import { Response } from 'playwright-chromium'

createNextDescribe(
'interception-route-prefetch-cache',
{
files: __dirname,
},
({ next }) => {
({ next, isNextStart }) => {
it('should render the correct interception when two distinct layouts share the same path structure', async () => {
const browser = await next.browser('/')

Expand Down Expand Up @@ -41,5 +42,41 @@ createNextDescribe(
/Intercepted on Bar Page/
)
})

if (isNextStart) {
it('should not be a cache HIT when prefetching an interception route', async () => {
const responses: { cacheStatus: string; pathname: string }[] = []
const browser = await next.browser('/baz', {
beforePageLoad(page) {
page.on('response', (response: Response) => {
const url = new URL(response.url())
const request = response.request()
const responseHeaders = response.headers()
const requestHeaders = request.headers()
if (requestHeaders['next-router-prefetch']) {
responses.push({
cacheStatus: responseHeaders['x-nextjs-cache'],
pathname: url.pathname,
})
}
})
},
})

expect(await browser.elementByCss('body').text()).toContain(
'Open Interception Modal'
)

const interceptionPrefetchResponse = responses.find(
(response) => response.pathname === '/baz/modal'
)
const homePrefetchResponse = responses.find(
(response) => response.pathname === '/'
)

expect(homePrefetchResponse.cacheStatus).toBe('HIT') // sanity check to ensure we're seeing cache statuses
expect(interceptionPrefetchResponse.cacheStatus).toBeUndefined()
})
}
}
)

0 comments on commit cf0f090

Please sign in to comment.