Skip to content

Commit

Permalink
fix missing stylesheets when parallel routes are present (#66300)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ztanner authored Jun 3, 2024
1 parent 786a703 commit ceef719
Show file tree
Hide file tree
Showing 13 changed files with 212 additions and 55 deletions.
23 changes: 21 additions & 2 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ function Head({
function Router({
buildId,
initialHead,
initialLayerAssets,
initialTree,
initialCanonicalUrl,
initialSeedData,
Expand All @@ -310,7 +311,7 @@ function Router({
initialParallelRoutes,
location: !isServer ? window.location : null,
initialHead,
initialLayerAssets: null,
initialLayerAssets,
couldBeIntercepted,
}),
[
Expand All @@ -319,6 +320,7 @@ function Router({
initialCanonicalUrl,
initialTree,
initialHead,
initialLayerAssets,
couldBeIntercepted,
]
)
Expand Down Expand Up @@ -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 = (
<RedirectBoundary>
{head}
{/* {cache.layerAssets} */}
{layerAssets}
{cache.rsc}
<AppRouterAnnouncer tree={tree} />
</RedirectBoundary>
Expand Down
19 changes: 18 additions & 1 deletion packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -468,7 +485,7 @@ function InnerLayoutRouter({
loading: childNode.loading,
}}
>
{/* {childNode.layerAssets} */}
{layerAssets}
{resolvedRsc}
</LayoutRouterContext.Provider>
)
Expand Down
53 changes: 25 additions & 28 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -482,34 +482,31 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) {
typeof varyHeader === 'string' && varyHeader.includes(NEXT_URL)

return (
<>
{styles}
<AppRouter
buildId={ctx.renderOpts.buildId}
assetPrefix={ctx.assetPrefix}
initialCanonicalUrl={urlPathname}
// This is the router state tree.
initialTree={initialTree}
// This is the tree of React nodes that are seeded into the cache
initialSeedData={seedData}
couldBeIntercepted={couldBeIntercepted}
initialHead={
<>
{typeof ctx.res.statusCode === 'number' &&
ctx.res.statusCode > 400 && (
<meta name="robots" content="noindex" />
)}
{/* Adding requestId as react key to make metadata remount for each render */}
<MetadataTree key={ctx.requestId} />
</>
}
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}
/>
</>
<AppRouter
buildId={ctx.renderOpts.buildId}
assetPrefix={ctx.assetPrefix}
initialCanonicalUrl={urlPathname}
// This is the router state tree.
initialTree={initialTree}
// This is the tree of React nodes that are seeded into the cache
initialSeedData={seedData}
couldBeIntercepted={couldBeIntercepted}
initialHead={
<>
{typeof ctx.res.statusCode === 'number' &&
ctx.res.statusCode > 400 && (
<meta name="robots" content="noindex" />
)}
{/* Adding requestId as react key to make metadata remount for each render */}
<MetadataTree key={ctx.requestId} />
</>
}
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}
/>
)
}

Expand Down
46 changes: 22 additions & 24 deletions test/development/acceptance-app/hydration-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -604,40 +604,38 @@ describe('Error overlay for hydration errors', () => {
if (isTurbopack) {
expect(fullPseudoHtml).toMatchInlineSnapshot(`
"...
<NotFoundErrorBoundary pathname="/" notFound={[...]} notFoundStyles={[...]} asNotFound={undefined} missingSlots={Set}>
<RedirectBoundary>
<RedirectErrorBoundary router={{...}}>
<InnerLayoutRouter parallelRouterKey="children" url="/" tree={[...]} childNodes={Map} segmentPath={[...]} ...>
<ClientPageRoot props={{params:{}, ...}} Component={function Page}>
<Page params={{}} searchParams={{}}>
<RedirectBoundary>
<RedirectErrorBoundary router={{...}}>
<InnerLayoutRouter parallelRouterKey="children" url="/" tree={[...]} childNodes={Map} segmentPath={[...]} ...>
<ClientPageRoot props={{params:{}, ...}} Component={function Page}>
<Page params={{}} searchParams={{}}>
<div>
<div>
<div>
<div>
<div>
<Mismatch>
<p>
<span>
+ client
- server"
<Mismatch>
<p>
<span>
+ client
- server"
`)
} else {
expect(fullPseudoHtml).toMatchInlineSnapshot(`
"...
<NotFoundErrorBoundary pathname="/" notFound={[...]} notFoundStyles={[...]} asNotFound={undefined} missingSlots={Set}>
<RedirectBoundary>
<RedirectErrorBoundary router={{...}}>
<InnerLayoutRouter parallelRouterKey="children" url="/" tree={[...]} childNodes={Map} segmentPath={[...]} ...>
<ClientPageRoot props={{params:{}, ...}} Component={function Page}>
<Page params={{}} searchParams={{}}>
<RedirectBoundary>
<RedirectErrorBoundary router={{...}}>
<InnerLayoutRouter parallelRouterKey="children" url="/" tree={[...]} childNodes={Map} segmentPath={[...]} ...>
<ClientPageRoot props={{params:{}, ...}} Component={function Page}>
<Page params={{}} searchParams={{}}>
<div>
<div>
<div>
<div>
<div>
<Mismatch>
<p>
<span>
+ client
- server"
<Mismatch>
<p>
<span>
+ client
- server"
`)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import styles from './style.module.css'

export default function Page() {
return (
<div className={styles.bgRed} id="slot">
Slot
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.bgRed {
background-color: red;
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/parallel-routes-css/app/@foo/default.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return null
}
20 changes: 20 additions & 0 deletions test/e2e/app-dir/parallel-routes-css/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { ReactNode } from 'react'

const RootLayout = ({
children,
foo,
}: {
children: ReactNode
foo: ReactNode
}) => {
return (
<html lang="en">
<body>
{children}
{foo}
</body>
</html>
)
}

export default RootLayout
14 changes: 14 additions & 0 deletions test/e2e/app-dir/parallel-routes-css/app/nested/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import Link from 'next/link'

export const metadata = {
title: 'Nested Page',
}

export default function Page() {
return (
<div className="sub">
<p>Sub</p>
<Link href="/">Home</Link>
</div>
)
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/parallel-routes-css/app/page.tsx
Original file line number Diff line number Diff line change
@@ -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 id="main" className={styles.bgBlue}>
<p>Main</p>
<Link href="/nested">Nested Page</Link>
</main>
)
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/parallel-routes-css/app/style.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.bgBlue {
background-color: blue;
}
50 changes: 50 additions & 0 deletions test/e2e/app-dir/parallel-routes-css/parallel-routes-css.test.ts
Original file line number Diff line number Diff line change
@@ -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)')
})
})
9 changes: 9 additions & 0 deletions test/turbopack-build-tests-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit ceef719

Please sign in to comment.