Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rsc: bundle legacy head as client component #43425

Merged
merged 6 commits into from Nov 30, 2022
Merged

rsc: bundle legacy head as client component #43425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5f023ef
rsc: bundle legacy head as client component
huozhi Nov 27, 2022
346bf7a
fix regex matching
huozhi Nov 27, 2022
4c2ca96
fix regex matching
huozhi Nov 28, 2022
fdba24a
change bundle tests
huozhi Nov 28, 2022
9f0b9ad
fix tests
huozhi Nov 28, 2022
7b860dc
Merge branch 'canary' into fix/head-bundle
huozhi Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/next-swc/crates/core/src/react_server_components.rs
View file
Open in desktop
Expand Up @@ -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, .. }) => {
Expand Down
21 changes: 9 additions & 12 deletions packages/next/build/webpack-config.ts
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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
}
}

Expand Down
5 changes: 0 additions & 5 deletions test/e2e/app-dir/app/app/internal/page.js
View file
Open in desktop
@@ -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>
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/head.test.ts
View file
Open in desktop
Expand Up @@ -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()
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/app-dir/head/app/next-head/page.js
View file
Open in desktop
@@ -0,0 +1,12 @@
import Head from 'next/head'

export default function page() {
return (
<>
<Head>
<title>legacy-head</title>
</Head>
<p>page</p>
</>
)
}
12 changes: 11 additions & 1 deletion test/integration/externalize-next-server/test/index.test.js
View file
Open in desktop
Expand Up @@ -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(
Expand Down