Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow CacheNode.loading to be a promise #72872

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 29 additions & 18 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import type {
ChildSegmentMap,
LazyCacheNode,
LoadingModuleData,
} from '../../shared/lib/app-router-context.shared-runtime'
import type {
FlightRouterState,
Expand Down Expand Up @@ -450,28 +451,43 @@ function InnerLayoutRouter({
* If no loading property is provided it renders the children without a suspense boundary.
*/
function LoadingBoundary({
children,
hasLoading,
loading,
loadingStyles,
loadingScripts,
children,
}: {
loading: LoadingModuleData | Promise<LoadingModuleData>
children: React.ReactNode
hasLoading: boolean
loading?: React.ReactNode
loadingStyles?: React.ReactNode
loadingScripts?: React.ReactNode
}): JSX.Element {
// We have an explicit prop for checking if `loading` is provided, to disambiguate between a loading
// component that returns `null` / `undefined`, vs not having a loading component at all.
if (hasLoading) {
// 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]
return (
<Suspense
fallback={
<>
{loadingStyles}
{loadingScripts}
{loading}
{loadingRsc}
</>
}
>
Expand Down Expand Up @@ -562,12 +578,7 @@ export default function OuterLayoutRouter({
errorStyles={errorStyles}
errorScripts={errorScripts}
>
<LoadingBoundary
hasLoading={Boolean(loading)}
loading={loading?.[0]}
loadingStyles={loading?.[1]}
loadingScripts={loading?.[2]}
>
<LoadingBoundary loading={loading}>
<HTTPAccessFallbackBoundary notFound={notFound}>
<RedirectBoundary>
<InnerLayoutRouter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export type LazyCacheNode = {
prefetchHead: React.ReactNode
head: React.ReactNode

loading: LoadingModuleData
loading: LoadingModuleData | Promise<LoadingModuleData>

/**
* Child parallel routes.
Expand Down Expand Up @@ -95,7 +95,7 @@ export type ReadyCacheNode = {
prefetchHead: React.ReactNode
head: React.ReactNode

loading: LoadingModuleData
loading: LoadingModuleData | Promise<LoadingModuleData>

parallelRoutes: Map<string, ChildSegmentMap>
}
Expand Down Expand Up @@ -149,7 +149,7 @@ export const LayoutRouterContext = React.createContext<{
childNodes: CacheNode['parallelRoutes']
tree: FlightRouterState
url: string
loading: LoadingModuleData
loading: LoadingModuleData | Promise<LoadingModuleData>
} | null>(null)

export const GlobalLayoutRouterContext = React.createContext<{
Expand Down
2 changes: 1 addition & 1 deletion test/development/acceptance-app/hydration-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ describe('Error overlay for hydration errors in App router', () => {
<ScrollAndFocusHandler segmentPath={[...]}>
<InnerScrollAndFocusHandler segmentPath={[...]} focusAndScrollRef={{apply:false, ...}}>
<ErrorBoundary errorComponent={undefined} errorStyles={undefined} errorScripts={undefined}>
<LoadingBoundary hasLoading={false} loading={undefined} loadingStyles={undefined} loadingScripts={undefined}>
<LoadingBoundary loading={null}>
<HTTPAccessFallbackBoundary notFound={[...]}>
<HTTPAccessFallbackErrorBoundary pathname="/" notFound={[...]} missingSlots={Set}>
<RedirectBoundary>
Expand Down
Loading