refactor: handle onlyHashChange logic sooner (#70569)

When only the hash fragment changes there's no reason to read anything from the server. The `onlyHashChange` logic was spilling out into places that shouldn't really care about it (like ppr-navigations) when really we just want to update some state flags and exit early. --------- Co-authored-by: JJ Kasper <jj@jjsweb.site>
vercel · Sep 27, 2024 · e701b37 · e701b37
1 parent 9ea0ff9
commit e701b37
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 23 deletions.
diff --git a/packages/next/src/client/components/router-reducer/ppr-navigations.ts b/packages/next/src/client/components/router-reducer/ppr-navigations.ts
@@ -67,8 +67,7 @@ export function updateCacheNodeOnNavigation(
   oldRouterState: FlightRouterState,
   newRouterState: FlightRouterState,
   prefetchData: CacheNodeSeedData,
-  prefetchHead: React.ReactNode,
-  onlyHashChange: boolean
+  prefetchHead: React.ReactNode
 ): Task | null {
   // Diff the old and new trees to reuse the shared layouts.
   const oldRouterStateChildren = oldRouterState[1]
@@ -137,12 +136,7 @@ export function updateCacheNodeOnNavigation(
         : undefined
 
     let taskChild: Task | null
-    if (onlyHashChange && oldRouterStateChild !== undefined) {
-      // If only the hash fragment changed, we can re-use the existing cache.
-      // We spawn a "task" just to keep track of the updated router state; unlike most, it's
-      // already fulfilled and won't be affected by the dynamic response.
-      taskChild = spawnReusedTask(oldRouterStateChild)
-    } else if (isPageSegment) {
+    if (isPageSegment) {
       // This is a leaf segment — a page, not a shared layout. We always apply
       // its data.
       taskChild = spawnPendingTask(
@@ -187,8 +181,7 @@ export function updateCacheNodeOnNavigation(
             oldRouterStateChild,
             newRouterStateChild,
             prefetchDataChild,
-            prefetchHead,
-            onlyHashChange
+            prefetchHead
           )
         } else {
           // The server didn't send any prefetch data for this segment. This

diff --git a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts
@@ -151,6 +151,26 @@ export function navigateReducer(
         return handleExternalUrl(state, mutable, flightData, pendingPush)
       }
 
+      const updatedCanonicalUrl = canonicalUrlOverride
+        ? createHrefFromUrl(canonicalUrlOverride)
+        : href
+
+      const onlyHashChange =
+        !!hash &&
+        state.canonicalUrl.split('#', 1)[0] ===
+          updatedCanonicalUrl.split('#', 1)[0]
+
+      // If only the hash has changed, the server hasn't sent us any new data. We can just update
+      // the mutable properties responsible for URL and scroll handling and return early.
+      if (onlyHashChange) {
+        mutable.onlyHashChange = true
+        mutable.canonicalUrl = updatedCanonicalUrl
+        mutable.shouldScroll = shouldScroll
+        mutable.hashFragment = hash
+        mutable.scrollableSegments = []
+        return handleMutable(state, mutable)
+      }
+
       if (prefetchValues.aliased) {
         const result = handleAliasedPrefetchEntry(
           state,
@@ -169,16 +189,6 @@ export function navigateReducer(
         return result
       }
 
-      const updatedCanonicalUrl = canonicalUrlOverride
-        ? createHrefFromUrl(canonicalUrlOverride)
-        : href
-
-      // Track if the navigation was only an update to the hash fragment
-      mutable.onlyHashChange =
-        !!hash &&
-        state.canonicalUrl.split('#', 1)[0] ===
-          updatedCanonicalUrl.split('#', 1)[0]
-
       let currentTree = state.tree
       let currentCache = state.cache
       let scrollableSegments: FlightSegmentPath[] = []
@@ -237,8 +247,7 @@ export function navigateReducer(
               currentTree,
               treePatch,
               seedData,
-              head,
-              mutable.onlyHashChange
+              head
             )
 
             if (task !== null) {
@@ -305,7 +314,6 @@ export function navigateReducer(
 
             if (
               prefetchValues.status === PrefetchCacheEntryStatus.stale &&
-              !mutable.onlyHashChange &&
               !isFirstRead
             ) {
               // When we have a stale prefetch entry, we only want to re-use the loading state of the route we're navigating to, to support instant loading navigations