Fix revalidate: false detection in app (#49473)

When revalidate isn't defined in the tree at all and a fetch without
cache/revalidate fields is done we are incorrectly marking the initial
revalidate period with a time based value when it should be `false`.
This causes pages that should be fully static to revalidate
unexpectedly.

x-ref: [twitter
thread](https://twitter.com/diegohaz/status/1655638433795014657)
x-ref: [slack
thread](https://vercel.slack.com/archives/C03S8ED1DKM/p1683566860136879)

packages/next/src/build/index.ts

@@ -289,7 +289,11 @@ export default async function build(
       hasAppDir = Boolean(appDir)
 
       if (isAppDirEnabled && hasAppDir) {
-        if (!process.env.__NEXT_TEST_MODE && ciEnvironment.hasNextSupport) {
+        if (
+          (!process.env.__NEXT_TEST_MODE ||
+            process.env.__NEXT_TEST_MODE === 'e2e') &&
+          ciEnvironment.hasNextSupport
+        ) {
           const requireHook = require.resolve('../server/require-hook')
           const contents = await promises.readFile(requireHook, 'utf8')
           await promises.writeFile(

packages/next/src/server/lib/patch-fetch.ts

@@ -173,14 +173,10 @@ export function patchFetch({
         if (['no-cache', 'no-store'].includes(_cache || '')) {
           curRevalidate = 0
         }
-        if (typeof curRevalidate === 'number') {
+        if (typeof curRevalidate === 'number' || curRevalidate === false) {
           revalidate = curRevalidate
         }
 
-        if (curRevalidate === false) {
-          revalidate = CACHE_ONE_YEAR
-        }
-
         const _headers = getRequestMeta('headers')
         const initHeaders: Headers =
           typeof _headers?.get === 'function'
@@ -229,7 +225,7 @@ export function patchFetch({
             revalidate =
               typeof staticGenerationStore.revalidate === 'boolean' ||
               typeof staticGenerationStore.revalidate === 'undefined'
-                ? CACHE_ONE_YEAR
+                ? false
                 : staticGenerationStore.revalidate
           }
         }
@@ -240,18 +236,19 @@ export function patchFetch({
           !autoNoCache &&
           (typeof staticGenerationStore.revalidate === 'undefined' ||
             (typeof revalidate === 'number' &&
-              typeof staticGenerationStore.revalidate === 'number' &&
-              revalidate < staticGenerationStore.revalidate))
+              (staticGenerationStore.revalidate === false ||
+                (typeof staticGenerationStore.revalidate === 'number' &&
+                  revalidate < staticGenerationStore.revalidate))))
         ) {
           staticGenerationStore.revalidate = revalidate
         }
 
+        const isCacheableRevalidate =
+          (typeof revalidate === 'number' && revalidate > 0) ||
+          revalidate === false
+
         let cacheKey: string | undefined
-        if (
-          staticGenerationStore.incrementalCache &&
-          typeof revalidate === 'number' &&
-          revalidate > 0
-        ) {
+        if (staticGenerationStore.incrementalCache && isCacheableRevalidate) {
           try {
             cacheKey =
               await staticGenerationStore.incrementalCache.fetchCacheKey(
@@ -304,6 +301,8 @@ export function patchFetch({
         const fetchIdx = staticGenerationStore.nextFetchId ?? 1
         staticGenerationStore.nextFetchId = fetchIdx + 1
 
+        const normalizedRevalidate = !revalidate ? CACHE_ONE_YEAR : revalidate
+
         const doOriginalFetch = async (isStale?: boolean) => {
           // add metadata to init without editing the original
           const clonedInit = {
@@ -325,8 +324,7 @@ export function patchFetch({
               res.status === 200 &&
               staticGenerationStore.incrementalCache &&
               cacheKey &&
-              typeof revalidate === 'number' &&
-              revalidate > 0
+              isCacheableRevalidate
             ) {
               const bodyBuffer = Buffer.from(await res.arrayBuffer())
 
@@ -341,9 +339,9 @@ export function patchFetch({
                       status: res.status,
                       tags,
                     },
-                    revalidate,
+                    revalidate: normalizedRevalidate,
                   },
-                  revalidate,
+                  normalizedRevalidate,
                   true,
                   fetchUrl,
                   fetchIdx
@@ -367,7 +365,7 @@ export function patchFetch({
             : await staticGenerationStore.incrementalCache.get(
                 cacheKey,
                 true,
-                revalidate,
+                normalizedRevalidate,
                 fetchUrl,
                 fetchIdx
               )

test/e2e/app-dir/app-static/app-static.test.ts

@@ -23,12 +23,14 @@ createNextDescribe(
   },
   ({ next, isNextDev: isDev, isNextStart, isNextDeploy }) => {
     let prerenderManifest
+    let buildCliOutputIndex = 0
 
     beforeAll(async () => {
       if (isNextStart) {
         prerenderManifest = JSON.parse(
           await next.readFile('.next/prerender-manifest.json')
         )
+        buildCliOutputIndex = next.cliOutput.length
       }
     })
 
@@ -171,6 +173,31 @@ createNextDescribe(
     // On-Demand Revalidate has not effect in dev since app routes
     // aren't considered static until prerendering
     if (!(global as any).isNextDev && !process.env.CUSTOM_CACHE_HANDLER) {
+      it('should not revalidate / when revalidate is not used', async () => {
+        let prevData
+
+        for (let i = 0; i < 5; i++) {
+          const res = await next.fetch('/')
+          const html = await res.text()
+          const $ = cheerio.load(html)
+          const data = $('#page-data').text()
+
+          expect(res.status).toBe(200)
+
+          if (prevData) {
+            expect(prevData).toBe(data)
+            prevData = data
+          }
+          await waitFor(500)
+        }
+
+        if (isNextStart) {
+          expect(next.cliOutput.substring(buildCliOutputIndex)).not.toContain(
+            'rendering index'
+          )
+        }
+      })
+
       it.each([
         {
           type: 'edge route handler',
@@ -399,6 +426,9 @@ createNextDescribe(
           'hooks/use-search-params/with-suspense.html',
           'hooks/use-search-params/with-suspense.rsc',
           'hooks/use-search-params/with-suspense/page.js',
+          'index.html',
+          'index.rsc',
+          'page.js',
           'partial-gen-params-no-additional-lang/[lang]/[slug]/page.js',
           'partial-gen-params-no-additional-lang/en/RAND.html',
           'partial-gen-params-no-additional-lang/en/RAND.rsc',
@@ -507,6 +537,11 @@ createNextDescribe(
 
         expect(curManifest.version).toBe(4)
         expect(curManifest.routes).toEqual({
+          '/': {
+            initialRevalidateSeconds: false,
+            srcRoute: '/',
+            dataRoute: '/index.rsc',
+          },
           '/blog/tim': {
             initialRevalidateSeconds: 10,
             srcRoute: '/blog/[author]',
@@ -659,7 +694,7 @@ createNextDescribe(
               'x-next-cache-tags':
                 'thankyounext,/route-handler/revalidate-360-isr/route',
             },
-            initialRevalidateSeconds: false,
+            initialRevalidateSeconds: 10,
             srcRoute: '/route-handler/revalidate-360-isr',
           },
           '/variable-config-revalidate/revalidate-3': {

test/e2e/app-dir/app-static/app/page.js

@@ -0,0 +1,15 @@
+export default async function Page() {
+  console.log('rendering index')
+
+  const data = await fetch(
+    'https://next-data-api-endpoint.vercel.app/api/random?page'
+  ).then((res) => res.text())
+
+  return (
+    <>
+      <p id="page">/variable-revalidate/revalidate-360</p>
+      <p id="page-data">{data}</p>
+      <p id="now">{Date.now()}</p>
+    </>
+  )
+}

test/lib/next-modes/next-deploy.ts

@@ -85,7 +85,7 @@ export class NextDeployInstance extends NextInstance {
       [
         'deploy',
         '--build-env',
-        'NEXT_PRIVATE_TEST_MODE=1',
+        'NEXT_PRIVATE_TEST_MODE=e2e',
         '--build-env',
         'NEXT_TELEMETRY_DISABLED=1',
         ...additionalEnv,

test/lib/next-modes/next-dev.ts

@@ -47,7 +47,7 @@ export class NextDevInstance extends NextInstance {
             ...this.env,
             NODE_ENV: '' as any,
             PORT: this.forcedPort || '0',
-            __NEXT_TEST_MODE: '1',
+            __NEXT_TEST_MODE: 'e2e',
             __NEXT_TEST_WITH_DEVTOOL: '1',
           },
         })

test/lib/next-modes/next-start.ts

@@ -50,7 +50,7 @@ export class NextStartInstance extends NextInstance {
         ...this.env,
         NODE_ENV: '' as any,
         PORT: this.forcedPort || '0',
-        __NEXT_TEST_MODE: '1',
+        __NEXT_TEST_MODE: 'e2e',
       },
     }
     let buildArgs = ['yarn', 'next', 'build']