From ceef71945990e6be72ea6f543a19ef6484217882 Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Mon, 3 Jun 2024 16:35:31 -0700 Subject: [PATCH] fix missing stylesheets when parallel routes are present (#66300) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This takes the `layerAssets` property from the previous PR and actually renders it, replacing the previous style handling. This ensures that when multiple page segments are rendered on screen, all of their associated CSS files are loaded. The existing `findHeadInCache` method only ever returns a single head node, which means it’d miss stylesheets. Fixes #59308 Fixes #63465 --- .../next/src/client/components/app-router.tsx | 23 +++++++- .../src/client/components/layout-router.tsx | 19 ++++++- .../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, 212 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..a6ea8659406fb 100644 --- a/packages/next/src/client/components/layout-router.tsx +++ b/packages/next/src/client/components/layout-router.tsx @@ -456,6 +456,23 @@ function InnerLayoutRouter({ } } + // 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 = + childNode.prefetchLayerAssets !== null + ? childNode.prefetchLayerAssets + : childNode.layerAssets + + const layerAssets = useDeferredValue( + childNode.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 + ) + // If we get to this point, then we know we have something we can render. const subtree = ( // The layout router context narrows down tree and childNodes at each level. @@ -468,7 +485,7 @@ function InnerLayoutRouter({ loading: childNode.loading, }} > - {/* {childNode.layerAssets} */} + {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 bbe34eed82437..3a50b406686b8 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -482,34 +482,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 f1d10887d6279..24456d556489f 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"