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

Previously we were assuming that `serverOnlyChanges` is the same as
"server component changes". However that's not always true, as one can
change a component from server component to client component, or vice
versa, where the change affects both server and client builds, so
`serverOnlyChanges` will be empty.

This PR fixes the logic by strictly hashing and comparing modules in the
server layer. Note that I intentionally skipped the test as this fix
[isn't
complete](https://vercel.slack.com/archives/C035J346QQL/p1670343453333079).

NEX-30

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ]
[e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

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<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>()
@@ -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) => {
@@ -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,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
@@ -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(

test/e2e/app-dir/app/app/dashboard/page/page.jsx

@@ -1,3 +1,5 @@
+// 'use client'
+
 export default function DashboardPagePage() {
   return (
     <>

test/e2e/app-dir/index.test.ts

@@ -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 () => {