loading.tsx should have no effect on partial rendering when PPR is enabled

packages/next/src/server/app-render/create-component-tree.tsx

@@ -56,7 +56,7 @@ export async function createComponentTree({
   ctx: AppRenderContext
 }): Promise<ComponentTree> {
   const {
-    renderOpts: { nextConfigOutput },
+    renderOpts: { nextConfigOutput, experimental },
     staticGenerationStore,
     componentMod: {
       staticGenerationBailout,
@@ -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({

packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx

@@ -58,7 +58,7 @@ export async function walkTreeWithFlightRouterState({
   ctx: AppRenderContext
 }): Promise<FlightDataPath[]> {
   const {
-    renderOpts: { nextFontManifest },
+    renderOpts: { nextFontManifest, experimental },
     query,
     isPrefetch,
     getDynamicParamFromSegment,
@@ -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 ||

test/e2e/app-dir/app/index.test.ts

@@ -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',
   {
@@ -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')
@@ -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')

test/e2e/app-dir/ppr-navigations/app/layout.tsx

@@ -0,0 +1,9 @@
+import React from 'react'
+
+export default function Root({ children }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}

test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/client.tsx

@@ -0,0 +1,24 @@
+'use client'
+
+import React, { useState, use } from 'react'
+
+const never = new Promise(() => {})
+
+export function TriggerBadSuspenseFallback() {
+  const [shouldSuspend, setShouldSuspend] = useState(false)
+
+  if (shouldSuspend) {
+    use(never)
+  }
+
+  return (
+    <button
+      id="trigger-bad-suspense-fallback"
+      onClick={() => {
+        setShouldSuspend(true)
+      }}
+    >
+      Trigger Bad Suspense Fallback
+    </button>
+  )
+}

test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/loading.tsx

@@ -0,0 +1,5 @@
+import React from 'react'
+
+export default async function LoadingInner() {
+  return <div id="loading-tsx">Loading [inner loading.tsx]...</div>
+}

test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/page.tsx

@@ -0,0 +1,33 @@
+import React, { Suspense } from 'react'
+import { TriggerBadSuspenseFallback } from './client'
+import { getDynamicTestData, getStaticTestData } from './test-data-service'
+
+async function Dynamic({ dataKey }) {
+  return (
+    <div id="dynamic">{await getDynamicTestData(`${dataKey} [dynamic]`)}</div>
+  )
+}
+
+async function Static({ dataKey }) {
+  return <div id="static">{await getStaticTestData(`${dataKey} [static]`)}</div>
+}
+
+export default async function Page({
+  params: { dataKey },
+}: {
+  params: { dataKey: string }
+}) {
+  return (
+    <>
+      <div id="container">
+        <Suspense fallback="Loading dynamic...">
+          <Dynamic dataKey={dataKey} />
+        </Suspense>
+        <Suspense fallback="Loading static...">
+          <Static dataKey={dataKey} />
+        </Suspense>
+      </div>
+      <TriggerBadSuspenseFallback />
+    </>
+  )
+}

test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/test-data-service.ts

@@ -0,0 +1,40 @@
+// NOTE: I've intentionally not yet moved these helpers into a shared module, to
+// avoid early abstraction. I will if/when we start using them for other tests.
+// They are based on the testing patterns we use all over the React codebase, so
+// I'm reasonably confident in them.
+const TEST_DATA_SERVICE_URL = process.env.TEST_DATA_SERVICE_URL
+const ARTIFICIAL_DELAY = 200
+
+async function getTestData(key: string, isStatic: boolean): Promise<string> {
+  const searchParams = new URLSearchParams({
+    key,
+  })
+  if (!TEST_DATA_SERVICE_URL) {
+    // If environment variable is not set, resolve automatically after a delay.
+    // This is so you can run the test app locally without spinning up a
+    // data server.
+    await new Promise<void>((resolve) =>
+      setTimeout(() => resolve(), ARTIFICIAL_DELAY)
+    )
+    return key
+  }
+  const response = await fetch(
+    TEST_DATA_SERVICE_URL + '?' + searchParams.toString(),
+    {
+      cache: isStatic ? 'force-cache' : 'no-store',
+    }
+  )
+  const text = await response.text()
+  if (response.status !== 200) {
+    throw new Error(text)
+  }
+  return text
+}
+
+export async function getStaticTestData(key: string): Promise<string> {
+  return getTestData(key, true)
+}
+
+export async function getDynamicTestData(key: string): Promise<string> {
+  return getTestData(key, false)
+}

test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/start/page.tsx

@@ -0,0 +1,19 @@
+'use client'
+
+import React, { useState } from 'react'
+import Link from 'next/link'
+
+export default function Start() {
+  const [href, setHref] = useState('')
+  return (
+    <form>
+      <input
+        type="text"
+        name="href"
+        onChange={(e) => setHref(e.target.value)}
+        value={href}
+      />
+      {href === '' ? null : <Link href={href}>Navigate</Link>}
+    </form>
+  )
+}

test/e2e/app-dir/ppr-navigations/app/loading.tsx

@@ -0,0 +1,5 @@
+import React from 'react'
+
+export default async function LoadingOuter() {
+  return <div id="loading-tsx">Loading [outer loading.tsx]...</div>
+}

test/e2e/app-dir/ppr-navigations/next.config.js

@@ -0,0 +1,10 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {
+  experimental: {
+    ppr: true,
+  },
+}
+
+module.exports = nextConfig