From 5f023ef316d2fcdb1aea3f37556d7d17e72ba977 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 27 Nov 2022 12:53:51 +0100 Subject: [PATCH 1/5] rsc: bundle legacy head as client component --- .../crates/core/src/react_server_components.rs | 2 +- packages/next/build/webpack-config.ts | 6 +++--- test/e2e/app-dir/app/app/internal/page.js | 5 ----- test/e2e/app-dir/head.test.ts | 5 +++++ test/e2e/app-dir/head/app/next-head/page.js | 12 ++++++++++++ 5 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 test/e2e/app-dir/head/app/next-head/page.js diff --git a/packages/next-swc/crates/core/src/react_server_components.rs b/packages/next-swc/crates/core/src/react_server_components.rs index 45a52f3bbe35..52f5860ab1af 100644 --- 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 ReactServerComponents { 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 index 4d7df85fd41a..8d0d8d5c9064 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -1151,13 +1151,13 @@ 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. 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( + : /next[/\\]dist[/\\](esm[\\/])?(shared|server)[/\\](?!lib[/\\](router[/\\]router|dynamic|head))/.test( localRes ) diff --git a/test/e2e/app-dir/app/app/internal/page.js b/test/e2e/app-dir/app/app/internal/page.js index 134daec5bbb4..525104c574ac 100644 --- 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 (
- {/* NOTE: next/head will not work in RSC for now but not break either */} - - internal-title - Navigate Rewrite diff --git a/test/e2e/app-dir/head.test.ts b/test/e2e/app-dir/head.test.ts index dfd4c126cd05..c441577a54bf 100644 --- 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(/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 new file mode 100644 index 000000000000..e55823f8d945 --- /dev/null +++ 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 + +

page

+ + ) +} From 346bf7a8eef77833624fe92357b3c1418e6b3b46 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 27 Nov 2022 14:12:47 +0100 Subject: [PATCH 2/5] fix regex matching --- packages/next/build/webpack-config.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index 8d0d8d5c9064..c03e86e63682 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -1150,16 +1150,17 @@ export default async function getBaseWebpackConfig( const isEsmRequested = dependencyType === 'esm' const isLocalCallback = (localRes: string) => { + if (layer === WEBPACK_LAYERS.server || layer === WEBPACK_LAYERS.client) { + return + } // Makes sure dist/shared and dist/server are not bundled - // we need to process shared `router/router`, `head` and `dynamic`, + // we need to process shared `router/router` and `dynamic`, // so that the DefinePlugin can inject process.env values. 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|head))/.test( - localRes - ) + /next[/\\]dist[/\\](esm[\\/])?(shared|server)[/\\](?!lib[/\\](router[/\\]router|dynamic))/.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 } } From 4c2ca961d83947dd595c0d1f131325963f43b56d Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 28 Nov 2022 16:36:02 +0100 Subject: [PATCH 3/5] fix regex matching --- packages/next/build/webpack-config.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index c03e86e63682..650a6b31d446 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -1150,15 +1150,15 @@ export default async function getBaseWebpackConfig( const isEsmRequested = dependencyType === 'esm' const isLocalCallback = (localRes: string) => { - if (layer === WEBPACK_LAYERS.server || layer === WEBPACK_LAYERS.client) { - return - } // Makes sure dist/shared and dist/server are not bundled - // we need to process shared `router/router` and `dynamic`, + // 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 - /next[/\\]dist[/\\](esm[\\/])?(shared|server)[/\\](?!lib[/\\](router[/\\]router|dynamic))/.test( + /next[/\\]dist[/\\](esm[\\/])?(shared|server)[/\\](?!lib[/\\](router[/\\]router|dynamic|head[^-]))/.test( localRes ) From fdba24a3851b0b11ba6bada3e76a993ceba6244c Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 28 Nov 2022 17:42:29 +0100 Subject: [PATCH 4/5] change bundle tests --- .../externalize-next-server/test/index.test.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/integration/externalize-next-server/test/index.test.js b/test/integration/externalize-next-server/test/index.test.js index bad17940528b..c27c21b36135 100644 --- a/test/integration/externalize-next-server/test/index.test.js +++ b/test/integration/externalize-next-server/test/index.test.js @@ -10,17 +10,27 @@ 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.js");` + `module.exports = require("next/dist/shared/lib/head-context-manager.js");` ) + ';?$', 'm' ) ) }) + expect(content).not.toMatch( + new RegExp( + '^' + + escapeStringRegexp( + `module.exports = require("next/dist/shared/lib/head.js");` + ) + + ';?$', + 'm' + ) + ) }) From 9f0b9ad2183f4a06647044efa89b1b442e883b60 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 28 Nov 2022 17:47:43 +0100 Subject: [PATCH 5/5] fix tests --- .../test/index.test.js | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/integration/externalize-next-server/test/index.test.js b/test/integration/externalize-next-server/test/index.test.js index c27c21b36135..c88f4627c1fd 100644 --- a/test/integration/externalize-next-server/test/index.test.js +++ b/test/integration/externalize-next-server/test/index.test.js @@ -10,27 +10,27 @@ describe('externalize next/dist/shared', () => { await nextBuild(appDir) }) - it('Bundle next/dist/shared/lib/head.js but not next/dist/shared/lib/head-manager.context.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-context-manager.js");` + `module.exports = require("next/dist/shared/lib/head-manager-context.js");` ) + ';?$', 'm' ) ) - }) - expect(content).not.toMatch( - new RegExp( - '^' + - escapeStringRegexp( - `module.exports = require("next/dist/shared/lib/head.js");` - ) + - ';?$', - 'm' + expect(content).not.toMatch( + new RegExp( + '^' + + escapeStringRegexp( + `module.exports = require("next/dist/shared/lib/head.js");` + ) + + ';?$', + 'm' + ) ) - ) + }) })