Fix misordered CSS resources (#48244)

### What? This PR fixes misordered CSS `<link>` tags. CSS imports in inner layers (e.g. a page) should always take precedence over outer layers (e.g. root layout), but currently it's reversed. ### Why? In layouts we usually define more general styles like globals, resets, and layout specific things. And in inner layers and pages, things need to be more detailed and override upper layers if there're any conflicts. Previously we defined the component segment as ```tsx <> <Component {...props} /> {assets} </> ``` which is necessary because of `findDOMNode` - if we put `assets` before the component, we can't find the correct scrolling DOM and position for that layer. However, with `assets` being the last Float will receive the reversed order of resources. ### How? I changed the `createComponentTree` function to return a `Component` and `assets` pair, so in the Layout Router they're no longer passed to the scroll wrapper altogether but separately. Closes NEXT-983 Fixes #47585, fixes #46347. fix NEXT-656 --------- Co-authored-by: JJ Kasper <jj@jjsweb.site>
vercel · Apr 17, 2023 · 2a61253 · 2a61253
1 parent 3ac7658
commit 2a61253
Show file tree

Hide file tree

Showing 8 changed files with 165 additions and 72 deletions.
diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx
@@ -453,6 +453,7 @@ export default function OuterLayoutRouter({
   notFound,
   notFoundStyles,
   asNotFound,
+  styles,
 }: {
   parallelRouterKey: string
   segmentPath: FlightSegmentPath
@@ -467,6 +468,7 @@ export default function OuterLayoutRouter({
   notFound: React.ReactNode | undefined
   notFoundStyles: React.ReactNode | undefined
   asNotFound?: boolean
+  styles?: React.ReactNode
 }) {
   const context = useContext(LayoutRouterContext)
   if (!context) {
@@ -501,6 +503,7 @@ export default function OuterLayoutRouter({
 
   return (
     <>
+      {styles}
       {preservedSegments.map((preservedSegment) => {
         const isChildPropSegment = matchSegment(
           preservedSegment,

diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx
@@ -468,15 +468,18 @@ export async function renderToHTMLOrFlight(
       return [Comp, styles]
     }
 
-    const createStaticAssets = async ({
+    const getLayerAssets = ({
       layoutOrPagePath,
       injectedCSS: injectedCSSWithCurrentLayout,
       injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout,
     }: {
       layoutOrPagePath: string | undefined
       injectedCSS: Set<string>
       injectedFontPreloadTags: Set<string>
-    }) => {
+    }): {
+      styles: React.ReactNode
+      preloads: React.ReactNode
+    } => {
       const stylesheets: string[] = layoutOrPagePath
         ? getCssInlinedLinkTags(
             clientReferenceManifest,
@@ -498,17 +501,19 @@ export async function renderToHTMLOrFlight(
           )
         : []
 
-      return (
+      const preloads = (
         <>
           {preloadedFontFiles?.length === 0 ? (
-            <link
-              data-next-font={
-                nextFontManifest?.appUsingSizeAdjust ? 'size-adjust' : ''
-              }
-              rel="preconnect"
-              href="/"
-              crossOrigin="anonymous"
-            />
+            <>
+              <link
+                data-next-font={
+                  nextFontManifest?.appUsingSizeAdjust ? 'size-adjust' : ''
+                }
+                rel="preconnect"
+                href="/"
+                crossOrigin="anonymous"
+              />
+            </>
           ) : null}
           {preloadedFontFiles
             ? preloadedFontFiles.map((fontFile) => {
@@ -528,27 +533,53 @@ export async function renderToHTMLOrFlight(
                 )
               })
             : null}
-          {stylesheets
-            ? stylesheets.map((href, index) => (
-                <link
-                  rel="stylesheet"
-                  // In dev, Safari will wrongly cache the resource if you preload it:
-                  // - https://github.com/vercel/next.js/issues/5860
-                  // - https://bugs.webkit.org/show_bug.cgi?id=187726
-                  // We used to add a `?ts=` query for resources in `pages` to bypass it,
-                  // but in this case it is fine as we don't need to preload the styles.
-                  href={`${assetPrefix}/_next/${href}`}
-                  // `Precedence` is an opt-in signal for React to handle
-                  // resource loading and deduplication, etc:
-                  // https://github.com/facebook/react/pull/25060
-                  // @ts-ignore
-                  precedence="next.js"
-                  key={index}
-                />
-              ))
-            : null}
         </>
       )
+
+      const styles = stylesheets
+        ? stylesheets.map((href, index) => (
+            <link
+              rel="stylesheet"
+              // In dev, Safari will wrongly cache the resource if you preload it:
+              // - https://github.com/vercel/next.js/issues/5860
+              // - https://bugs.webkit.org/show_bug.cgi?id=187726
+              // We used to add a `?ts=` query for resources in `pages` to bypass it,
+              // but in this case it is fine as we don't need to preload the styles.
+              href={`${assetPrefix}/_next/${href}`}
+              // `Precedence` is an opt-in signal for React to handle
+              // resource loading and deduplication, etc:
+              // https://github.com/facebook/react/pull/25060
+              // @ts-ignore
+              precedence="next.js"
+              key={index}
+            />
+          ))
+        : null
+
+      return {
+        styles,
+        preloads,
+      }
+    }
+
+    const parseLoaderTree = (tree: LoaderTree) => {
+      const [segment, parallelRoutes, components] = tree
+      const { layout } = components
+      let { page } = components
+      // a __DEFAULT__ segment means that this route didn't match any of the
+      // segments in the route, so we should use the default page
+
+      page = segment === '__DEFAULT__' ? components.defaultPage : page
+
+      const layoutOrPagePath = layout?.[1] || page?.[1]
+
+      return {
+        page,
+        segment,
+        components,
+        layoutOrPagePath,
+        parallelRoutes,
+      }
     }
 
     /**
@@ -572,29 +603,27 @@ export async function renderToHTMLOrFlight(
       injectedCSS: Set<string>
       injectedFontPreloadTags: Set<string>
       asNotFound?: boolean
-    }): Promise<{ Component: React.ComponentType }> => {
-      const [segment, parallelRoutes, components] = tree
+    }): Promise<{
+      Component: React.ComponentType
+      styles: React.ReactNode
+    }> => {
+      const { page, layoutOrPagePath, segment, components, parallelRoutes } =
+        parseLoaderTree(tree)
+
       const {
         layout,
         template,
         error,
         loading,
         'not-found': notFound,
       } = components
-      let { page } = components
-      // a __DEFAULT__ segment means that this route didn't match any of the
-      // segments in the route, so we should use the default page
-
-      page = segment === '__DEFAULT__' ? components.defaultPage : page
-
-      const layoutOrPagePath = layout?.[1] || page?.[1]
 
       const injectedCSSWithCurrentLayout = new Set(injectedCSS)
       const injectedFontPreloadTagsWithCurrentLayout = new Set(
         injectedFontPreloadTags
       )
 
-      const assets = createStaticAssets({
+      const { styles, preloads } = getLayerAssets({
         layoutOrPagePath,
         injectedCSS: injectedCSSWithCurrentLayout,
         injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout,
@@ -825,17 +854,19 @@ export async function renderToHTMLOrFlight(
             }
 
             // Create the child component
-            const { Component: ChildComponent } = await createComponentTree({
-              createSegmentPath: (child) => {
-                return createSegmentPath([...currentSegmentPath, ...child])
-              },
-              loaderTree: parallelRoute,
-              parentParams: currentParams,
-              rootLayoutIncluded: rootLayoutIncludedAtThisLevelOrAbove,
-              injectedCSS: injectedCSSWithCurrentLayout,
-              injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout,
-              asNotFound,
-            })
+            const { Component: ChildComponent, styles: childStyles } =
+              await createComponentTree({
+                createSegmentPath: (child) => {
+                  return createSegmentPath([...currentSegmentPath, ...child])
+                },
+                loaderTree: parallelRoute,
+                parentParams: currentParams,
+                rootLayoutIncluded: rootLayoutIncludedAtThisLevelOrAbove,
+                injectedCSS: injectedCSSWithCurrentLayout,
+                injectedFontPreloadTags:
+                  injectedFontPreloadTagsWithCurrentLayout,
+                asNotFound,
+              })
 
             const childProp: ChildProp = {
               current: <ChildComponent />,
@@ -871,6 +902,7 @@ export async function renderToHTMLOrFlight(
                 notFoundStyles={notFoundStyles}
                 asNotFound={asNotFound}
                 childProp={childProp}
+                styles={childStyles}
               />,
             ]
           }
@@ -890,6 +922,7 @@ export async function renderToHTMLOrFlight(
       if (!Component) {
         return {
           Component: () => <>{parallelRouteComponents.children}</>,
+          styles,
         }
       }
 
@@ -940,7 +973,7 @@ export async function renderToHTMLOrFlight(
         Component: () => {
           return (
             <>
-              {/* <Component /> needs to be the first element because we use `findDOMONode` in layout router to locate it. */}
+              {/* <Component /> needs to be the first element because we use `findDOMNode` in layout router to locate it. */}
               {isPage && isClientComponent && isStaticGeneration ? (
                 <StaticGenerationSearchParamsBailoutProvider
                   propsForComponent={props}
@@ -949,10 +982,11 @@ export async function renderToHTMLOrFlight(
               ) : (
                 <Component {...props} />
               )}
-              {assets}
+              {preloads}
             </>
           )
         },
+        styles,
       }
     }
 
@@ -1038,6 +1072,7 @@ export async function renderToHTMLOrFlight(
             canSegmentBeOverridden(actualSegment, flightRouterState[0])
               ? flightRouterState[0]
               : null
+
           return [
             [
               overriddenSegment ?? actualSegment,
@@ -1056,16 +1091,14 @@ export async function renderToHTMLOrFlight(
                     const { Component } = await createComponentTree(
                       // This ensures flightRouterPath is valid and filters down the tree
                       {
-                        createSegmentPath: (child) => {
-                          return createSegmentPath(child)
-                        },
+                        createSegmentPath,
                         loaderTree: loaderTreeToFilter,
                         parentParams: currentParams,
                         firstItem: isFirst,
                         injectedCSS,
                         injectedFontPreloadTags,
                         // This is intentionally not "rootLayoutIncludedAtThisLevelOrAbove" as createComponentTree starts at the current level and does a check for "rootLayoutAtThisLevel" too.
-                        rootLayoutIncluded: rootLayoutIncluded,
+                        rootLayoutIncluded,
                         asNotFound,
                       }
                     )
@@ -1074,7 +1107,23 @@ export async function renderToHTMLOrFlight(
                   }),
               isPrefetch && !Boolean(components.loading)
                 ? null
-                : rscPayloadHead,
+                : (() => {
+                    const { layoutOrPagePath } =
+                      parseLoaderTree(loaderTreeToFilter)
+
+                    const { styles } = getLayerAssets({
+                      layoutOrPagePath,
+                      injectedCSS: new Set(injectedCSS),
+                      injectedFontPreloadTags: new Set(injectedFontPreloadTags),
+                    })
+
+                    return (
+                      <>
+                        {styles}
+                        {rscPayloadHead}
+                      </>
+                    )
+                  })(),
             ],
           ]
         }
@@ -1271,7 +1320,7 @@ export async function renderToHTMLOrFlight(
         const injectedCSS = new Set<string>()
         const injectedFontPreloadTags = new Set<string>()
 
-        const { Component: ComponentTree } = await createComponentTree({
+        const { Component: ComponentTree, styles } = await createComponentTree({
           createSegmentPath: (child) => child,
           loaderTree,
           parentParams: {},
@@ -1299,12 +1348,6 @@ export async function renderToHTMLOrFlight(
           ? [DefaultNotFound]
           : []
 
-        const assets = createStaticAssets({
-          layoutOrPagePath: layout?.[1],
-          injectedCSS,
-          injectedFontPreloadTags,
-        })
-
         const initialTree = createFlightRouterStateFromLoaderTree(
           loaderTree,
           getDynamicParamFromSegment,
@@ -1313,6 +1356,7 @@ export async function renderToHTMLOrFlight(
 
         return (
           <>
+            {styles}
             <AppRouter
               assetPrefix={assetPrefix}
               initialCanonicalUrl={initialCanonicalUrl}
@@ -1331,13 +1375,10 @@ export async function renderToHTMLOrFlight(
               globalErrorComponent={GlobalError}
               notFound={
                 NotFound && RootLayout ? (
-                  <>
-                    {assets}
-                    <RootLayout params={{}}>
-                      {notFoundStyles}
-                      <NotFound />
-                    </RootLayout>
-                  </>
+                  <RootLayout params={{}}>
+                    {notFoundStyles}
+                    <NotFound />
+                  </RootLayout>
                 ) : undefined
               }
               asNotFound={props.asNotFound}

diff --git a/test/e2e/app-dir/app-css/app/ordering/index.css b/test/e2e/app-dir/app-css/app/ordering/index.css
@@ -0,0 +1,7 @@
+@layer default;
+
+@layer default {
+  h1 {
+    color: black;
+  }
+}
diff --git a/test/e2e/app-dir/app-css/app/ordering/input.css b/test/e2e/app-dir/app-css/app/ordering/input.css
@@ -0,0 +1,9 @@
+@layer default {
+  input[type='text'] {
+    padding: 0.5rem;
+  }
+
+  h1 {
+    color: red;
+  }
+}
diff --git a/test/e2e/app-dir/app-css/app/ordering/input.js b/test/e2e/app-dir/app-css/app/ordering/input.js
@@ -0,0 +1,7 @@
+'use client'
+
+import './input.css'
+
+export default function InputText() {
+  return <input />
+}
diff --git a/test/e2e/app-dir/app-css/app/ordering/layout.js b/test/e2e/app-dir/app-css/app/ordering/layout.js
@@ -0,0 +1,5 @@
+import './index.css'
+
+export default function Layout({ children }) {
+  return <div>{children}</div>
+}
diff --git a/test/e2e/app-dir/app-css/app/ordering/page.js b/test/e2e/app-dir/app-css/app/ordering/page.js
@@ -0,0 +1,10 @@
+import InputText from './input'
+
+export default function Home() {
+  return (
+    <div>
+      <h1>Hello</h1>
+      <InputText />
+    </div>
+  )
+}