Fix app client child entry not being disposed when deleting the file (v…

…ercel#46583)

Currently if a file or folder (that contains an entry) is renamed in app dir, the dev server will stop working because we never remove the old entry. Since all client entries in app dir are created as child entries programmatically via the RSC plugin, they're different and not handled by our existing hot reloader logic:

https://github.com/vercel/next.js/blob/f0cbe84e4c2ed159df70a926d2f8f23b4c1025c9/packages/next/src/server/dev/hot-reloader.ts#L666-L677

This PR adds a file path to child entries as well (it can be layout, page and other entries) so in the entry generation step we can prune the invalid ones.

Fixes vercel#46379, fixes NEXT-650.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] 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/src/build/webpack/plugins/flight-client-entry-plugin.ts

@@ -247,7 +247,7 @@ export class FlightClientEntryPlugin {
 
         // Replace file suffix as `.js` will be added.
         const bundlePath = normalizePathSep(
-          relativeRequest.replace(/\.(js|ts)x?$/, '').replace(/^src[\\/]/, '')
+          relativeRequest.replace(/\.[^.\\/]+$/, '').replace(/^src[\\/]/, '')
         )
 
         addClientEntryAndSSRModulesList.push(
@@ -257,6 +257,7 @@ export class FlightClientEntryPlugin {
             entryName: name,
             clientImports,
             bundlePath,
+            absolutePagePath: entryRequest,
           })
         )
       }
@@ -574,12 +575,14 @@ export class FlightClientEntryPlugin {
     entryName,
     clientImports,
     bundlePath,
+    absolutePagePath,
   }: {
     compiler: webpack.Compiler
     compilation: webpack.Compilation
     entryName: string
     clientImports: ClientComponentImports
     bundlePath: string
+    absolutePagePath?: string
   }): [shouldInvalidate: boolean, addEntryPromise: Promise<void>] {
     let shouldInvalidate = false
 
@@ -618,6 +621,7 @@ export class FlightClientEntryPlugin {
         entries[pageKey] = {
           type: EntryTypes.CHILD_ENTRY,
           parentEntries: new Set([entryName]),
+          absoluteEntryFilePath: absolutePagePath,
           bundlePath,
           request: clientLoader,
           dispose: false,
@@ -634,6 +638,8 @@ export class FlightClientEntryPlugin {
         if (entryData.type === EntryTypes.CHILD_ENTRY) {
           entryData.parentEntries.add(entryName)
         }
+        entryData.dispose = false
+        entryData.lastActiveTime = Date.now()
       }
     } else {
       injectedClientEntries.set(bundlePath, clientLoader)

packages/next/src/server/dev/hot-reloader.ts

@@ -676,6 +676,19 @@ export default class HotReloader {
               }
             }
 
+            // For child entries, if it has an entry file and it's gone, remove it
+            if (isChildEntry) {
+              if (entryData.absoluteEntryFilePath) {
+                const pageExists =
+                  !dispose &&
+                  (await fileExists(entryData.absoluteEntryFilePath))
+                if (!pageExists) {
+                  delete entries[entryKey]
+                  return
+                }
+              }
+            }
+
             const hasAppDir = !!this.appDir
             const isAppPath = hasAppDir && bundlePath.startsWith('app/')
             const staticInfo = isEntry

packages/next/src/server/dev/next-dev-server.ts

@@ -1641,8 +1641,8 @@ export default class DevServer extends Server {
   }
 
   async getCompilationError(page: string): Promise<any> {
-    const errors = (await this.hotReloader?.getCompilationErrors(page)) || []
-    if (errors.length === 0) return
+    const errors = await this.hotReloader?.getCompilationErrors(page)
+    if (!errors) return
 
     // Return the very first error we found.
     return errors[0]

packages/next/src/server/dev/on-demand-entry-handler.ts

@@ -167,6 +167,11 @@ interface ChildEntry extends EntryType {
    * Which parent entries use this childEntry.
    */
   parentEntries: Set<string>
+  /**
+   * The absolute page to the entry file. Used for detecting if the file was removed. For example:
+   * `/Users/Rick/project/app/about/layout.js`
+   */
+  absoluteEntryFilePath?: string
 }
 
 const entriesMap: Map<
@@ -498,7 +503,7 @@ export function onDemandEntryHandler({
   multiCompiler.hooks.done.tap('NextJsOnDemandEntries', (multiStats) => {
     const [clientStats, serverStats, edgeServerStats] = multiStats.stats
     const root = !!appDir
-    const pagePaths = [
+    const entryNames = [
       ...getPagePathsFromEntrypoints(
         COMPILER_NAMES.client,
         clientStats.compilation.entrypoints,
@@ -518,8 +523,8 @@ export function onDemandEntryHandler({
         : []),
     ]
 
-    for (const page of pagePaths) {
-      const entry = curEntries[page]
+    for (const name of entryNames) {
+      const entry = curEntries[name]
       if (!entry) {
         continue
       }
@@ -529,7 +534,7 @@ export function onDemandEntryHandler({
       }
 
       entry.status = BUILT
-      doneCallbacks!.emit(page)
+      doneCallbacks!.emit(name)
     }
 
     getInvalidator(multiCompiler.outputPath)?.doneBuilding([...COMPILER_KEYS])

test/development/app-hmr/app/folder/page.jsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <h1>Hello</h1>
+}

test/development/app-hmr/app/layout.jsx

@@ -0,0 +1,7 @@
+export default function Root({ children }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}

test/development/app-hmr/hmr.test.ts

@@ -0,0 +1,38 @@
+import { createNextDescribe } from 'e2e-utils'
+import { check } from 'next-test-utils'
+
+createNextDescribe(
+  `app-dir-hmr`,
+  {
+    files: __dirname,
+  },
+  ({ next }) => {
+    describe('filesystem changes', () => {
+      it('should not break when renaming a folder', async () => {
+        console.log(next.url)
+        const browser = await next.browser('/folder')
+        const text = await browser.elementByCss('h1').text()
+        expect(text).toBe('Hello')
+
+        // Rename folder
+        await next.renameFolder('app/folder', 'app/folder-renamed')
+
+        try {
+          // Should be 404 in a few seconds
+          await check(async () => {
+            const body = await browser.elementByCss('body').text()
+            expect(body).toContain('404')
+            return 'success'
+          }, 'success')
+
+          // The new page should be rendered
+          const newHTML = await next.render('/folder-renamed')
+          expect(newHTML).toContain('Hello')
+        } finally {
+          // Rename it back
+          await next.renameFolder('app/folder-renamed', 'app/folder')
+        }
+      })
+    })
+  }
+)

test/development/app-hmr/next.config.js

@@ -0,0 +1,8 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+module.exports = {
+  experimental: {
+    appDir: true,
+  },
+}

test/lib/next-modes/base.ts

@@ -401,6 +401,12 @@ export class NextInstance {
       path.join(this.testDir, newFilename)
     )
   }
+  public async renameFolder(foldername: string, newFoldername: string) {
+    return fs.move(
+      path.join(this.testDir, foldername),
+      path.join(this.testDir, newFoldername)
+    )
+  }
   public async deleteFile(filename: string) {
     return fs.remove(path.join(this.testDir, filename))
   }