Repository: vercel/next.js
Pull request: Fix fallback: true cache-control

This ensures we don't treat cache hits for lazily generated `fallback: true` routes as the fallback itself which has a different cache-control header than cache hits should.  

Fixes: https://github.com/vercel/next.js/issues/80838

Changed files:
filename: packages/next/src/build/templates/pages.ts
patch: @@ -162,7 +162,7 @@ export async function handler(
   )
   const isAmp = query.amp && config.amp
   let cacheKey: null | string = null
-  let isIsrFallback = Boolean(getRequestMeta(req, 'isIsrFallback'))
+  let isIsrFallback = false
   let isNextDataRequest =
     prepareResult.isNextDataRequest && (hasStaticProps || hasServerProps)
 
@@ -530,6 +530,7 @@ export async function handler(
             // Remove the cache control from the response to prevent it from being
             // used in the surrounding cache.
             delete fallbackResponse.cacheControl
+            fallbackResponse.isMiss = true
             return fallbackResponse
           }
         }
@@ -586,6 +587,13 @@ export async function handler(
         prerenderManifest,
       })
 
+      // if we got a cache hit this wasn't an ISR fallback
+      // but it wasn't generated during build so isn't in the
+      // prerender-manifest
+      if (isIsrFallback && !result?.isMiss) {
+        isIsrFallback = false
+      }
+
       // response is finished is no cache entry
       if (!result) {
         return
@@ -629,7 +637,6 @@ export async function handler(
               `Invalid revalidate configuration provided: ${result.cacheControl.revalidate} < 1`
             )
           }
