rsc: bundle legacy head as client component (#43425)

Related to the build failure from #41931 We need to bundle `next/head` so that it can appear in the flight-client-manifest, so we changed the behavior to not treat it as non externals. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)
vercel · Nov 30, 2022 · 00836ad · 00836ad
1 parent b304a18
commit 00836ad
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 19 deletions.
diff --git a/packages/next-swc/crates/core/src/react_server_components.rs b/packages/next-swc/crates/core/src/react_server_components.rs
@@ -111,7 +111,7 @@ impl<C: Comments> ReactServerComponents<C> {
                                         return false;
                                     }
                                 }
-                                // Match `ParenthesisExpression` which is some formartting tools
+                                // Match `ParenthesisExpression` which is some formatting tools
                                 // usually do: ('use client'). In these case we need to throw
                                 // an exception because they are not valid directives.
                                 Expr::Paren(ParenExpr { expr, .. }) => {

diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts
@@ -1151,15 +1151,16 @@ export default async function getBaseWebpackConfig(
 
     const isLocalCallback = (localRes: string) => {
       // Makes sure dist/shared and dist/server are not bundled
-      // we need to process shared `router/router` and `dynamic`,
-      // so that the DefinePlugin can inject process.env values
+      // we need to process shared `router/router`, `head` and `dynamic`,
+      // so that the DefinePlugin can inject process.env values.
+
+      // Treat next internals as non-external for server layer
+      if (layer === WEBPACK_LAYERS.server) return
+
       const isNextExternal =
-        // Treat next internals as non-external for server layer
-        layer === WEBPACK_LAYERS.server
-          ? false
-          : /next[/\\]dist[/\\](esm[\\/])?(shared|server)[/\\](?!lib[/\\](router[/\\]router|dynamic))/.test(
-              localRes
-            )
+        /next[/\\]dist[/\\](esm[\\/])?(shared|server)[/\\](?!lib[/\\](router[/\\]router|dynamic|head[^-]))/.test(
+          localRes
+        )
 
       if (isNextExternal) {
         // Generate Next.js external import
@@ -1176,10 +1177,6 @@ export default async function getBaseWebpackConfig(
             .replace(/\\/g, '/')
         )
         return `commonjs ${externalRequest}`
-      } else if (layer !== WEBPACK_LAYERS.client) {
-        // We don't want to retry local requests
-        // with other preferEsm options
-        return
       }
     }
 

diff --git a/test/e2e/app-dir/app/app/internal/page.js b/test/e2e/app-dir/app/app/internal/page.js
@@ -1,14 +1,9 @@
 import Link from 'next/link'
-import Head from 'next/head'
 
 export default function Page() {
   return (
     <div>
       <div>
-        {/* NOTE: next/head will not work in RSC for now but not break either */}
-        <Head>
-          <title>internal-title</title>
-        </Head>
         <Link href="/internal/test/rewrite" id="navigate-rewrite">
           Navigate Rewrite
         </Link>

diff --git a/test/e2e/app-dir/head.test.ts b/test/e2e/app-dir/head.test.ts
@@ -113,6 +113,11 @@ describe('app dir head', () => {
         .waitForElementByCss('#layout', 2000)
       expect(await getTitle()).toBe('hello from dynamic blog page post-1')
     })
+
+    it('should treat next/head as client components but not apply', async () => {
+      const html = await renderViaHTTP(next.url, '/next-head')
+      expect(html).not.toMatch(/<title>legacy-head<\/title>/)
+    })
   }
 
   runTests()

diff --git a/test/e2e/app-dir/head/app/next-head/page.js b/test/e2e/app-dir/head/app/next-head/page.js
@@ -0,0 +1,12 @@
+import Head from 'next/head'
+
+export default function page() {
+  return (
+    <>
+      <Head>
+        <title>legacy-head</title>
+      </Head>
+      <p>page</p>
+    </>
+  )
+}
diff --git a/test/integration/externalize-next-server/test/index.test.js b/test/integration/externalize-next-server/test/index.test.js
@@ -10,9 +10,19 @@ describe('externalize next/dist/shared', () => {
     await nextBuild(appDir)
   })
 
-  it('Does not bundle next/dist/shared/lib/head.js in _error', async () => {
+  it('Bundle next/dist/shared/lib/head.js but not next/dist/shared/lib/head-manager-context.js in _error', async () => {
     const content = readNextBuildServerPageFile(appDir, '/_error')
     expect(content).toMatch(
+      new RegExp(
+        '^' +
+          escapeStringRegexp(
+            `module.exports = require("next/dist/shared/lib/head-manager-context.js");`
+          ) +
+          ';?$',
+        'm'
+      )
+    )
+    expect(content).not.toMatch(
       new RegExp(
         '^' +
           escapeStringRegexp(