Pass loading boundary as prop to LayoutRouter

Removes the `loading` as a separate field in the server response
payload. Instead, it is passed as a prop to LayoutRouter, similar to
how the other boundary types (NotFound, et al) work.

This simplifies much of the logic on the client because we no longer
need to track `loading` separately from the rest of the segment data.

Author: acdlite

packages/next/src/client/components/app-router.tsx

@@ -424,6 +424,7 @@ function Router({
       parentCacheNode: cache,
       parentSegmentPath: null,
       parentParams: {},
+      parentLoadingData: null,
       // This is the <Activity> "name" that shows up in the Suspense DevTools.
       // It represents the root of the app.
       debugNameContext: '/',

packages/next/src/client/components/layout-router.tsx

@@ -277,6 +277,7 @@ function InnerLayoutRouter({
   debugNameContext,
   cacheNode: maybeCacheNode,
   params,
+  loading,
   url,
   isActive,
 }: {
@@ -285,6 +286,7 @@ function InnerLayoutRouter({
   debugNameContext: string
   cacheNode: CacheNode | null
   params: Params
+  loading: LoadingModuleData | null
   url: string
   isActive: boolean
 }) {
@@ -378,6 +380,7 @@ function InnerLayoutRouter({
         parentCacheNode: cacheNode,
         parentSegmentPath: segmentPath,
         parentParams: params,
+        parentLoadingData: loading,
         debugNameContext: debugNameContext,
 
         // TODO-APP: overriding of url for parallel routes
@@ -402,33 +405,18 @@ function LoadingBoundary({
   children,
 }: {
   name: ActivityProps['name']
-  loading: LoadingModuleData | Promise<LoadingModuleData>
+  loading: LoadingModuleData | null
   children: React.ReactNode
 }): JSX.Element {
-  // If loading is a promise, unwrap it. This happens in cases where we haven't
-  // yet received the loading data from the server — which includes whether or
-  // not this layout has a loading component at all.
-  //
-  // It's OK to suspend here instead of inside the fallback because this
-  // promise will resolve simultaneously with the data for the segment itself.
-  // So it will never suspend for longer than it would have if we didn't use
-  // a Suspense fallback at all.
-  let loadingModuleData
-  if (
-    typeof loading === 'object' &&
-    loading !== null &&
-    typeof (loading as any).then === 'function'
-  ) {
-    const promiseForLoading = loading as Promise<LoadingModuleData>
-    loadingModuleData = use(promiseForLoading)
-  } else {
-    loadingModuleData = loading as LoadingModuleData
-  }
-
-  if (loadingModuleData) {
-    const loadingRsc = loadingModuleData[0]
-    const loadingStyles = loadingModuleData[1]
-    const loadingScripts = loadingModuleData[2]
+  // TODO: For LoadingBoundary, and the other built-in boundary types, don't
+  // wrap in an extra function component if no user-defined boundary is
+  // provided. In other words, inline this conditional wrapping logic into
+  // the parent component. More efficient and keeps unnecessary junk out of
+  // the component stack.
+  if (loading !== null) {
+    const loadingRsc = loading[0]
+    const loadingStyles = loading[1]
+    const loadingScripts = loading[2]
     return (
       <Suspense
         name={name}
@@ -460,6 +448,7 @@ export default function OuterLayoutRouter({
   templateStyles,
   templateScripts,
   template,
+  loading,
   notFound,
   forbidden,
   unauthorized,
@@ -472,6 +461,7 @@ export default function OuterLayoutRouter({
   templateStyles: React.ReactNode | undefined
   templateScripts: React.ReactNode | undefined
   template: React.ReactNode
+  loading: LoadingModuleData | null
   notFound: React.ReactNode | undefined
   forbidden: React.ReactNode | undefined
   unauthorized: React.ReactNode | undefined
@@ -487,6 +477,7 @@ export default function OuterLayoutRouter({
     parentCacheNode,
     parentSegmentPath,
     parentParams,
+    parentLoadingData,
     url,
     isActive,
     debugNameContext,
@@ -616,15 +607,6 @@ export default function OuterLayoutRouter({
     const isVirtual = debugName === undefined
     const debugNameToDisplay = isVirtual ? undefined : debugNameContext
 
-    // TODO: The loading module data for a segment is stored on the parent, then
-    // applied to each of that parent segment's parallel route slots. In the
-    // simple case where there's only one parallel route (the `children` slot),
-    // this is no different from if the loading module data where stored on the
-    // child directly. But I'm not sure this actually makes sense when there are
-    // multiple parallel routes. It's not a huge issue because you always have
-    // the option to define a narrower loading boundary for a particular slot. But
-    // this sort of smells like an implementation accident to me.
-    const loadingModuleData = parentCacheNode.loading
     let child = (
       <TemplateContext.Provider
         key={stateKey}
@@ -637,7 +619,17 @@ export default function OuterLayoutRouter({
             >
               <LoadingBoundary
                 name={debugNameToDisplay}
-                loading={loadingModuleData}
+                // TODO: The loading module data for a segment is stored on the
+                // parent, then applied to each of that parent segment's
+                // parallel route slots. In the simple case where there's only
+                // one parallel route (the `children` slot), this is no
+                // different from if the loading module data where stored on the
+                // child directly. But I'm not sure this actually makes sense
+                // when there are multiple parallel routes. It's not a huge
+                // issue because you always have the option to define a narrower
+                // loading boundary for a particular slot. But this sort of
+                // smells like an implementation accident to me.
+                loading={parentLoadingData}
               >
                 <HTTPAccessFallbackBoundary
                   notFound={notFound}
@@ -649,6 +641,7 @@ export default function OuterLayoutRouter({
                       url={url}
                       tree={tree}
                       params={params}
+                      loading={loading}
                       cacheNode={cacheNode}
                       segmentPath={segmentPath}
                       debugNameContext={childDebugNameContext}

packages/next/src/client/components/router-reducer/ppr-navigations.ts

@@ -8,10 +8,7 @@ import type {
   ChildSegmentMap,
   CacheNode,
 } from '../../../shared/lib/app-router-types'
-import type {
-  HeadData,
-  LoadingModuleData,
-} from '../../../shared/lib/app-router-types'
+import type { HeadData } from '../../../shared/lib/app-router-types'
 import {
   PAGE_SEGMENT_KEY,
   DEFAULT_SEGMENT_KEY,
@@ -382,12 +379,10 @@ function updateCacheNodeOnNavigation(
     needsDynamicRequest = false
   } else {
     const seedRsc = seedData !== null ? seedData[0] : null
-    const seedLoading = seedData !== null ? seedData[2] : null
     const result = createCacheNodeForSegment(
       navigatedAt,
       newRouteTree,
       seedRsc,
-      seedLoading,
       newMetadataVaryPath,
       seedHead,
       newParallelRoutes,
@@ -719,12 +714,10 @@ function createCacheNodeOnNavigation(
     needsDynamicRequest = false
   } else {
     const seedRsc = seedData !== null ? seedData[0] : null
-    const seedLoading = seedData !== null ? seedData[2] : null
     const result = createCacheNodeForSegment(
       navigatedAt,
       newRouteTree,
       seedRsc,
-      seedLoading,
       newMetadataVaryPath,
       seedHead,
       newParallelRoutes,
@@ -992,7 +985,6 @@ function reuseDynamicCacheNode(
     dropPrefetchRsc ? null : existingCacheNode.prefetchRsc,
     existingCacheNode.head,
     dropPrefetchRsc ? null : existingCacheNode.prefetchHead,
-    existingCacheNode.loading,
     parallelRoutes,
     existingCacheNode.navigatedAt
   )
@@ -1002,7 +994,6 @@ function createCacheNodeForSegment(
   now: number,
   tree: RouteTree,
   seedRsc: React.ReactNode | null,
-  seedLoading: LoadingModuleData | Promise<LoadingModuleData> | null,
   metadataVaryPath: PageVaryPath | null,
   seedHead: HeadData | null,
   newParallelRoutes: Map<string, ChildSegmentMap>,
@@ -1044,7 +1035,6 @@ function createCacheNodeForSegment(
         null,
         isPage ? seedHead : null,
         null,
-        seedLoading,
         newParallelRoutes,
         now
       ),
@@ -1054,8 +1044,6 @@ function createCacheNodeForSegment(
 
   let cachedRsc: React.ReactNode | null = null
   let isCachedRscPartial: boolean = true
-  let cachedLoading: LoadingModuleData | Promise<LoadingModuleData> | null =
-    null
 
   const segmentEntry = readSegmentCacheEntry(now, tree.varyPath)
   if (segmentEntry !== null) {
@@ -1064,7 +1052,6 @@ function createCacheNodeForSegment(
         // Happy path: a cache hit
         cachedRsc = segmentEntry.rsc
         isCachedRscPartial = segmentEntry.isPartial
-        cachedLoading = segmentEntry.loading
         break
       }
       case EntryStatus.Pending: {
@@ -1086,9 +1073,6 @@ function createCacheNodeForSegment(
         // we can assume the response will be full. This field is set to `false`
         // for such segments.
         isCachedRscPartial = segmentEntry.isPartial
-        cachedLoading = promiseForFulfilledEntry.then((entry) =>
-          entry !== null ? entry.loading : null
-        )
         break
       }
       case EntryStatus.Empty:
@@ -1114,7 +1098,6 @@ function createCacheNodeForSegment(
   // means the data failed to load; the LayoutRouter will suspend indefinitely
   // until the router updates again (refer to finishNavigationTask).
   let rsc: React.ReactNode | null
-  let loading: LoadingModuleData | Promise<LoadingModuleData> | null
   let doesSegmentNeedDynamicRequest: boolean
 
   if (seedRsc !== null) {
@@ -1124,15 +1107,13 @@ function createCacheNodeForSegment(
       // partial cached state in the meantime.
       prefetchRsc = cachedRsc
       rsc = seedRsc
-      loading = seedLoading
     } else {
       // We already have a completely cached segment. Ignore the seed data,
       // which may still be streaming in. This shouldn't happen in the normal
       // case because the client will inform the server which segments are
       // already fully cached, and the server will skip rendering them.
       prefetchRsc = null
       rsc = cachedRsc
-      loading = cachedLoading
     }
     doesSegmentNeedDynamicRequest = false
   } else {
@@ -1145,16 +1126,10 @@ function createCacheNodeForSegment(
       // data arrives from the server.
       prefetchRsc = cachedRsc
       rsc = createDeferredRsc()
-      // TODO: Technically, a loading boundary could contain dynamic data. We
-      // should have separate `loading` and `prefetchLoading` fields to handle
-      // this, like we do for the segment data and head.
-      const isCacheHit = cachedRsc !== null
-      loading = isCacheHit ? cachedLoading : createDeferredRsc()
     } else {
       // The data is fully cached.
       prefetchRsc = null
       rsc = cachedRsc
-      loading = cachedLoading
     }
     doesSegmentNeedDynamicRequest = isCachedRscPartial
   }
@@ -1227,7 +1202,6 @@ function createCacheNodeForSegment(
       prefetchRsc,
       head,
       prefetchHead,
-      loading,
       newParallelRoutes,
       now
     ),
@@ -1244,7 +1218,6 @@ function createCacheNode(
   prefetchRsc: React.ReactNode | null,
   head: React.ReactNode | null,
   prefetchHead: HeadData | null,
-  loading: LoadingModuleData | Promise<LoadingModuleData> | null,
   parallelRoutes: Map<string, ChildSegmentMap>,
   navigatedAt: number
 ): CacheNode {
@@ -1253,7 +1226,6 @@ function createCacheNode(
     prefetchRsc,
     head,
     prefetchHead,
-    loading,
     parallelRoutes,
     navigatedAt,
   }
@@ -1702,14 +1674,6 @@ function finishPendingCacheNode(
     // been populated by a different navigation. We must not overwrite it.
   }
 
-  // If we navigated without a prefetch, then `loading` will be a deferred promise too.
-  // Fulfill it using the dynamic response so that we can display the loading boundary.
-  const loading = cacheNode.loading
-  if (isDeferredRsc(loading)) {
-    const dynamicLoading = dynamicData[2]
-    loading.resolve(dynamicLoading, debugInfo)
-  }
-
   // Check if this is a leaf segment. If so, it will have a `head` property with
   // a pending promise that needs to be resolved with the dynamic head from
   // the server.
@@ -1796,11 +1760,6 @@ function abortPendingCacheNode(
     }
   }
 
-  const loading = cacheNode.loading
-  if (isDeferredRsc(loading)) {
-    loading.resolve(null, debugInfo)
-  }
-
   // Check if this is a leaf segment. If so, it will have a `head` property with
   // a pending promise that needs to be resolved. If an error was provided, we
   // will not resolve it with an error, since this is rendered at the root of

packages/next/src/client/components/router-reducer/reducers/find-head-in-cache.test.tsx

@@ -34,7 +34,6 @@ describe('findHeadInCache', () => {
       prefetchRsc: null,
       head: null,
       prefetchHead: null,
-      loading: null,
       parallelRoutes: new Map([
         [
           'children',
@@ -47,7 +46,6 @@ describe('findHeadInCache', () => {
                 prefetchRsc: null,
                 head: null,
                 prefetchHead: null,
-                loading: null,
                 parallelRoutes: new Map([
                   [
                     'children',
@@ -58,7 +56,6 @@ describe('findHeadInCache', () => {
                           navigatedAt,
                           head: null,
                           prefetchHead: null,
-                          loading: null,
                           parallelRoutes: new Map([
                             [
                               'children',
@@ -70,7 +67,6 @@ describe('findHeadInCache', () => {
                                     rsc: null,
                                     prefetchRsc: null,
                                     prefetchHead: null,
-                                    loading: null,
                                     parallelRoutes: new Map(),
                                     head: null,
                                   },