From 41515cb932f2381a4d0ae35fd2d3ca1af4763155 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 21 Nov 2022 14:01:56 +0100 Subject: [PATCH 1/4] Fix React.cache() in layout/page file Since the component preloading happened outside of React rendering it caused cache() to not have an available dispatcher, moving creating the component tree into a wrapper component solves this. Handled for both HTML rendering and client-side navigation. Added tests for both cases as well as server/client component. --- 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/index.test.ts | 45 +++++++++++++++++ 5 files changed, 117 insertions(+), 25 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 diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 4a6d4b55b3d0..a3ca3cda425b 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -374,7 +374,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 + }), 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..735bea606a07 --- /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(1) + const val2 = getRandomMemoized(1) + return ( + <> +

React Cache Client Component

+

{val1}

+

{val2}

+ + ) +} 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 ( + <> +

+ + To Server Component + +

+

+ + To Client Component + +

+ + ) +} 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..4a173c2a910d --- /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(1) + const val2 = getRandomMemoized(1) + return ( + <> +

React Cache Server Component

+

{val1}

+

{val2}

+ + ) +} diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 6af1d4603ae2..f2675ea7a260 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -2096,6 +2096,51 @@ 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', 2000) + 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', 2000) + 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(() => From c08df6e998dfe85d0694701240547e4919685564 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 21 Nov 2022 14:47:04 +0100 Subject: [PATCH 2/4] Add test for fetch deduping --- .../app/react-cache/client-component/page.js | 4 +- .../app/react-cache/server-component/page.js | 4 +- 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 | 48 +++++++++++++++++++ 5 files changed, 87 insertions(+), 4 deletions(-) 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/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 index 735bea606a07..5cca1ce3ad5f 100644 --- 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 @@ -4,8 +4,8 @@ import { cache } from 'react' const getRandomMemoized = cache(() => Math.random()) export default function Page() { - const val1 = getRandomMemoized(1) - const val2 = getRandomMemoized(1) + const val1 = getRandomMemoized() + const val2 = getRandomMemoized() return ( <>

React Cache Client Component

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 index 4a173c2a910d..94ecf7429b0b 100644 --- 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 @@ -3,8 +3,8 @@ import { cache } from 'react' const getRandomMemoized = cache(() => Math.random()) export default function Page() { - const val1 = getRandomMemoized(1) - const val2 = getRandomMemoized(1) + const val1 = getRandomMemoized() + const val2 = getRandomMemoized() return ( <>

React Cache Server Component

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 ( + <> +

+ + To Server Component + +

+

+ + To Client Component + +

+ + ) +} 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 ( + <> +

React Fetch Server Component

+

{val1}

+

{val2}

+ + ) +} diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index f2675ea7a260..8f04fdfb6269 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -2141,6 +2141,54 @@ describe('app dir', () => { 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', 2000) + 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', 2000) + 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(() => From 7d3779247b61d42b9768f363a35b39c6d9f658f0 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Tue, 22 Nov 2022 15:47:32 +0100 Subject: [PATCH 3/4] Fix test --- test/e2e/app-dir/rsc-errors.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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:' ) }) }) From 555e3ed6b3ff13befd24d3563b9f49e52df31261 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Tue, 22 Nov 2022 18:02:58 +0100 Subject: [PATCH 4/4] Increase timeout --- test/e2e/app-dir/index.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 215d1af06884..b6ee34e452ee 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -2182,7 +2182,7 @@ describe('app dir', () => { await browser .elementByCss('#to-server-component') .click() - .waitForElementByCss('#value-1', 2000) + .waitForElementByCss('#value-1', 10000) const val1 = await browser.elementByCss('#value-1').text() const val2 = await browser.elementByCss('#value-2').text() expect(val1).toBe(val2) @@ -2204,7 +2204,7 @@ describe('app dir', () => { await browser .elementByCss('#to-client-component') .click() - .waitForElementByCss('#value-1', 2000) + .waitForElementByCss('#value-1', 10000) const val1 = await browser.elementByCss('#value-1').text() const val2 = await browser.elementByCss('#value-2').text() expect(val1).toBe(val2) @@ -2228,7 +2228,7 @@ describe('app dir', () => { await browser .elementByCss('#to-server-component') .click() - .waitForElementByCss('#value-1', 2000) + .waitForElementByCss('#value-1', 10000) const val1 = await browser.elementByCss('#value-1').text() const val2 = await browser.elementByCss('#value-2').text() expect(val1).toBe(val2) @@ -2252,7 +2252,7 @@ describe('app dir', () => { await browser .elementByCss('#to-client-component') .click() - .waitForElementByCss('#value-1', 2000) + .waitForElementByCss('#value-1', 10000) const val1 = await browser.elementByCss('#value-1').text() const val2 = await browser.elementByCss('#value-2').text() expect(val1).toBe(val2)