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.

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

Fix chunk hash logic in hot-reloader for server components #43778

Merged
merged 5 commits into from Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 53 additions & 17 deletions packages/next/server/dev/hot-reloader.ts
Expand Up @@ -18,7 +18,7 @@ import {
import { watchCompilers } from '../../build/output'
import * as Log from '../../build/output/log'
import getBaseWebpackConfig from '../../build/webpack-config'
import { APP_DIR_ALIAS } from '../../lib/constants'
import { APP_DIR_ALIAS, WEBPACK_LAYERS } from '../../lib/constants'
import { recursiveDelete } from '../../lib/recursive-delete'
import {
BLOCKED_PAGES,
Expand Down Expand Up @@ -753,6 +753,8 @@ export default class HotReloader {
const changedClientPages = new Set<string>()
const changedServerPages = new Set<string>()
const changedEdgeServerPages = new Set<string>()

const changedServerComponentPages = new Set<string>()
const changedCSSImportPages = new Set<string>()

const prevClientPageHashes = new Map<string, string>()
Expand All @@ -761,7 +763,11 @@ export default class HotReloader {
const prevCSSImportModuleHashes = new Map<string, string>()

const trackPageChanges =
(pageHashMap: Map<string, string>, changedItems: Set<string>) =>
(
pageHashMap: Map<string, string>,
changedItems: Set<string>,
serverComponentChangeCallback?: (key: string) => void
) =>
(stats: webpack.Compilation) => {
try {
stats.entrypoints.forEach((entry, key) => {
Expand All @@ -778,6 +784,7 @@ export default class HotReloader {

let hasCSSModuleChanges = false
let chunksHash = new StringXor()
let chunksHashServerLayer = new StringXor()

modsIterable.forEach((mod: any) => {
if (
Expand All @@ -794,13 +801,28 @@ export default class HotReloader {
.digest()
.toString('hex')

if (
mod.layer === WEBPACK_LAYERS.server &&
mod?.buildInfo?.rsc?.type !== 'client'
) {
chunksHashServerLayer.add(hash)
}

chunksHash.add(hash)
} else {
// for non-pages we can use the module hash directly
const hash = stats.chunkGraph.getModuleHash(
mod,
chunk.runtime
)

if (
mod.layer === WEBPACK_LAYERS.server &&
mod?.buildInfo?.rsc?.type !== 'client'
) {
chunksHashServerLayer.add(hash)
}

chunksHash.add(hash)

// Both CSS import changes from server and client
Expand All @@ -819,14 +841,24 @@ export default class HotReloader {
}
}
})

const prevHash = pageHashMap.get(key)
const curHash = chunksHash.toString()

if (prevHash && prevHash !== curHash) {
changedItems.add(key)
}
pageHashMap.set(key, curHash)

if (serverComponentChangeCallback) {
const serverKey = WEBPACK_LAYERS.server + ':' + key
const prevServerHash = pageHashMap.get(serverKey)
const curServerHash = chunksHashServerLayer.toString()
if (prevServerHash && prevServerHash !== curServerHash) {
serverComponentChangeCallback(key)
}
pageHashMap.set(serverKey, curServerHash)
}

if (hasCSSModuleChanges) {
changedCSSImportPages.add(key)
}
Expand All @@ -845,11 +877,17 @@ export default class HotReloader {
)
this.multiCompiler.compilers[1].hooks.emit.tap(
'NextjsHotReloaderForServer',
trackPageChanges(prevServerPageHashes, changedServerPages)
trackPageChanges(prevServerPageHashes, changedServerPages, (key) =>
changedServerComponentPages.add(key)
)
)
this.multiCompiler.compilers[2].hooks.emit.tap(
'NextjsHotReloaderForServer',
trackPageChanges(prevEdgeServerPageHashes, changedEdgeServerPages)
trackPageChanges(
prevEdgeServerPageHashes,
changedEdgeServerPages,
(key) => changedServerComponentPages.add(key)
)
)

// This plugin watches for changes to _document.js and notifies the client side that it should reload the page
Expand Down Expand Up @@ -915,22 +953,14 @@ export default class HotReloader {
changedEdgeServerPages,
changedClientPages
)
const serverComponentChanges = serverOnlyChanges

const pageChanges = serverOnlyChanges
.concat(edgeServerOnlyChanges)
.filter((key) => key.startsWith('app/'))
.concat(Array.from(changedCSSImportPages))
const pageChanges = serverOnlyChanges.filter((key) =>
key.startsWith('pages/')
)
.filter((key) => key.startsWith('pages/'))
const middlewareChanges = Array.from(changedEdgeServerPages).filter(
(name) => isMiddlewareFilename(name)
)

changedClientPages.clear()
changedServerPages.clear()
changedEdgeServerPages.clear()
changedCSSImportPages.clear()

if (middlewareChanges.length > 0) {
this.send({
event: 'middlewareChanges',
Expand All @@ -946,13 +976,19 @@ export default class HotReloader {
})
}

if (serverComponentChanges.length > 0) {
if (changedServerComponentPages.size || changedCSSImportPages.size) {
this.send({
action: 'serverComponentChanges',
// TODO: granular reloading of changes
// entrypoints: serverComponentChanges,
})
}

changedClientPages.clear()
changedServerPages.clear()
changedEdgeServerPages.clear()
changedServerComponentPages.clear()
changedCSSImportPages.clear()
})

this.multiCompiler.compilers[0].hooks.failed.tap(
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/app-dir/app/app/dashboard/page/page.jsx
@@ -1,3 +1,5 @@
// 'use client'

export default function DashboardPagePage() {
return (
<>
Expand Down
59 changes: 59 additions & 0 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -440,6 +440,65 @@ describe('app dir', () => {
await next.patchFile(filePath, origContent)
}
})

it.skip('should HMR correctly when changing the component type', async () => {
const filePath = 'app/dashboard/page/page.jsx'
const origContent = await next.readFile(filePath)

try {
const browser = await webdriver(next.url, '/dashboard/page')

expect(await browser.elementByCss('p').text()).toContain(
'hello dashboard/page!'
)

// Test HMR with server component
await next.patchFile(
filePath,
origContent.replace(
'hello dashboard/page!',
'hello dashboard/page in server component!'
)
)
await check(
() => browser.elementByCss('p').text(),
/in server component/
)

// Change to client component
await new Promise((resolve) => setTimeout(resolve, 1000))
await next.patchFile(
filePath,
origContent
.replace("// 'use client'", "'use client'")
.replace(
'hello dashboard/page in server component!',
'hello dashboard/page in client component!'
)
)
await check(
() => browser.elementByCss('p').text(),
/in client component/
)

// Change back to server component
await next.patchFile(
filePath,
origContent
.replace("'use client'", "// 'use client'")
.replace(
'hello dashboard/page in client component!',
'hello dashboard/page in server component!'
)
)
await check(
() => browser.elementByCss('p').text(),
/in server component/
)
} finally {
await next.patchFile(filePath, origContent)
}
})
})

it('should handle hash in initial url', async () => {
Expand Down