Skip to content

Commit

Permalink
No partial prefetch via loading.js when PPR is on
Browse files Browse the repository at this point in the history
Before PPR, the way instant navigations work in Next.js is we prefetch
everything up to the first route segment that defines a loading.js
boundary. The rest of the tree is defered until the actual navigation.
It does not take into account whether the data is dynamic — even if the
tree is completely static, it will still defer everything inside the
loading boundary.

The approach with PPR is different — we prefetch as deeply as possible,
and only defer when dynamic data is accessed. If so, we only defer the
nearest parent Suspense boundary of the dynamic data access, regardless
of whether the boundary is defined by loading.js or a normal <Suspense>
component in userspace.

This PR removes the partial behavior of loading.js when the PPR flag is
enabled. In effect, loading.js now acts like a regular Suspense boundary
with no additional special behavior.

Note that in practice this usually means we'll end up prefetching more
than we were before PPR, which may or may not be considered a
performance regression by some apps. The plan is to address this before
General Availability of PPR by introducing granular per-segment
fetching, so we can reuse as much of the tree as possible during both
prefetches and dynamic navigations. But during the beta period, we
should be clear about this trade off in our communications.
  • Loading branch information
acdlite committed Dec 8, 2023
1 parent 0f36cfc commit 1d4c4a7
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 8 deletions.
41 changes: 36 additions & 5 deletions packages/next/src/server/app-render/create-component-tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export async function createComponentTree({
ctx: AppRenderContext
}): Promise<ComponentTree> {
const {
renderOpts: { nextConfigOutput },
renderOpts: { nextConfigOutput, experimental },
staticGenerationStore,
componentMod: {
staticGenerationBailout,
Expand Down Expand Up @@ -325,12 +325,43 @@ export async function createComponentTree({
// We also want to bail out if there's no Loading component in the tree.
let currentStyles = undefined
let childCacheNodeSeedData: CacheNodeSeedData | null = null

if (
!(
isPrefetch &&
(Loading || !hasLoadingComponentInTree(parallelRoute))
)
// Before PPR, the way instant navigations work in Next.js is we
// prefetch everything up to the first route segment that defines a
// loading.tsx boundary. (We do the same if there's no loading
// boundary in the entire tree, because we don't want to prefetch too
// much) The rest of the tree is defered until the actual navigation.
// It does not take into account whether the data is dynamic — even if
// the tree is completely static, it will still defer everything
// inside the loading boundary.
//
// This behavior predates PPR and is only relevant if the
// PPR flag is not enabled.
isPrefetch &&
(Loading || !hasLoadingComponentInTree(parallelRoute)) &&
// The approach with PPR is different — loading.tsx behaves like a
// regular Suspense boundary and has no special behavior.
//
// With PPR, we prefetch as deeply as possible, and only defer when
// dynamic data is accessed. If so, we only defer the nearest parent
// Suspense boundary of the dynamic data access, regardless of whether
// the boundary is defined by loading.tsx or a normal <Suspense>
// component in userspace.
//
// NOTE: In practice this usually means we'll end up prefetching more
// than we were before PPR, which may or may not be considered a
// performance regression by some apps. The plan is to address this
// before General Availability of PPR by introducing granular
// per-segment fetching, so we can reuse as much of the tree as
// possible during both prefetches and dynamic navigations. But during
// the beta period, we should be clear about this trade off in our
// communications.
!experimental.ppr
) {
// Don't prefetch this child. This will trigger a lazy fetch by the
// client router.
} else {
// Create the child component
const { seedData, styles: childComponentStyles } =
await createComponentTree({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export async function walkTreeWithFlightRouterState({
ctx: AppRenderContext
}): Promise<FlightDataPath[]> {
const {
renderOpts: { nextFontManifest },
renderOpts: { nextFontManifest, experimental },
query,
isPrefetch,
getDynamicParamFromSegment,
Expand Down Expand Up @@ -111,6 +111,8 @@ export async function walkTreeWithFlightRouterState({
flightRouterState[3] === 'refetch'

const shouldSkipComponentTree =
// loading.tsx has no effect on prefetching when PPR is enabled
!experimental.ppr &&
isPrefetch &&
!Boolean(components.loading) &&
(flightRouterState ||
Expand Down
19 changes: 17 additions & 2 deletions test/e2e/app-dir/app/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ import { check, getRedboxHeader, hasRedbox, waitFor } from 'next-test-utils'
import cheerio from 'cheerio'
import stripAnsi from 'strip-ansi'

// TODO: We should decide on an established pattern for gating test assertions
// on experimental flags. For example, as a first step we could all the common
// gates like this one into a single module.
const isPPREnabledByDefault = process.env.__NEXT_EXPERIMENTAL_PPR === 'true'

createNextDescribe(
'app dir - basic',
{
Expand Down Expand Up @@ -550,7 +555,12 @@ createNextDescribe(
})

// TODO-APP: Enable in development
;(isDev ? it.skip : it)(
;(isDev ||
// When PPR is enabled, the shared layouts re-render because we prefetch
// from the root. This will be addressed before GA.
isPPREnabledByDefault
? it.skip
: it)(
'should not rerender layout when navigating between routes in the same layout',
async () => {
const browser = await next.browser('/same-layout/first')
Expand Down Expand Up @@ -1334,7 +1344,12 @@ createNextDescribe(
})

// TODO-APP: disable failing test and investigate later
;(isDev ? it.skip : it)(
;(isDev ||
// When PPR is enabled, the shared layouts re-render because we prefetch
// from the root. This will be addressed before GA.
isPPREnabledByDefault
? it.skip
: it)(
'should render the template that is a server component and rerender on navigation',
async () => {
const browser = await next.browser('/template/servercomponent')
Expand Down

0 comments on commit 1d4c4a7

Please sign in to comment.