Fix instant loading states

packages/next/src/client/components/router-reducer/fill-cache-with-data-property.test.tsx → packages/next/src/client/components/router-reducer/clear-cache-node-data-for-segment-path.test.tsx

@@ -1,18 +1,9 @@
 import React from 'react'
-import type { FetchServerResponseResult } from './fetch-server-response'
-import { fillCacheWithDataProperty } from './fill-cache-with-data-property'
+import { clearCacheNodeDataForSegmentPath } from './clear-cache-node-data-for-segment-path'
 import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'
 
-describe('fillCacheWithDataProperty', () => {
-  it('should add data property', () => {
-    const fetchServerResponseMock: jest.Mock<
-      Promise<FetchServerResponseResult>
-    > = jest.fn(() =>
-      Promise.resolve([
-        /* TODO-APP: replace with actual FlightData */ '',
-        undefined,
-      ])
-    )
+describe('clearCacheNodeDataForSegmentPath', () => {
+  it('should clear the data property', () => {
     const pathname = '/dashboard/settings'
     const segments = pathname.split('/')
 
@@ -80,9 +71,7 @@ describe('fillCacheWithDataProperty', () => {
       ]),
     }
 
-    fillCacheWithDataProperty(cache, existingCache, flightSegmentPath, () =>
-      fetchServerResponseMock()
-    )
+    clearCacheNodeDataForSegmentPath(cache, existingCache, flightSegmentPath)
 
     expect(cache).toMatchInlineSnapshot(`
       {
@@ -121,7 +110,7 @@ describe('fillCacheWithDataProperty', () => {
             },
             "dashboard" => {
               "head": null,
-              "lazyData": Promise {},
+              "lazyData": null,
               "lazyDataResolved": false,
               "loading": null,
               "parallelRoutes": Map {},

packages/next/src/client/components/router-reducer/fill-cache-with-data-property.ts → packages/next/src/client/components/router-reducer/clear-cache-node-data-for-segment-path.ts

@@ -1,16 +1,14 @@
-import type { FetchServerResponseResult } from './fetch-server-response'
 import type { FlightSegmentPath } from '../../../server/app-render/types'
 import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'
 import { createRouterCacheKey } from './create-router-cache-key'
 
 /**
- * Kick off fetch based on the common layout between two routes. Fill cache with data property holding the in-progress fetch.
+ * This will clear the CacheNode data for a particular segment path. This will cause a lazy-fetch in layout router to fill in new data.
  */
-export function fillCacheWithDataProperty(
+export function clearCacheNodeDataForSegmentPath(
   newCache: CacheNode,
   existingCache: CacheNode,
-  flightSegmentPath: FlightSegmentPath,
-  fetchResponse: () => Promise<FetchServerResponseResult>
+  flightSegmentPath: FlightSegmentPath
 ): void {
   const isLastEntry = flightSegmentPath.length <= 2
 
@@ -38,7 +36,7 @@ export function fillCacheWithDataProperty(
       childCacheNode === existingChildCacheNode
     ) {
       childSegmentMap.set(cacheKey, {
-        lazyData: fetchResponse(),
+        lazyData: null,
         rsc: null,
         prefetchRsc: null,
         head: null,
@@ -55,7 +53,7 @@ export function fillCacheWithDataProperty(
     // Start fetch in the place where the existing cache doesn't have the data yet.
     if (!childCacheNode) {
       childSegmentMap.set(cacheKey, {
-        lazyData: fetchResponse(),
+        lazyData: null,
         rsc: null,
         prefetchRsc: null,
         head: null,
@@ -82,10 +80,9 @@ export function fillCacheWithDataProperty(
     childSegmentMap.set(cacheKey, childCacheNode)
   }
 
-  return fillCacheWithDataProperty(
+  return clearCacheNodeDataForSegmentPath(
     childCacheNode,
     existingChildCacheNode,
-    flightSegmentPath.slice(2),
-    fetchResponse
+    flightSegmentPath.slice(2)
   )
 }

packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts

@@ -79,12 +79,7 @@ export function getOrCreatePrefetchCacheEntry({
       existingCacheEntry.kind !== PrefetchKind.FULL &&
       kind === PrefetchKind.FULL
 
-    // If the cache entry isn't reusable, rather than returning it, we want to create a new entry.
-    const hasReusablePrefetch =
-      existingCacheEntry.status === PrefetchCacheEntryStatus.reusable ||
-      existingCacheEntry.status === PrefetchCacheEntryStatus.fresh
-
-    if (switchedToFullPrefetch || !hasReusablePrefetch) {
+    if (switchedToFullPrefetch) {
       return createLazyPrefetchEntry({
         tree,
         url,

packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts

@@ -9,11 +9,12 @@ import { invalidateCacheBelowFlightSegmentPath } from '../invalidate-cache-below
 import { applyRouterStatePatchToTreeSkipDefault } from '../apply-router-state-patch-to-tree'
 import { shouldHardNavigate } from '../should-hard-navigate'
 import { isNavigatingToNewRootLayout } from '../is-navigating-to-new-root-layout'
-import type {
-  Mutable,
-  NavigateAction,
-  ReadonlyReducerState,
-  ReducerState,
+import {
+  PrefetchCacheEntryStatus,
+  type Mutable,
+  type NavigateAction,
+  type ReadonlyReducerState,
+  type ReducerState,
 } from '../router-reducer-types'
 import { handleMutable } from '../handle-mutable'
 import { applyFlightData } from '../apply-flight-data'
@@ -28,6 +29,7 @@ import {
   getOrCreatePrefetchCacheEntry,
   prunePrefetchCache,
 } from '../prefetch-cache-utils'
+import { clearCacheNodeDataForSegmentPath } from '../clear-cache-node-data-for-segment-path'
 
 export function handleExternalUrl(
   state: ReadonlyReducerState,
@@ -69,6 +71,32 @@ function generateSegmentsFromPatch(
   return segments
 }
 
+function triggerLazyFetchForLeafSegments(
+  newCache: CacheNode,
+  currentCache: CacheNode,
+  flightSegmentPath: FlightSegmentPath,
+  treePatch: FlightRouterState
+) {
+  let appliedPatch = false
+
+  newCache.rsc = currentCache.rsc
+  newCache.prefetchRsc = currentCache.prefetchRsc
+  newCache.loading = currentCache.loading
+  newCache.parallelRoutes = new Map(currentCache.parallelRoutes)
+
+  const segmentPathsToFill = generateSegmentsFromPatch(treePatch).map(
+    (segment) => [...flightSegmentPath, ...segment]
+  )
+
+  for (const segmentPaths of segmentPathsToFill) {
+    clearCacheNodeDataForSegmentPath(newCache, currentCache, segmentPaths)
+
+    appliedPatch = true
+  }
+
+  return appliedPatch
+}
+
 // These implementations are expected to diverge significantly, so I've forked
 // the entire function. The one that's disabled should be dead code eliminated
 // because the check here is statically inlined at build time.
@@ -109,10 +137,12 @@ function navigateReducer_noPPR(
 
   return data.then(
     ([flightData, canonicalUrlOverride]) => {
+      let isFirstRead = false
       // we only want to mark this once
       if (!prefetchValues.lastUsedTime) {
         // important: we should only mark the cache node as dirty after we unsuspend from the call above
         prefetchValues.lastUsedTime = Date.now()
+        isFirstRead = true
       }
 
       // Handle case when navigating to page in `pages` from `app`
@@ -159,12 +189,33 @@ function navigateReducer_noPPR(
           }
 
           const cache: CacheNode = createEmptyCacheNode()
-          let applied = applyFlightData(
-            currentCache,
-            cache,
-            flightDataPath,
-            prefetchValues
-          )
+          let applied = false
+
+          if (
+            prefetchValues.status === PrefetchCacheEntryStatus.stale &&
+            !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
+            // this will trigger a lazy fetch for the actual page data by nulling the `rsc` and `prefetchRsc` values for page data,
+            // while copying over the `loading` for the segment that contains the page data.
+            // We only do this on subsequent reads, as otherwise there'd be no loading data to re-use.
+            applied = triggerLazyFetchForLeafSegments(
+              cache,
+              currentCache,
+              flightSegmentPath,
+              treePatch
+            )
+            // since we re-used the stale cache's loading state & refreshed the data,
+            // update the `lastUsedTime` so that it can continue to be re-used for the next 30s
+            prefetchValues.lastUsedTime = Date.now()
+          } else {
+            applied = applyFlightData(
+              currentCache,
+              cache,
+              flightDataPath,
+              prefetchValues
+            )
+          }
 
           const hardNavigate = shouldHardNavigate(
             // TODO-APP: remove ''
@@ -252,10 +303,12 @@ function navigateReducer_PPR(
 
   return data.then(
     ([flightData, canonicalUrlOverride, _postponed]) => {
+      let isFirstRead = false
       // we only want to mark this once
       if (!prefetchValues.lastUsedTime) {
         // important: we should only mark the cache node as dirty after we unsuspend from the call above
         prefetchValues.lastUsedTime = Date.now()
+        isFirstRead = true
       }
 
       // Handle case when navigating to page in `pages` from `app`
@@ -379,12 +432,33 @@ function navigateReducer_PPR(
             // tree. Or in the meantime we could factor it out into a
             // separate function.
             const cache: CacheNode = createEmptyCacheNode()
-            let applied = applyFlightData(
-              currentCache,
-              cache,
-              flightDataPath,
-              prefetchValues
-            )
+            let applied = false
+
+            if (
+              prefetchValues.status === PrefetchCacheEntryStatus.stale &&
+              !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
+              // this will trigger a lazy fetch for the actual page data by nulling the `rsc` and `prefetchRsc` values for page data,
+              // while copying over the `loading` for the segment that contains the page data.
+              // We only do this on subsequent reads, as otherwise there'd be no loading data to re-use.
+              applied = triggerLazyFetchForLeafSegments(
+                cache,
+                currentCache,
+                flightSegmentPath,
+                treePatch
+              )
+              // since we re-used the stale cache's loading state & refreshed the data,
+              // update the `lastUsedTime` so that it can continue to be re-used for the next 30s
+              prefetchValues.lastUsedTime = Date.now()
+            } else {
+              applied = applyFlightData(
+                currentCache,
+                cache,
+                flightDataPath,
+                prefetchValues
+              )
+            }
 
             const hardNavigate = shouldHardNavigate(
               // TODO-APP: remove ''

test/e2e/app-dir/app-client-cache/client-cache.test.ts

@@ -373,9 +373,7 @@ createNextDescribe(
           expect(newNumber).toBe(initialNumber)
         })
 
-        // TODO: Rather than reusing parts of a stale prefetch cache entry to make this work,
-        // we should be able to copy over the existing loading from a previous cache node on navigation.
-        it.skip('should refetch below the fold after 30 seconds', async () => {
+        it('should refetch below the fold after 30 seconds', async () => {
           const randomLoadingNumber = await browser
             .elementByCss('[href="/1?timeout=1000"]')
             .click()

test/ppr-tests-manifest.json

@@ -12,7 +12,6 @@
     "test/e2e/app-dir/app-client-cache/client-cache.test.ts": {
       "failed": [
         "app dir client cache semantics prefetch={undefined} - default should re-use the full cache for only 30 seconds",
-        "app dir client cache semantics prefetch={undefined} - default should refetch below the fold after 30 seconds",
         "app dir client cache semantics prefetch={undefined} - default should renew the 30s cache once the data is revalidated",
         "app dir client cache semantics prefetch={undefined} - default should refetch the full page after 5 mins"
       ]