Fix not found css not being preloaded while navigation (#53906)

huozhi · web-flow · commit 9834ab7e158e · 2023-08-23T13:07:30.000Z
### Problem One style of `not-found` has `precendence` property with "undefined" value, which can't be handled by React Float, then during navigation the style could not load properly, lead to the style missing in issue #53210. ### Solution Always enable `precendence` for all links, so all the css styles of page and convention components can be hoist by react properly. Float will decide which one should be handled. Previously this change only applies to template, actually we can apply it to all components so that they can all be handled properly especially during client navigation. Related react change: facebook/react#27265 Fixes #53210
diff --git a/package.json b/package.json
@@ -82,6 +82,7 @@
     "@next/swc": "workspace:*",
     "@next/third-parties": "workspace:*",
     "@opentelemetry/api": "1.4.1",
+    "@picocss/pico": "1.5.10",
     "@svgr/webpack": "5.5.0",
     "@swc/cli": "0.1.55",
     "@swc/core": "1.3.55",
diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx
@@ -401,12 +401,10 @@ export async function renderToHTMLOrFlight(
     const createComponentAndStyles = async ({
       filePath,
       getComponent,
-      shouldPreload,
       injectedCSS,
     }: {
       filePath: string
       getComponent: () => any
-      shouldPreload?: boolean
       injectedCSS: Set<string>
     }): Promise<any> => {
       const cssHrefs = getCssInlinedLinkTags(
@@ -433,11 +431,8 @@ export async function renderToHTMLOrFlight(
             // During HMR, it's critical to use different `precedence` values
             // for different stylesheets, so their order will be kept.
             // https://github.com/facebook/react/pull/25060
-            const precedence = shouldPreload
-              ? process.env.NODE_ENV === 'development'
-                ? 'next_' + href
-                : 'next'
-              : undefined
+            const precedence =
+              process.env.NODE_ENV === 'development' ? 'next_' + href : 'next'
 
             return (
               <link
@@ -614,7 +609,6 @@ export async function renderToHTMLOrFlight(
         ? await createComponentAndStyles({
             filePath: template[1],
             getComponent: template[0],
-            shouldPreload: true,
             injectedCSS: injectedCSSWithCurrentLayout,
           })
         : [React.Fragment]
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/test/e2e/app-dir/app-css/index.test.ts b/test/e2e/app-dir/app-css/index.test.ts
@@ -175,11 +175,11 @@ createNextDescribe(
 
       describe('special entries', () => {
         it('should include css imported in loading.js', async () => {
-          const html = await next.render('/loading-bug/hi')
-          // The link tag should be included together with loading
-          expect(html).toMatch(
-            /<link rel="stylesheet" href="(.+)\.css(\?v=\d+)?"\/><h2>Loading...<\/h2>/
-          )
+          const $ = await next.render$('/loading-bug/hi')
+          // The link tag should be hoist into head with precedence properties
+          expect($('head link[data-precedence]').length).toBe(2)
+
+          expect($('body h2').text()).toBe('Loading...')
         })
 
         it('should include css imported in client template.js', async () => {
diff --git a/test/e2e/app-dir/not-found/css-precedence/app/layout.js b/test/e2e/app-dir/not-found/css-precedence/app/layout.js
@@ -0,0 +1,7 @@
+export default function Layout({ children }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/not-found/css-precedence/app/not-found.js b/test/e2e/app-dir/not-found/css-precedence/app/not-found.js
@@ -0,0 +1,23 @@
+'use client'
+
+import { Button } from '../components/button/button'
+import { useRouter } from 'next/navigation'
+
+function NotFound() {
+  const router = useRouter()
+  return (
+    <div>
+      <h1>404 - Page Not Found</h1>
+      <Button
+        id="go-to-index"
+        onClick={() => {
+          router.push('/')
+        }}
+      >
+        Home
+      </Button>
+    </div>
+  )
+}
+
+export default NotFound
diff --git a/test/e2e/app-dir/not-found/css-precedence/app/page.js b/test/e2e/app-dir/not-found/css-precedence/app/page.js
@@ -0,0 +1,20 @@
+'use client'
+
+import { useRouter } from 'next/navigation'
+import { Button } from '../components/button/button'
+
+export default function Page() {
+  const router = useRouter()
+  return (
+    <>
+      <Button
+        id="go-to-404"
+        onClick={() => {
+          router.push('/404')
+        }}
+      >
+        Not Found
+      </Button>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/not-found/css-precedence/components/button/button.js b/test/e2e/app-dir/not-found/css-precedence/components/button/button.js
@@ -0,0 +1,12 @@
+'use client'
+
+import styles from './button.module.css'
+import React from 'react'
+
+export const Button = ({ children, ...props }) => {
+  return (
+    <button {...props} className={styles.button}>
+      {children}
+    </button>
+  )
+}
diff --git a/test/e2e/app-dir/not-found/css-precedence/components/button/button.module.css b/test/e2e/app-dir/not-found/css-precedence/components/button/button.module.css
@@ -0,0 +1,7 @@
+.button {
+  border: 1px solid #ccc;
+  border-radius: 4px;
+  background: rgb(0, 128, 0);
+  color: #fff;
+  padding: 8px 16px;
+}
diff --git a/test/e2e/app-dir/not-found/css-precedence/components/navigate-404-section.js b/test/e2e/app-dir/not-found/css-precedence/components/navigate-404-section.js
@@ -0,0 +1,20 @@
+'use client'
+
+import { Button } from './button/button'
+import { useRouter } from 'next/navigation'
+
+export default function Navigate404Section() {
+  const router = useRouter()
+  return (
+    <p>
+      <Button
+        id="go-to-navigate-404"
+        onClick={() => {
+          router.push('/navigate-404')
+        }}
+      >
+        Go to /navigate-404
+      </Button>
+    </p>
+  )
+}
diff --git a/test/e2e/app-dir/not-found/css-precedence/index.test.ts b/test/e2e/app-dir/not-found/css-precedence/index.test.ts
@@ -0,0 +1,43 @@
+import { createNextDescribe } from 'e2e-utils'
+import { check } from 'next-test-utils'
+
+createNextDescribe(
+  'app dir css',
+  {
+    files: __dirname,
+    skipDeployment: true,
+    dependencies: {
+      sass: 'latest',
+    },
+  },
+  ({ next }) => {
+    it('should load css while navigation between not-found and page', async () => {
+      const browser = await next.browser('/')
+      await check(
+        async () =>
+          await browser.eval(
+            `window.getComputedStyle(document.querySelector('#go-to-404')).backgroundColor`
+          ),
+        'rgb(0, 128, 0)'
+      )
+      await browser.elementByCss('#go-to-404').click()
+      await browser.waitForElementByCss('#go-to-index')
+      await check(
+        async () =>
+          await browser.eval(
+            `window.getComputedStyle(document.querySelector('#go-to-index')).backgroundColor`
+          ),
+        'rgb(0, 128, 0)'
+      )
+      await browser.elementByCss('#go-to-index').click()
+      await browser.waitForElementByCss('#go-to-404')
+      await check(
+        async () =>
+          await browser.eval(
+            `window.getComputedStyle(document.querySelector('#go-to-404')).backgroundColor`
+          ),
+        'rgb(0, 128, 0)'
+      )
+    })
+  }
+)
