From fa5dcbd7fa15ca5852ab0e93c035b84aa454c469 Mon Sep 17 00:00:00 2001
From: Tim Neutkens <tim@timneutkens.nl>
Date: Tue, 22 Nov 2022 19:49:51 +0100
Subject: [PATCH] Fix React.cache() in layout/page file (#43187)

Co-authored-by: JJ Kasper <jj@jjsweb.site>
---
 packages/next/server/app-render.tsx           | 49 +++++-----
 .../app/react-cache/client-component/page.js  | 16 ++++
 test/e2e/app-dir/app/app/react-cache/page.js  | 17 ++++
 .../app/react-cache/server-component/page.js  | 15 +++
 test/e2e/app-dir/app/app/react-fetch/page.js  | 17 ++++
 .../app/react-fetch/server-component/page.js  | 18 ++++
 test/e2e/app-dir/index.test.ts                | 93 +++++++++++++++++++
 test/e2e/app-dir/rsc-errors.test.ts           |  2 +-
 8 files changed, 201 insertions(+), 26 deletions(-)
 create mode 100644 test/e2e/app-dir/app/app/react-cache/client-component/page.js
 create mode 100644 test/e2e/app-dir/app/app/react-cache/page.js
 create mode 100644 test/e2e/app-dir/app/app/react-cache/server-component/page.js
 create mode 100644 test/e2e/app-dir/app/app/react-fetch/page.js
 create mode 100644 test/e2e/app-dir/app/app/react-fetch/server-component/page.js

diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx
index 293360ad2146..9c2723e8ec50 100644
--- a/packages/next/server/app-render.tsx
+++ b/packages/next/server/app-render.tsx
@@ -375,7 +375,7 @@ function useFlightResponse(
  * This is only used for renderToHTML, the Flight response does not need additional wrappers.
  */
 function createServerComponentRenderer(
-  ComponentToRender: React.ComponentType,
+  ComponentToRender: any,
   ComponentMod: {
     renderToReadableStream: any
     __next_app_webpack_require__?: any
@@ -1442,21 +1442,21 @@ export async function renderToHTMLOrFlight(
             isPrefetch && !Boolean(loaderTreeToFilter[2].loading)
               ? null
               : // Create component tree using the slice of the loaderTree
-                React.createElement(
-                  (
-                    await createComponentTree(
-                      // This ensures flightRouterPath is valid and filters down the tree
-                      {
-                        createSegmentPath: (child) => {
-                          return createSegmentPath(child)
-                        },
-                        loaderTree: loaderTreeToFilter,
-                        parentParams: currentParams,
-                        firstItem: isFirst,
-                      }
-                    )
-                  ).Component
-                ),
+                // @ts-expect-error TODO-APP: fix async component type
+                React.createElement(async () => {
+                  const { Component } = await createComponentTree(
+                    // This ensures flightRouterPath is valid and filters down the tree
+                    {
+                      createSegmentPath: (child) => {
+                        return createSegmentPath(child)
+                      },
+                      loaderTree: loaderTreeToFilter,
+                      parentParams: currentParams,
+                      firstItem: isFirst,
+                    }
+                  )
+                  return <Component />
+                }),
             isPrefetch && !Boolean(loaderTreeToFilter[2].loading) ? null : (
               <>{rscPayloadHead}</>
             ),
@@ -1529,14 +1529,6 @@ export async function renderToHTMLOrFlight(
 
     // Below this line is handling for rendering to HTML.
 
-    // Create full component tree from root to leaf.
-    const { Component: ComponentTree } = await createComponentTree({
-      createSegmentPath: (child) => child,
-      loaderTree: loaderTree,
-      parentParams: {},
-      firstItem: true,
-    })
-
     // AppRouter is provided by next-app-loader
     const AppRouter =
       ComponentMod.AppRouter as typeof import('../client/components/app-router').default
@@ -1579,7 +1571,14 @@ export async function renderToHTMLOrFlight(
      * using Flight which can then be rendered to HTML.
      */
     const ServerComponentsRenderer = createServerComponentRenderer(
-      () => {
+      async () => {
+        // Create full component tree from root to leaf.
+        const { Component: ComponentTree } = await createComponentTree({
+          createSegmentPath: (child) => child,
+          loaderTree: loaderTree,
+          parentParams: {},
+          firstItem: true,
+        })
         const initialTree = createFlightRouterStateFromLoaderTree(loaderTree)
 
         return (
diff --git a/test/e2e/app-dir/app/app/react-cache/client-component/page.js b/test/e2e/app-dir/app/app/react-cache/client-component/page.js
new file mode 100644
index 000000000000..5cca1ce3ad5f
--- /dev/null
+++ b/test/e2e/app-dir/app/app/react-cache/client-component/page.js
@@ -0,0 +1,16 @@
+'use client'
+import { cache } from 'react'
+
+const getRandomMemoized = cache(() => Math.random())
+
+export default function Page() {
+  const val1 = getRandomMemoized()
+  const val2 = getRandomMemoized()
+  return (
+    <>
+      <h1>React Cache Client Component</h1>
+      <p id="value-1">{val1}</p>
+      <p id="value-2">{val2}</p>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/app/app/react-cache/page.js b/test/e2e/app-dir/app/app/react-cache/page.js
new file mode 100644
index 000000000000..14049016a7ff
--- /dev/null
+++ b/test/e2e/app-dir/app/app/react-cache/page.js
@@ -0,0 +1,17 @@
+import Link from 'next/link'
+export default function Page() {
+  return (
+    <>
+      <p>
+        <Link id="to-server-component" href="/react-cache/server-component">
+          To Server Component
+        </Link>
+      </p>
+      <p>
+        <Link id="to-client-component" href="/react-cache/client-component">
+          To Client Component
+        </Link>
+      </p>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/app/app/react-cache/server-component/page.js b/test/e2e/app-dir/app/app/react-cache/server-component/page.js
new file mode 100644
index 000000000000..94ecf7429b0b
--- /dev/null
+++ b/test/e2e/app-dir/app/app/react-cache/server-component/page.js
@@ -0,0 +1,15 @@
+import { cache } from 'react'
+
+const getRandomMemoized = cache(() => Math.random())
+
+export default function Page() {
+  const val1 = getRandomMemoized()
+  const val2 = getRandomMemoized()
+  return (
+    <>
+      <h1>React Cache Server Component</h1>
+      <p id="value-1">{val1}</p>
+      <p id="value-2">{val2}</p>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/app/app/react-fetch/page.js b/test/e2e/app-dir/app/app/react-fetch/page.js
new file mode 100644
index 000000000000..59e6d2a8bd6f
--- /dev/null
+++ b/test/e2e/app-dir/app/app/react-fetch/page.js
@@ -0,0 +1,17 @@
+import Link from 'next/link'
+export default function Page() {
+  return (
+    <>
+      <p>
+        <Link id="to-server-component" href="/react-fetch/server-component">
+          To Server Component
+        </Link>
+      </p>
+      <p>
+        <Link id="to-client-component" href="/react-fetch/client-component">
+          To Client Component
+        </Link>
+      </p>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/app/app/react-fetch/server-component/page.js b/test/e2e/app-dir/app/app/react-fetch/server-component/page.js
new file mode 100644
index 000000000000..a1ae02f41e37
--- /dev/null
+++ b/test/e2e/app-dir/app/app/react-fetch/server-component/page.js
@@ -0,0 +1,18 @@
+async function getRandomMemoizedByFetch() {
+  const res = await fetch(
+    'https://next-data-api-endpoint.vercel.app/api/random'
+  )
+  return res.text()
+}
+
+export default async function Page() {
+  const val1 = await getRandomMemoizedByFetch()
+  const val2 = await getRandomMemoizedByFetch()
+  return (
+    <>
+      <h1>React Fetch Server Component</h1>
+      <p id="value-1">{val1}</p>
+      <p id="value-2">{val2}</p>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts
index af5fa3044f75..b6ee34e452ee 100644
--- a/test/e2e/app-dir/index.test.ts
+++ b/test/e2e/app-dir/index.test.ts
@@ -2165,6 +2165,99 @@ describe('app dir', () => {
     })
 
     describe('known bugs', () => {
+      describe('should support React cache', () => {
+        it('server component', async () => {
+          const browser = await webdriver(
+            next.url,
+            '/react-cache/server-component'
+          )
+          const val1 = await browser.elementByCss('#value-1').text()
+          const val2 = await browser.elementByCss('#value-2').text()
+          expect(val1).toBe(val2)
+        })
+
+        it('server component client-navigation', async () => {
+          const browser = await webdriver(next.url, '/react-cache')
+
+          await browser
+            .elementByCss('#to-server-component')
+            .click()
+            .waitForElementByCss('#value-1', 10000)
+          const val1 = await browser.elementByCss('#value-1').text()
+          const val2 = await browser.elementByCss('#value-2').text()
+          expect(val1).toBe(val2)
+        })
+
+        it('client component', async () => {
+          const browser = await webdriver(
+            next.url,
+            '/react-cache/client-component'
+          )
+          const val1 = await browser.elementByCss('#value-1').text()
+          const val2 = await browser.elementByCss('#value-2').text()
+          expect(val1).toBe(val2)
+        })
+
+        it('client component client-navigation', async () => {
+          const browser = await webdriver(next.url, '/react-cache')
+
+          await browser
+            .elementByCss('#to-client-component')
+            .click()
+            .waitForElementByCss('#value-1', 10000)
+          const val1 = await browser.elementByCss('#value-1').text()
+          const val2 = await browser.elementByCss('#value-2').text()
+          expect(val1).toBe(val2)
+        })
+      })
+
+      describe('should support React fetch instrumentation', () => {
+        it('server component', async () => {
+          const browser = await webdriver(
+            next.url,
+            '/react-fetch/server-component'
+          )
+          const val1 = await browser.elementByCss('#value-1').text()
+          const val2 = await browser.elementByCss('#value-2').text()
+          expect(val1).toBe(val2)
+        })
+
+        it('server component client-navigation', async () => {
+          const browser = await webdriver(next.url, '/react-fetch')
+
+          await browser
+            .elementByCss('#to-server-component')
+            .click()
+            .waitForElementByCss('#value-1', 10000)
+          const val1 = await browser.elementByCss('#value-1').text()
+          const val2 = await browser.elementByCss('#value-2').text()
+          expect(val1).toBe(val2)
+        })
+
+        // TODO-APP: React doesn't have fetch deduping for client components yet.
+        it.skip('client component', async () => {
+          const browser = await webdriver(
+            next.url,
+            '/react-fetch/client-component'
+          )
+          const val1 = await browser.elementByCss('#value-1').text()
+          const val2 = await browser.elementByCss('#value-2').text()
+          expect(val1).toBe(val2)
+        })
+
+        // TODO-APP: React doesn't have fetch deduping for client components yet.
+        it.skip('client component client-navigation', async () => {
+          const browser = await webdriver(next.url, '/react-fetch')
+
+          await browser
+            .elementByCss('#to-client-component')
+            .click()
+            .waitForElementByCss('#value-1', 10000)
+          const val1 = await browser.elementByCss('#value-1').text()
+          const val2 = await browser.elementByCss('#value-2').text()
+          expect(val1).toBe(val2)
+        })
+      })
       it('should not share flight data between requests', async () => {
         const fetches = await Promise.all(
           [...new Array(5)].map(() =>
diff --git a/test/e2e/app-dir/rsc-errors.test.ts b/test/e2e/app-dir/rsc-errors.test.ts
index e007b8b44798..5c8decd205b3 100644
--- a/test/e2e/app-dir/rsc-errors.test.ts
+++ b/test/e2e/app-dir/rsc-errors.test.ts
@@ -104,7 +104,7 @@ describe('app dir - rsc errors', () => {
       '/server-with-errors/page-export'
     )
     expect(html).toContain(
-      'The default export is not a React Component in page: \\"/server-with-errors/page-export\\"'
+      'The default export is not a React Component in page:'
     )
   })
 })