review feedback

Author: lukesandberg

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

@@ -59,6 +59,7 @@ import {
   processTopLevelIssues,
   printNonFatalIssue,
   normalizedPageToTurbopackStructureRoute,
+  type StartChangeSubscription,
 } from './turbopack-utils'
 import {
   propagateServerField,
@@ -1081,10 +1082,20 @@ export async function createHotReloaderTurbopack(
               // trailing /page or /route segment (e.g. "/page" for the root route,
               // "/blog/[slug]/page" for a dynamic page). Use normalizeAppPath to
               // strip that suffix and find the entry matching the user-facing route.
-              let appOriginalName: string | undefined
+              let extraOptions: object | undefined = undefined
               for (const [name] of currentEntrypoints.app) {
                 if (normalizeAppPath(name) === page) {
-                  appOriginalName = name
+                  extraOptions = {
+                    // Synthesize a definition so ensurePage bypasses findPagePathData.
+                    // Only page and bundlePath are used from the definition:
+                    // - page: the originalName used as the route key for currentEntrypoints lookup
+                    // - bundlePath: must start with "app/" to set isInsideAppDir=true
+                    definition: {
+                      page: name,
+                      bundlePath: `app${name}`,
+                      filename: '',
+                    } as any,
+                  }
                   break
                 }
               }
@@ -1098,19 +1109,7 @@ export async function createHotReloaderTurbopack(
                 // updates for routes compiled this way, and these subscriptions
                 // are never unsubscribed (see TODOs in handleRouteType).
                 subscribeToChanges: false,
-                ...(appOriginalName
-                  ? {
-                      // Synthesize a definition so ensurePage bypasses findPagePathData.
-                      // Only page and bundlePath are used from the definition:
-                      // - page: the originalName used as the route key for currentEntrypoints lookup
-                      // - bundlePath: must start with "app/" to set isInsideAppDir=true
-                      definition: {
-                        page: appOriginalName,
-                        bundlePath: `app${appOriginalName}`,
-                        filename: '',
-                      } as any,
-                    }
-                  : {}),
+                ...extraOptions,
               }
 
               // Snapshot the current issue maps before compilation so we can
@@ -1798,11 +1797,11 @@ export async function createHotReloaderTurbopack(
               logErrors: true,
 
               hooks: {
-                // Omit subscribeToChanges to skip wiring HMR subscriptions for
+                // Pass a no-o subscribeToChanges to skip wiring HMR subscriptions for
                 // one-shot compilations (e.g. compile_route MCP tool).
-                ...(subscribeToChanges
-                  ? { subscribeToChanges: subscribeToClientChanges }
-                  : null),
+                subscribeToChanges: subscribeToChanges
+                  ? subscribeToClientChanges
+                  : ((async () => {}) as StartChangeSubscription),
                 handleWrittenEndpoint: (id, result, forceDeleteCache) => {
                   currentWrittenEntrypoints.set(id, result)
                   assetMapper.setPathsForKey(id, result.clientPaths)

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

@@ -140,7 +140,7 @@ export type ClientStateMap = WeakMap<ws, ClientState>
 // is no client to receive updates and no unsubscribe path.
 type HandleRouteTypeHooks = {
   handleWrittenEndpoint: HandleWrittenEndpoint
-  subscribeToChanges?: StartChangeSubscription
+  subscribeToChanges: StartChangeSubscription
 }
 
 export async function handleRouteType({
@@ -258,7 +258,7 @@ export async function handleRouteType({
         if (dev) {
           // TODO subscriptions should only be caused by the WebSocket connections
           // otherwise we don't known when to unsubscribe and this leaking
-          hooks?.subscribeToChanges?.(
+          hooks?.subscribeToChanges(
             serverKey,
             false,
             route.dataEndpoint,
@@ -277,7 +277,7 @@ export async function handleRouteType({
               }
             }
           )
-          hooks?.subscribeToChanges?.(
+          hooks?.subscribeToChanges(
             clientKey,
             false,
             route.htmlEndpoint,
@@ -294,7 +294,7 @@ export async function handleRouteType({
             }
           )
           if (entrypoints.global.document) {
-            hooks?.subscribeToChanges?.(
+            hooks?.subscribeToChanges(
               getEntryKey('pages', 'server', '_document'),
               false,
               entrypoints.global.document,
@@ -352,7 +352,7 @@ export async function handleRouteType({
       if (dev) {
         // TODO subscriptions should only be caused by the WebSocket connections
         // otherwise we don't known when to unsubscribe and this leaking
-        hooks?.subscribeToChanges?.(
+        hooks?.subscribeToChanges(
           key,
           true,
           route.rscEndpoint,
@@ -874,7 +874,7 @@ export async function handlePagesErrorRoute({
 
     const writtenEndpoint = await entrypoints.global.app.writeToDisk()
     hooks.handleWrittenEndpoint(key, writtenEndpoint, false)
-    hooks.subscribeToChanges?.(
+    hooks.subscribeToChanges(
       key,
       false,
       entrypoints.global.app,
@@ -903,7 +903,7 @@ export async function handlePagesErrorRoute({
 
     const writtenEndpoint = await entrypoints.global.document.writeToDisk()
     hooks.handleWrittenEndpoint(key, writtenEndpoint, false)
-    hooks.subscribeToChanges?.(
+    hooks.subscribeToChanges(
       key,
       false,
       entrypoints.global.document,
@@ -929,7 +929,7 @@ export async function handlePagesErrorRoute({
 
     const writtenEndpoint = await entrypoints.global.error.writeToDisk()
     hooks.handleWrittenEndpoint(key, writtenEndpoint, false)
-    hooks.subscribeToChanges?.(
+    hooks.subscribeToChanges(
       key,
       false,
       entrypoints.global.error,