Skip to content

Commit cb3adac

Browse files
acdliteacdlite
committed
Pass loading boundary as part of RSC data
Removes the `loading` as a separate field in the server response payload. Instead, it is passed as part of the component data (the `rsc` field). This simplifies much of the logic on the client because, logically, they were already coupled. Unifying the fields removes the possibility that they'll get out of sync due to some implementation mistake.
1 parent 6795538 commit cb3adac

File tree

11 files changed

+130
-133
lines changed

11 files changed

+130
-133
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,7 @@ function Router({
424424
parentCacheNode: cache,
425425
parentSegmentPath: null,
426426
parentParams: {},
427+
parentLoadingData: null,
427428
// This is the <Activity> "name" that shows up in the Suspense DevTools.
428429
// It represents the root of the app.
429430
debugNameContext: '/',

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

Lines changed: 71 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,10 @@ function InnerLayoutRouter({
378378
parentCacheNode: cacheNode,
379379
parentSegmentPath: segmentPath,
380380
parentParams: params,
381+
// This is always set to null as we enter a child segment. It's
382+
// populated by LoadingBoundaryProvider the next time we reach a
383+
// loading boundary.
384+
parentLoadingData: null,
381385
debugNameContext: debugNameContext,
382386

383387
// TODO-APP: overriding of url for parallel routes
@@ -392,6 +396,51 @@ function InnerLayoutRouter({
392396
return children
393397
}
394398

399+
export function LoadingBoundaryProvider({
400+
loading,
401+
children,
402+
}: {
403+
loading: LoadingModuleData
404+
children: React.ReactNode
405+
}) {
406+
// Provides the data needed to render a loading.tsx boundary, via context.
407+
//
408+
// loading.tsx creates a Suspense boundary around each of a layout's child
409+
// slots. (Might be bit confusing to think about the data flow, but: if
410+
// loading.tsx and layout.tsx are in the same directory, they are assigned
411+
// to the same CacheNode.)
412+
//
413+
// This provider component does not render the Suspense boundary directly;
414+
// that's handled by LoadingBoundary.
415+
//
416+
// TODO: For simplicity, we should combine this provider with LoadingBoundary
417+
// and render the Suspense boundary directly. The only real benefit of doing
418+
// it separately is so that when there are multiple parallel routes, we only
419+
// send the boundary data once, rather than once per child. But that's a
420+
// negligible benefit and can be achieved via caching instead.
421+
const parentContext = use(LayoutRouterContext)
422+
if (parentContext === null) {
423+
return children
424+
}
425+
// All values except for parentLoadingData are the same as the parent context.
426+
return (
427+
<LayoutRouterContext.Provider
428+
value={{
429+
parentTree: parentContext.parentTree,
430+
parentCacheNode: parentContext.parentCacheNode,
431+
parentSegmentPath: parentContext.parentSegmentPath,
432+
parentParams: parentContext.parentParams,
433+
parentLoadingData: loading,
434+
debugNameContext: parentContext.debugNameContext,
435+
url: parentContext.url,
436+
isActive: parentContext.isActive,
437+
}}
438+
>
439+
{children}
440+
</LayoutRouterContext.Provider>
441+
)
442+
}
443+
395444
/**
396445
* Renders suspense boundary with the provided "loading" property as the fallback.
397446
* If no loading property is provided it renders the children without a suspense boundary.
@@ -402,33 +451,18 @@ function LoadingBoundary({
402451
children,
403452
}: {
404453
name: ActivityProps['name']
405-
loading: LoadingModuleData | Promise<LoadingModuleData>
454+
loading: LoadingModuleData | null
406455
children: React.ReactNode
407456
}): JSX.Element {
408-
// If loading is a promise, unwrap it. This happens in cases where we haven't
409-
// yet received the loading data from the server — which includes whether or
410-
// not this layout has a loading component at all.
411-
//
412-
// It's OK to suspend here instead of inside the fallback because this
413-
// promise will resolve simultaneously with the data for the segment itself.
414-
// So it will never suspend for longer than it would have if we didn't use
415-
// a Suspense fallback at all.
416-
let loadingModuleData
417-
if (
418-
typeof loading === 'object' &&
419-
loading !== null &&
420-
typeof (loading as any).then === 'function'
421-
) {
422-
const promiseForLoading = loading as Promise<LoadingModuleData>
423-
loadingModuleData = use(promiseForLoading)
424-
} else {
425-
loadingModuleData = loading as LoadingModuleData
426-
}
427-
428-
if (loadingModuleData) {
429-
const loadingRsc = loadingModuleData[0]
430-
const loadingStyles = loadingModuleData[1]
431-
const loadingScripts = loadingModuleData[2]
457+
// TODO: For LoadingBoundary, and the other built-in boundary types, don't
458+
// wrap in an extra function component if no user-defined boundary is
459+
// provided. In other words, inline this conditional wrapping logic into
460+
// the parent component. More efficient and keeps unnecessary junk out of
461+
// the component stack.
462+
if (loading !== null) {
463+
const loadingRsc = loading[0]
464+
const loadingStyles = loading[1]
465+
const loadingScripts = loading[2]
432466
return (
433467
<Suspense
434468
name={name}
@@ -487,6 +521,7 @@ export default function OuterLayoutRouter({
487521
parentCacheNode,
488522
parentSegmentPath,
489523
parentParams,
524+
parentLoadingData,
490525
url,
491526
isActive,
492527
debugNameContext,
@@ -616,15 +651,6 @@ export default function OuterLayoutRouter({
616651
const isVirtual = debugName === undefined
617652
const debugNameToDisplay = isVirtual ? undefined : debugNameContext
618653

619-
// TODO: The loading module data for a segment is stored on the parent, then
620-
// applied to each of that parent segment's parallel route slots. In the
621-
// simple case where there's only one parallel route (the `children` slot),
622-
// this is no different from if the loading module data where stored on the
623-
// child directly. But I'm not sure this actually makes sense when there are
624-
// multiple parallel routes. It's not a huge issue because you always have
625-
// the option to define a narrower loading boundary for a particular slot. But
626-
// this sort of smells like an implementation accident to me.
627-
const loadingModuleData = parentCacheNode.loading
628654
let child = (
629655
<TemplateContext.Provider
630656
key={stateKey}
@@ -637,7 +663,17 @@ export default function OuterLayoutRouter({
637663
>
638664
<LoadingBoundary
639665
name={debugNameToDisplay}
640-
loading={loadingModuleData}
666+
// TODO: The loading module data for a segment is stored on the
667+
// parent, then applied to each of that parent segment's
668+
// parallel route slots. In the simple case where there's only
669+
// one parallel route (the `children` slot), this is no
670+
// different from if the loading module data where stored on the
671+
// child directly. But I'm not sure this actually makes sense
672+
// when there are multiple parallel routes. It's not a huge
673+
// issue because you always have the option to define a narrower
674+
// loading boundary for a particular slot. But this sort of
675+
// smells like an implementation accident to me.
676+
loading={parentLoadingData}
641677
>
642678
<HTTPAccessFallbackBoundary
643679
notFound={notFound}

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

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,7 @@ import type {
88
ChildSegmentMap,
99
CacheNode,
1010
} from '../../../shared/lib/app-router-types'
11-
import type {
12-
HeadData,
13-
LoadingModuleData,
14-
} from '../../../shared/lib/app-router-types'
11+
import type { HeadData } from '../../../shared/lib/app-router-types'
1512
import {
1613
PAGE_SEGMENT_KEY,
1714
DEFAULT_SEGMENT_KEY,
@@ -382,12 +379,10 @@ function updateCacheNodeOnNavigation(
382379
needsDynamicRequest = false
383380
} else {
384381
const seedRsc = seedData !== null ? seedData[0] : null
385-
const seedLoading = seedData !== null ? seedData[2] : null
386382
const result = createCacheNodeForSegment(
387383
navigatedAt,
388384
newRouteTree,
389385
seedRsc,
390-
seedLoading,
391386
newMetadataVaryPath,
392387
seedHead,
393388
newParallelRoutes,
@@ -719,12 +714,10 @@ function createCacheNodeOnNavigation(
719714
needsDynamicRequest = false
720715
} else {
721716
const seedRsc = seedData !== null ? seedData[0] : null
722-
const seedLoading = seedData !== null ? seedData[2] : null
723717
const result = createCacheNodeForSegment(
724718
navigatedAt,
725719
newRouteTree,
726720
seedRsc,
727-
seedLoading,
728721
newMetadataVaryPath,
729722
seedHead,
730723
newParallelRoutes,
@@ -992,7 +985,6 @@ function reuseDynamicCacheNode(
992985
dropPrefetchRsc ? null : existingCacheNode.prefetchRsc,
993986
existingCacheNode.head,
994987
dropPrefetchRsc ? null : existingCacheNode.prefetchHead,
995-
existingCacheNode.loading,
996988
parallelRoutes,
997989
existingCacheNode.navigatedAt
998990
)
@@ -1002,7 +994,6 @@ function createCacheNodeForSegment(
1002994
now: number,
1003995
tree: RouteTree,
1004996
seedRsc: React.ReactNode | null,
1005-
seedLoading: LoadingModuleData | Promise<LoadingModuleData> | null,
1006997
metadataVaryPath: PageVaryPath | null,
1007998
seedHead: HeadData | null,
1008999
newParallelRoutes: Map<string, ChildSegmentMap>,
@@ -1044,7 +1035,6 @@ function createCacheNodeForSegment(
10441035
null,
10451036
isPage ? seedHead : null,
10461037
null,
1047-
seedLoading,
10481038
newParallelRoutes,
10491039
now
10501040
),
@@ -1054,8 +1044,6 @@ function createCacheNodeForSegment(
10541044

10551045
let cachedRsc: React.ReactNode | null = null
10561046
let isCachedRscPartial: boolean = true
1057-
let cachedLoading: LoadingModuleData | Promise<LoadingModuleData> | null =
1058-
null
10591047

10601048
const segmentEntry = readSegmentCacheEntry(now, tree.varyPath)
10611049
if (segmentEntry !== null) {
@@ -1064,7 +1052,6 @@ function createCacheNodeForSegment(
10641052
// Happy path: a cache hit
10651053
cachedRsc = segmentEntry.rsc
10661054
isCachedRscPartial = segmentEntry.isPartial
1067-
cachedLoading = segmentEntry.loading
10681055
break
10691056
}
10701057
case EntryStatus.Pending: {
@@ -1086,9 +1073,6 @@ function createCacheNodeForSegment(
10861073
// we can assume the response will be full. This field is set to `false`
10871074
// for such segments.
10881075
isCachedRscPartial = segmentEntry.isPartial
1089-
cachedLoading = promiseForFulfilledEntry.then((entry) =>
1090-
entry !== null ? entry.loading : null
1091-
)
10921076
break
10931077
}
10941078
case EntryStatus.Empty:
@@ -1114,7 +1098,6 @@ function createCacheNodeForSegment(
11141098
// means the data failed to load; the LayoutRouter will suspend indefinitely
11151099
// until the router updates again (refer to finishNavigationTask).
11161100
let rsc: React.ReactNode | null
1117-
let loading: LoadingModuleData | Promise<LoadingModuleData> | null
11181101
let doesSegmentNeedDynamicRequest: boolean
11191102

11201103
if (seedRsc !== null) {
@@ -1124,15 +1107,13 @@ function createCacheNodeForSegment(
11241107
// partial cached state in the meantime.
11251108
prefetchRsc = cachedRsc
11261109
rsc = seedRsc
1127-
loading = seedLoading
11281110
} else {
11291111
// We already have a completely cached segment. Ignore the seed data,
11301112
// which may still be streaming in. This shouldn't happen in the normal
11311113
// case because the client will inform the server which segments are
11321114
// already fully cached, and the server will skip rendering them.
11331115
prefetchRsc = null
11341116
rsc = cachedRsc
1135-
loading = cachedLoading
11361117
}
11371118
doesSegmentNeedDynamicRequest = false
11381119
} else {
@@ -1145,16 +1126,10 @@ function createCacheNodeForSegment(
11451126
// data arrives from the server.
11461127
prefetchRsc = cachedRsc
11471128
rsc = createDeferredRsc()
1148-
// TODO: Technically, a loading boundary could contain dynamic data. We
1149-
// should have separate `loading` and `prefetchLoading` fields to handle
1150-
// this, like we do for the segment data and head.
1151-
const isCacheHit = cachedRsc !== null
1152-
loading = isCacheHit ? cachedLoading : createDeferredRsc()
11531129
} else {
11541130
// The data is fully cached.
11551131
prefetchRsc = null
11561132
rsc = cachedRsc
1157-
loading = cachedLoading
11581133
}
11591134
doesSegmentNeedDynamicRequest = isCachedRscPartial
11601135
}
@@ -1227,7 +1202,6 @@ function createCacheNodeForSegment(
12271202
prefetchRsc,
12281203
head,
12291204
prefetchHead,
1230-
loading,
12311205
newParallelRoutes,
12321206
now
12331207
),
@@ -1244,7 +1218,6 @@ function createCacheNode(
12441218
prefetchRsc: React.ReactNode | null,
12451219
head: React.ReactNode | null,
12461220
prefetchHead: HeadData | null,
1247-
loading: LoadingModuleData | Promise<LoadingModuleData> | null,
12481221
parallelRoutes: Map<string, ChildSegmentMap>,
12491222
navigatedAt: number
12501223
): CacheNode {
@@ -1253,7 +1226,6 @@ function createCacheNode(
12531226
prefetchRsc,
12541227
head,
12551228
prefetchHead,
1256-
loading,
12571229
parallelRoutes,
12581230
navigatedAt,
12591231
}
@@ -1702,14 +1674,6 @@ function finishPendingCacheNode(
17021674
// been populated by a different navigation. We must not overwrite it.
17031675
}
17041676

1705-
// If we navigated without a prefetch, then `loading` will be a deferred promise too.
1706-
// Fulfill it using the dynamic response so that we can display the loading boundary.
1707-
const loading = cacheNode.loading
1708-
if (isDeferredRsc(loading)) {
1709-
const dynamicLoading = dynamicData[2]
1710-
loading.resolve(dynamicLoading, debugInfo)
1711-
}
1712-
17131677
// Check if this is a leaf segment. If so, it will have a `head` property with
17141678
// a pending promise that needs to be resolved with the dynamic head from
17151679
// the server.
@@ -1796,11 +1760,6 @@ function abortPendingCacheNode(
17961760
}
17971761
}
17981762

1799-
const loading = cacheNode.loading
1800-
if (isDeferredRsc(loading)) {
1801-
loading.resolve(null, debugInfo)
1802-
}
1803-
18041763
// Check if this is a leaf segment. If so, it will have a `head` property with
18051764
// a pending promise that needs to be resolved. If an error was provided, we
18061765
// 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‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ describe('findHeadInCache', () => {
3434
prefetchRsc: null,
3535
head: null,
3636
prefetchHead: null,
37-
loading: null,
3837
parallelRoutes: new Map([
3938
[
4039
'children',
@@ -47,7 +46,6 @@ describe('findHeadInCache', () => {
4746
prefetchRsc: null,
4847
head: null,
4948
prefetchHead: null,
50-
loading: null,
5149
parallelRoutes: new Map([
5250
[
5351
'children',
@@ -58,7 +56,6 @@ describe('findHeadInCache', () => {
5856
navigatedAt,
5957
head: null,
6058
prefetchHead: null,
61-
loading: null,
6259
parallelRoutes: new Map([
6360
[
6461
'children',
@@ -70,7 +67,6 @@ describe('findHeadInCache', () => {
7067
rsc: null,
7168
prefetchRsc: null,
7269
prefetchHead: null,
73-
loading: null,
7470
parallelRoutes: new Map(),
7571
head: null,
7672
},

0 commit comments

Comments
 (0)