fix: prevent revalidateTag/Path during render

also revert changes from #70446
vercel · Oct 10, 2024 · 13c3633 · 13c3633
1 parent f10a6a8
commit 13c3633
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 35 deletions.
diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx
@@ -518,22 +518,6 @@ async function generateDynamicFlightRenderResult(
       onError,
     }
   )
-  await waitAtLeastOneReactRenderTask()
-
-  if (
-    ctx.workStore.pendingRevalidates ||
-    ctx.workStore.revalidatedTags ||
-    ctx.workStore.pendingRevalidateWrites
-  ) {
-    const promises = Promise.all([
-      ctx.workStore.incrementalCache?.revalidateTag(
-        ctx.workStore.revalidatedTags || []
-      ),
-      ...Object.values(ctx.workStore.pendingRevalidates || {}),
-      ...(ctx.workStore.pendingRevalidateWrites || []),
-    ])
-    ctx.renderOpts.waitUntil = (p) => promises.then(() => p)
-  }
 
   return new FlightRenderResult(flightReadableStream, {
     fetchMetrics: ctx.workStore.fetchMetrics,

diff --git a/packages/next/src/server/web/spec-extension/revalidate.ts b/packages/next/src/server/web/spec-extension/revalidate.ts
@@ -60,6 +60,11 @@ function revalidate(tag: string, expression: string) {
         `Route ${store.route} used "${expression}" inside a function cached with "unstable_cache(...)" which is unsupported. To ensure revalidation is performed consistently it must always happen outside of renders and cached functions. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering`
       )
     }
+    if (workUnitStore.phase === 'render') {
+      throw new Error(
+        `Route ${store.route} used "${expression}" during render which is unsupported. To ensure revalidation is performed consistently it must always happen outside of renders and cached functions. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering`
+      )
+    }
   }
 
   // a route that makes use of revalidation APIs should be considered dynamic

diff --git a/test/e2e/app-dir/revalidatetag-rsc/revalidatetag-rsc.test.ts b/test/e2e/app-dir/revalidatetag-rsc/revalidatetag-rsc.test.ts
@@ -1,8 +1,8 @@
 import { nextTestSetup } from 'e2e-utils'
-import { retry } from 'next-test-utils'
+import { getRedboxHeader, retry } from 'next-test-utils'
 
 describe('revalidateTag-rsc', () => {
-  const { next } = nextTestSetup({
+  const { next, isNextDev, isNextDeploy } = nextTestSetup({
     files: __dirname,
   })
 
@@ -21,22 +21,29 @@ describe('revalidateTag-rsc', () => {
     })
   })
 
-  it('should revalidate fetch cache if revalidateTag invoked via server component', async () => {
-    const browser = await next.browser('/')
-    const randomNumber = await browser.elementById('data').text()
-    await browser.refresh()
-    const randomNumber2 = await browser.elementById('data').text()
-    expect(randomNumber).toEqual(randomNumber2)
-    await browser.elementByCss('#revalidate-via-page').click()
-    await browser.waitForElementByCss('#home')
-    await browser.elementByCss('#home').click()
-    await browser.waitForElementByCss('#data')
-    await retry(async () => {
-      // need to refresh to evict client router cache
-      await browser.refresh()
-      await browser.waitForElementByCss('#data')
-      const randomNumber3 = await browser.elementById('data').text()
-      expect(randomNumber3).not.toEqual(randomNumber)
+  if (!isNextDeploy) {
+    // skipped in deploy because it uses `next.cliOutput`
+    it('should error if revalidateTag is called during render', async () => {
+      const browser = await next.browser('/')
+      await browser.elementByCss('#revalidate-via-page').click()
+
+      if (isNextDev) {
+        await retry(async () => {
+          expect(await getRedboxHeader(browser)).toContain(
+            'Route /revalidate_via_page used "revalidateTag data"'
+          )
+        })
+      } else {
+        await retry(async () => {
+          expect(
+            await browser.eval('document.documentElement.innerHTML')
+          ).toContain('Application error: a server-side exception has occurred')
+        })
+      }
+
+      expect(next.cliOutput).toContain(
+        'Route /revalidate_via_page used "revalidateTag data"'
+      )
     })
-  })
+  }
 })