From 01f9a97846c0d783696f4354a655724059149cb0 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 6 Dec 2022 17:44:08 +0100 Subject: [PATCH] fix hash logic for server components --- packages/next/server/dev/hot-reloader.ts | 70 ++++++++++++++----- .../app-dir/app/app/dashboard/page/page.jsx | 2 + test/e2e/app-dir/index.test.ts | 59 ++++++++++++++++ 3 files changed, 114 insertions(+), 17 deletions(-) diff --git a/packages/next/server/dev/hot-reloader.ts b/packages/next/server/dev/hot-reloader.ts index 940ab3e35e6947a..d95f281a9fd361d 100644 --- a/packages/next/server/dev/hot-reloader.ts +++ b/packages/next/server/dev/hot-reloader.ts @@ -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, @@ -753,6 +753,8 @@ export default class HotReloader { const changedClientPages = new Set() const changedServerPages = new Set() const changedEdgeServerPages = new Set() + + const changedServerComponentPages = new Set() const changedCSSImportPages = new Set() const prevClientPageHashes = new Map() @@ -761,7 +763,11 @@ export default class HotReloader { const prevCSSImportModuleHashes = new Map() const trackPageChanges = - (pageHashMap: Map, changedItems: Set) => + ( + pageHashMap: Map, + changedItems: Set, + serverComponentChangeCallback?: (key: string) => void + ) => (stats: webpack.Compilation) => { try { stats.entrypoints.forEach((entry, key) => { @@ -778,6 +784,7 @@ export default class HotReloader { let hasCSSModuleChanges = false let chunksHash = new StringXor() + let chunksHashServerLayer = new StringXor() modsIterable.forEach((mod: any) => { if ( @@ -794,6 +801,13 @@ 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 @@ -801,6 +815,14 @@ export default class HotReloader { 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 @@ -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) } @@ -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 @@ -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', @@ -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( diff --git a/test/e2e/app-dir/app/app/dashboard/page/page.jsx b/test/e2e/app-dir/app/app/dashboard/page/page.jsx index d328445b834cf44..042fc7e3f927f47 100644 --- a/test/e2e/app-dir/app/app/dashboard/page/page.jsx +++ b/test/e2e/app-dir/app/app/dashboard/page/page.jsx @@ -1,3 +1,5 @@ +// 'use client' + export default function DashboardPagePage() { return ( <> diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 3bf7b92d542f8a7..41237003d0f073f 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -419,6 +419,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 () => {