-
           cacheControl = {
             revalidate: result.cacheControl.revalidate,
             expire: result.cacheControl?.expire ?? nextConfig.expireTime,

filename: test/e2e/prerender.test.ts
patch: @@ -302,6 +302,13 @@ describe('Prerender', () => {
     //   initialRevalidateSeconds: false,
     //   srcRoute: null,
     // },
+    '/fallback-true/first': {
+      allowHeader,
+      dataRoute: `/_next/data/${next.buildId}/fallback-true/first.json`,
+      initialExpireSeconds: 31536000,
+      initialRevalidateSeconds: 2,
+      srcRoute: '/fallback-true/[slug]',
+    },
     '/lang/de/about': {
       allowHeader,
       dataRoute: `/_next/data/${next.buildId}/lang/de/about.json`,
@@ -676,6 +683,72 @@ describe('Prerender', () => {
             : 's-maxage=2, stale-while-revalidate=31535998'
         )
       })
+
+      it('should use correct caching headers for a fallback-true page (prerendered)', async () => {
+        const initialRes = await fetchViaHTTP(next.url, '/fallback-true/first')
+        expect(initialRes.status).toBe(200)
+        expect(initialRes.headers.get('cache-control')).toBe(
+          isDeploy
+            ? 'public, max-age=0, must-revalidate'
+            : 's-maxage=2, stale-while-revalidate=31535998'
+        )
+        expect(await initialRes.text()).not.toContain('hi fallback')
+
+        const dataRes = await fetchViaHTTP(
+          next.url,
+          `/_next/data/${next.buildId}/fallback-true/first.json`
+        )
+        expect(dataRes.status).toBe(200)
+        expect(dataRes.headers.get('cache-control')).toBe(
+          isDeploy
+            ? 'public, max-age=0, must-revalidate'
+            : 's-maxage=2, stale-while-revalidate=31535998'
+        )
+
+        await retry(async () => {
+          const finalRes = await fetchViaHTTP(next.url, `/fallback-true/first`)
+          expect(finalRes.status).toBe(200)
+          expect(finalRes.headers.get('cache-control')).toBe(
+            isDeploy
+              ? 'public, max-age=0, must-revalidate'
+              : 's-maxage=2, stale-while-revalidate=31535998'
+          )
+          expect(await finalRes.text()).not.toContain('hi fallback')
+        })
+      })
+
+      it('should use correct caching headers for a fallback-true page (lazy)', async () => {
+        const initialRes = await fetchViaHTTP(next.url, '/fallback-true/second')
+        expect(initialRes.status).toBe(200)
+        expect(initialRes.headers.get('cache-control')).toBe(
+          isDeploy
+            ? 'public, max-age=0, must-revalidate'
+            : 'private, no-cache, no-store, max-age=0, must-revalidate'
+        )
+        expect(await initialRes.text()).toContain('hi fallback')
+
+        const dataRes = await fetchViaHTTP(
+          next.url,
+          `/_next/data/${next.buildId}/fallback-true/second.json`
+        )
+        expect(dataRes.status).toBe(200)
+        expect(dataRes.headers.get('cache-control')).toBe(
+          isDeploy
+            ? 'public, max-age=0, must-revalidate'
+            : 's-maxage=2, stale-while-revalidate=31535998'
+        )
+
+        await retry(async () => {
+          const finalRes = await fetchViaHTTP(next.url, `/fallback-true/second`)
+          expect(finalRes.status).toBe(200)
+          expect(finalRes.headers.get('cache-control')).toBe(
+            isDeploy
+              ? 'public, max-age=0, must-revalidate'
+              : 's-maxage=2, stale-while-revalidate=31535998'
+          )
+          expect(await finalRes.text()).not.toContain('hi fallback')
+        })
+      })
     }
 
     it('should navigate to a normal page and back', async () => {
@@ -1545,6 +1618,16 @@ describe('Prerender', () => {
             //   ),
             //   page: '/index',
             // },
+            {
+              dataRouteRegex: normalizeRegEx(
+                `^\\/_next\\/data\\/${escapeRegex(next.buildId)}\\/fallback\\-true\\/([^\\/]+?)\\.json$`
+              ),
+              namedDataRouteRegex: `^/_next/data/${escapeRegex(next.buildId)}/fallback\\-true/(?<nxtPslug>[^/]+?)\\.json$`,
+              page: '/fallback-true/[slug]',
+              routeKeys: {
+                nxtPslug: 'nxtPslug',
+              },
+            },
             {
               namedDataRouteRegex: `^/_next/data/${escapeRegex(
                 next.buildId
@@ -1740,6 +1823,17 @@ describe('Prerender', () => {
               ),
               allowHeader,
             },
+            '/fallback-true/[slug]': {
+              allowHeader,
+              dataRoute: `/_next/data/${next.buildId}/fallback-true/[slug].json`,
+              dataRouteRegex: normalizeRegEx(
+                `^\\/_next\\/data\\/${escapedBuildId}\\/fallback\\-true\\/([^\\/]+?)\\.json$`
+              ),
+              fallback: '/fallback-true/[slug].html',
+              routeRegex: normalizeRegEx(
+                '^\\/fallback\\-true\\/([^\\/]+?)(?:\\/)?$'
+              ),
+            },
             '/lang/[lang]/about': {
               dataRoute: `/_next/data/${next.buildId}/lang/[lang]/about.json`,
               dataRouteRegex: normalizeRegEx(

filename: test/e2e/prerender/pages/fallback-true/[slug].js
patch: @@ -0,0 +1,40 @@
+import Link from 'next/link'
+import { useRouter } from 'next/router'
+
+export async function getStaticPaths() {
+  return {
+    paths: ['/fallback-true/first'],
+    fallback: true,
+  }
+}
+
+export async function getStaticProps({ params }) {
+  return {
+    props: {
+      params,
+      hello: 'world',
+      post: params.slug,
+      random: Math.random(),
+      time: (await import('perf_hooks')).performance.now(),
+    },
+    revalidate: 2,
+  }
+}
+
+export default ({ post, time, params }) => {
+  if (useRouter().isFallback) {
+    return <p>hi fallback</p>
+  }
+
+  return (
+    <>
+      <p>Post: {post}</p>
+      <span>time: {time}</span>
+      <div id="params">{JSON.stringify(params)}</div>
+      <div id="query">{JSON.stringify(useRouter().query)}</div>
+      <Link href="/" id="home">
+        to home
+      </Link>
+    </>
+  )
+}