make sure children is first in loader tree to fix head css bug on client navigation (#65279)

sokra · timneutkens · web-flow · commit 84602cdf6689 · 2024-05-08T08:35:25.000+02:00
### What? make sure children is first in loader tree to fix head css bug on client navigation ### Why? ### How? Fixes PACK-3028 --------- Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/packages/next-swc/crates/napi/Cargo.toml b/packages/next-swc/crates/napi/Cargo.toml
@@ -69,6 +69,7 @@ anyhow = "1.0.66"
 backtrace = "0.3"
 fxhash = "0.2.1"
 dhat = { workspace = true, optional = true }
+indexmap = { workspace = true }
 napi = { version = "2", default-features = false, features = [
   "napi3",
   "serde-json",
diff --git a/packages/next-swc/crates/napi/src/app_structure.rs b/packages/next-swc/crates/napi/src/app_structure.rs
@@ -1,6 +1,7 @@
 use std::{collections::HashMap, path::MAIN_SEPARATOR, sync::Arc};
 
 use anyhow::{anyhow, Result};
+use indexmap::IndexMap;
 use napi::{
     bindgen_prelude::External,
     threadsafe_function::{ErrorStrategy, ThreadsafeFunction, ThreadsafeFunctionCallMode},
@@ -42,7 +43,7 @@ async fn project_fs(project_dir: String, watching: bool) -> Result<Vc<Box<dyn Fi
 #[serde(rename_all = "camelCase")]
 struct LoaderTreeForJs {
     segment: String,
-    parallel_routes: HashMap<String, ReadRef<LoaderTreeForJs>>,
+    parallel_routes: IndexMap<String, ReadRef<LoaderTreeForJs>>,
     #[turbo_tasks(trace_ignore)]
     components: ComponentsForJs,
     #[turbo_tasks(trace_ignore)]
diff --git a/packages/next-swc/crates/next-core/src/app_structure.rs b/packages/next-swc/crates/next-core/src/app_structure.rs
@@ -1018,6 +1018,14 @@ async fn directory_tree_to_loader_tree(
         );
     }
 
+    if tree.parallel_routes.len() > 1
+        && tree.parallel_routes.keys().next().map(|s| s.as_str()) != Some("children")
+    {
+        // children must go first for next.js to work correctly
+        tree.parallel_routes
+            .move_index(tree.parallel_routes.len() - 1, 0);
+    }
+
     Ok(Vc::cell(Some(tree.cell())))
 }
 
diff --git a/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/@modal/abc/page.tsx b/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/@modal/abc/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <main></main>
+}
diff --git a/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/@modal/default.tsx b/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/@modal/default.tsx
@@ -0,0 +1,3 @@
+export default function Default() {
+  return null
+}
diff --git a/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/global.css b/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/global.css
@@ -0,0 +1,3 @@
+#global {
+  background: rgb(0, 255, 0);
+}
diff --git a/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/layout.tsx b/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/layout.tsx
@@ -0,0 +1,16 @@
+export default function Root({
+  children,
+  modal,
+}: {
+  children: React.ReactNode
+  modal: React.ReactNode
+}) {
+  return (
+    <html>
+      <body>
+        {children}
+        {modal}
+      </body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/page.tsx b/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/page.tsx
@@ -0,0 +1,15 @@
+'use client'
+
+import './global.css'
+import styles from './styles.module.css'
+
+export default function Page() {
+  return (
+    <main>
+      <p id="global">Hello World</p>
+      <p id="module" className={styles.module}>
+        Hello World
+      </p>
+    </main>
+  )
+}
diff --git a/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/source/page.tsx b/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/source/page.tsx
@@ -0,0 +1,9 @@
+import Link from 'next/link'
+
+export default function Page() {
+  return (
+    <main>
+      <Link href="/">Link</Link>
+    </main>
+  )
+}
diff --git a/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/styles.module.css b/test/e2e/app-dir/css-client-side-nav-parallel-routes/app/styles.module.css
@@ -0,0 +1,3 @@
+.module {
+  background: rgb(0, 255, 0);
+}
diff --git a/test/e2e/app-dir/css-client-side-nav-parallel-routes/css-client-side-nav-parallel-routes.test.ts b/test/e2e/app-dir/css-client-side-nav-parallel-routes/css-client-side-nav-parallel-routes.test.ts
@@ -0,0 +1,19 @@
+import { nextTestSetup } from 'e2e-utils'
+
+describe('css-client-side-nav-parallel-routes', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+
+  // Recommended for tests that need a full browser
+  it('should apply styles after navigation', async () => {
+    const browser = await next.browser('/source')
+    await browser.elementByCss('a').click()
+    expect(
+      await browser.elementByCss('#global').getComputedCss('background-color')
+    ).toBe('rgb(0, 255, 0)')
+    expect(
+      await browser.elementByCss('#module').getComputedCss('background-color')
+    ).toBe('rgb(0, 255, 0)')
+  })
+})
diff --git a/test/e2e/app-dir/css-client-side-nav-parallel-routes/next.config.js b/test/e2e/app-dir/css-client-side-nav-parallel-routes/next.config.js
@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig

Original file line number	Diff line number	Diff line change
`@@ -1018,6 +1018,14 @@ async fn directory_tree_to_loader_tree(`
`1018`	`1018`	`);`
`1019`	`1019`	`}`
`1020`	`1020`
	`1021`	`+ if tree.parallel_routes.len() > 1`
	`1022`	`+ && tree.parallel_routes.keys().next().map(\|s\| s.as_str()) != Some("children")`
	`1023`	`+ {`
	`1024`	`+ // children must go first for next.js to work correctly`
	`1025`	`+ tree.parallel_routes`
	`1026`	`+ .move_index(tree.parallel_routes.len() - 1, 0);`
	`1027`	`+ }`
	`1028`	`+`
`1021`	`1029`	`Ok(Vc::cell(Some(tree.cell())))`
`1022`	`1030`	`}`
`1023`	`1031`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export default function Page() {`
	`2`	`+ return <main></main>`
	`3`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export default function Default() {`
	`2`	`+ return null`
	`3`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+#global {`
	`2`	`+ background: rgb(0, 255, 0);`
	`3`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+.module {`
	`2`	`+ background: rgb(0, 255, 0);`
	`3`	`+}`