From 3050b0696bc3642bd1ce8590f9b40e1a682e000b Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Tue, 28 May 2024 16:25:53 -0700 Subject: [PATCH] fix Head rendering when parallel routes are present --- .../next/src/client/components/app-router.tsx | 23 +++++++- .../src/client/components/layout-router.tsx | 2 +- .../next/src/server/app-render/app-render.tsx | 53 +++++++++---------- .../acceptance-app/hydration-error.test.ts | 46 ++++++++-------- .../app/@foo/[...catchAll]/page.tsx | 9 ++++ .../app/@foo/[...catchAll]/style.module.css | 3 ++ .../parallel-routes-css/app/@foo/default.tsx | 3 ++ .../parallel-routes-css/app/layout.tsx | 20 +++++++ .../parallel-routes-css/app/nested/page.tsx | 14 +++++ .../app-dir/parallel-routes-css/app/page.tsx | 15 ++++++ .../parallel-routes-css/app/style.module.css | 3 ++ .../parallel-routes-css.test.ts | 50 +++++++++++++++++ test/turbopack-build-tests-manifest.json | 9 ++++ 13 files changed, 195 insertions(+), 55 deletions(-) create mode 100644 test/e2e/app-dir/parallel-routes-css/app/@foo/[...catchAll]/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-css/app/@foo/[...catchAll]/style.module.css create mode 100644 test/e2e/app-dir/parallel-routes-css/app/@foo/default.tsx create mode 100644 test/e2e/app-dir/parallel-routes-css/app/layout.tsx create mode 100644 test/e2e/app-dir/parallel-routes-css/app/nested/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-css/app/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-css/app/style.module.css create mode 100644 test/e2e/app-dir/parallel-routes-css/parallel-routes-css.test.ts diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index e36addc60376b..38a15539ce813 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -293,6 +293,7 @@ function Head({ function Router({ buildId, initialHead, + initialLayerAssets, initialTree, initialCanonicalUrl, initialSeedData, @@ -310,7 +311,7 @@ function Router({ initialParallelRoutes, location: !isServer ? window.location : null, initialHead, - initialLayerAssets: null, + initialLayerAssets, couldBeIntercepted, }), [ @@ -319,6 +320,7 @@ function Router({ initialCanonicalUrl, initialTree, initialHead, + initialLayerAssets, couldBeIntercepted, ] ) @@ -657,10 +659,27 @@ function Router({ head = null } + // We use `useDeferredValue` to handle switching between the prefetched and + // final values. The second argument is returned on initial render, then it + // re-renders with the first argument. We only use the prefetched layer assets + // if they are available. Otherwise, we use the non-prefetched version. + const resolvedPrefetchLayerAssets = + cache.prefetchLayerAssets !== null + ? cache.prefetchLayerAssets + : cache.layerAssets + + const layerAssets = useDeferredValue( + cache.layerAssets, + // @ts-expect-error The second argument to `useDeferredValue` is only + // available in the experimental builds. When its disabled, it will always + // return `cache.layerAssets`. + resolvedPrefetchLayerAssets + ) + let content = ( {head} - {/* {cache.layerAssets} */} + {layerAssets} {cache.rsc} diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx index eb7b7824effea..2e7aa80adf74d 100644 --- a/packages/next/src/client/components/layout-router.tsx +++ b/packages/next/src/client/components/layout-router.tsx @@ -468,7 +468,7 @@ function InnerLayoutRouter({ loading: childNode.loading, }} > - {/* {childNode.layerAssets} */} + {childNode.layerAssets} {resolvedRsc} ) diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index dd929a651cfcb..1b7985b6dacf1 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -481,34 +481,31 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) { typeof varyHeader === 'string' && varyHeader.includes(NEXT_URL) return ( - <> - {styles} - - {typeof ctx.res.statusCode === 'number' && - ctx.res.statusCode > 400 && ( - - )} - {/* Adding requestId as react key to make metadata remount for each render */} - - - } - initialLayerAssets={null} - globalErrorComponent={GlobalError} - // This is used to provide debug information (when in development mode) - // about which slots were not filled by page components while creating the component tree. - missingSlots={missingSlots} - /> - + + {typeof ctx.res.statusCode === 'number' && + ctx.res.statusCode > 400 && ( + + )} + {/* Adding requestId as react key to make metadata remount for each render */} + + + } + initialLayerAssets={styles} + globalErrorComponent={GlobalError} + // This is used to provide debug information (when in development mode) + // about which slots were not filled by page components while creating the component tree. + missingSlots={missingSlots} + /> ) } diff --git a/test/development/acceptance-app/hydration-error.test.ts b/test/development/acceptance-app/hydration-error.test.ts index ee51218152876..1602329ad03e3 100644 --- a/test/development/acceptance-app/hydration-error.test.ts +++ b/test/development/acceptance-app/hydration-error.test.ts @@ -604,40 +604,38 @@ describe('Error overlay for hydration errors', () => { if (isTurbopack) { expect(fullPseudoHtml).toMatchInlineSnapshot(` "... - - - - - - + + + + + +
-
- -

- - + client - - server" + +

+ + + client + - server" `) } else { expect(fullPseudoHtml).toMatchInlineSnapshot(` "... - - - - - - + + + + + +

-
- -

- - + client - - server" + +

+ + + client + - server" `) } diff --git a/test/e2e/app-dir/parallel-routes-css/app/@foo/[...catchAll]/page.tsx b/test/e2e/app-dir/parallel-routes-css/app/@foo/[...catchAll]/page.tsx new file mode 100644 index 0000000000000..6c26d0f730cfb --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-css/app/@foo/[...catchAll]/page.tsx @@ -0,0 +1,9 @@ +import styles from './style.module.css' + +export default function Page() { + return ( +

+ Slot +
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-css/app/@foo/[...catchAll]/style.module.css b/test/e2e/app-dir/parallel-routes-css/app/@foo/[...catchAll]/style.module.css new file mode 100644 index 0000000000000..dc9f187170250 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-css/app/@foo/[...catchAll]/style.module.css @@ -0,0 +1,3 @@ +.bgRed { + background-color: red; +} diff --git a/test/e2e/app-dir/parallel-routes-css/app/@foo/default.tsx b/test/e2e/app-dir/parallel-routes-css/app/@foo/default.tsx new file mode 100644 index 0000000000000..c17431379f962 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-css/app/@foo/default.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return null +} diff --git a/test/e2e/app-dir/parallel-routes-css/app/layout.tsx b/test/e2e/app-dir/parallel-routes-css/app/layout.tsx new file mode 100644 index 0000000000000..e76395794b57e --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-css/app/layout.tsx @@ -0,0 +1,20 @@ +import { ReactNode } from 'react' + +const RootLayout = ({ + children, + foo, +}: { + children: ReactNode + foo: ReactNode +}) => { + return ( + + + {children} + {foo} + + + ) +} + +export default RootLayout diff --git a/test/e2e/app-dir/parallel-routes-css/app/nested/page.tsx b/test/e2e/app-dir/parallel-routes-css/app/nested/page.tsx new file mode 100644 index 0000000000000..05a571d2744d5 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-css/app/nested/page.tsx @@ -0,0 +1,14 @@ +import Link from 'next/link' + +export const metadata = { + title: 'Nested Page', +} + +export default function Page() { + return ( +
+

Sub

+ Home +
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-css/app/page.tsx b/test/e2e/app-dir/parallel-routes-css/app/page.tsx new file mode 100644 index 0000000000000..8b7a4d47f02b6 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-css/app/page.tsx @@ -0,0 +1,15 @@ +import Link from 'next/link' +import styles from './style.module.css' + +export const metadata = { + title: 'Home Page', +} + +export default function Home() { + return ( +
+

Main

+ Nested Page +
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-css/app/style.module.css b/test/e2e/app-dir/parallel-routes-css/app/style.module.css new file mode 100644 index 0000000000000..74c611f0ba380 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-css/app/style.module.css @@ -0,0 +1,3 @@ +.bgBlue { + background-color: blue; +} diff --git a/test/e2e/app-dir/parallel-routes-css/parallel-routes-css.test.ts b/test/e2e/app-dir/parallel-routes-css/parallel-routes-css.test.ts new file mode 100644 index 0000000000000..5b9945817c6f4 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-css/parallel-routes-css.test.ts @@ -0,0 +1,50 @@ +import { nextTestSetup } from 'e2e-utils' +import type { BrowserInterface } from 'next-webdriver' + +describe('parallel-routes-catchall-css', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + async function getChildrenBackgroundColor(browser: BrowserInterface) { + return browser.eval( + `window.getComputedStyle(document.getElementById('main')).backgroundColor` + ) + } + + async function getSlotBackgroundColor(browser: BrowserInterface) { + return browser.eval( + `window.getComputedStyle(document.getElementById('slot')).backgroundColor` + ) + } + + it('should properly load the Head content from multiple leaf segments', async () => { + const browser = await next.browser('/') + + // the page background should be blue + expect(await getChildrenBackgroundColor(browser)).toBe('rgb(0, 0, 255)') + + expect(await browser.elementByCss('title').text()).toBe('Home Page') + expect(await browser.elementsByCss('title')).toHaveLength(1) + + // navigate to the page that matches a parallel route + await browser.elementByCss("[href='/nested']").click() + await browser.waitForElementByCss('#slot') + + // the slot's background color should be red + expect(await getSlotBackgroundColor(browser)).toBe('rgb(255, 0, 0)') + + // there should no longer be a main element + expect(await browser.hasElementByCssSelector('#main')).toBeFalsy() + + // the slot background should still be red on a fresh load + await browser.refresh() + expect(await getSlotBackgroundColor(browser)).toBe('rgb(255, 0, 0)') + + // when we navigate from the route that matched the catch-all, we should see the CSS for the main element + await browser.elementByCss("[href='/']").click() + await browser.waitForElementByCss('#main') + + expect(await getChildrenBackgroundColor(browser)).toBe('rgb(0, 0, 255)') + }) +}) diff --git a/test/turbopack-build-tests-manifest.json b/test/turbopack-build-tests-manifest.json index bc6f5c8891678..cc3b369088c45 100644 --- a/test/turbopack-build-tests-manifest.json +++ b/test/turbopack-build-tests-manifest.json @@ -2415,6 +2415,15 @@ "flakey": [], "runtimeError": false }, + "test/e2e/app-dir/parallel-routes-css/parallel-routes-css.test.ts": { + "passed": [], + "failed": [ + "parallel-routes-catchall-css should properly load the Head content from multiple leaf segments" + ], + "pending": [], + "flakey": [], + "runtimeError": false + }, "test/e2e/app-dir/parallel-routes-layouts/parallel-routes-layouts.test.ts": { "passed": [ "parallel-routes-layouts should properly render layouts for multiple slots"