diff --git a/packages/next/src/server/app-render/create-component-tree.tsx b/packages/next/src/server/app-render/create-component-tree.tsx index 12a01c9c63efd..0fbe4ecd0fc68 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -56,7 +56,7 @@ export async function createComponentTree({ ctx: AppRenderContext }): Promise { 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 + // 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({ diff --git a/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx b/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx index 4bc0b0596ef12..b5fc42b78a96c 100644 --- a/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx +++ b/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx @@ -58,7 +58,7 @@ export async function walkTreeWithFlightRouterState({ ctx: AppRenderContext }): Promise { 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 || diff --git a/test/e2e/app-dir/app/index.test.ts b/test/e2e/app-dir/app/index.test.ts index bef69d621e64e..b6a42c932b12d 100644 --- a/test/e2e/app-dir/app/index.test.ts +++ b/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') diff --git a/test/e2e/app-dir/ppr-navigations/app/layout.tsx b/test/e2e/app-dir/ppr-navigations/app/layout.tsx new file mode 100644 index 0000000000000..c4215a0c905a5 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/app/layout.tsx @@ -0,0 +1,9 @@ +import React from 'react' + +export default function Root({ children }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/client.tsx b/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/client.tsx new file mode 100644 index 0000000000000..627deda15c463 --- /dev/null +++ b/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 ( + + ) +} diff --git a/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/loading.tsx b/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/loading.tsx new file mode 100644 index 0000000000000..8c47211f651d2 --- /dev/null +++ b/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
Loading [inner loading.tsx]...
+} diff --git a/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/page.tsx b/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/page.tsx new file mode 100644 index 0000000000000..4e61a304a72c3 --- /dev/null +++ b/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 ( +
{await getDynamicTestData(`${dataKey} [dynamic]`)}
+ ) +} + +async function Static({ dataKey }) { + return
{await getStaticTestData(`${dataKey} [static]`)}
+} + +export default async function Page({ + params: { dataKey }, +}: { + params: { dataKey: string } +}) { + return ( + <> +
+ + + + + + +
+ + + ) +} diff --git a/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/test-data-service.ts b/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/[dataKey]/test-data-service.ts new file mode 100644 index 0000000000000..3a16a9f702091 --- /dev/null +++ b/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 { + 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((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 { + return getTestData(key, true) +} + +export async function getDynamicTestData(key: string): Promise { + return getTestData(key, false) +} diff --git a/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/start/page.tsx b/test/e2e/app-dir/ppr-navigations/app/loading-tsx-no-partial-rendering/start/page.tsx new file mode 100644 index 0000000000000..657745045e2c8 --- /dev/null +++ b/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 ( +
+ setHref(e.target.value)} + value={href} + /> + {href === '' ? null : Navigate} +
+ ) +} diff --git a/test/e2e/app-dir/ppr-navigations/app/loading.tsx b/test/e2e/app-dir/ppr-navigations/app/loading.tsx new file mode 100644 index 0000000000000..5fd1843a2ae05 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/app/loading.tsx @@ -0,0 +1,5 @@ +import React from 'react' + +export default async function LoadingOuter() { + return
Loading [outer loading.tsx]...
+} diff --git a/test/e2e/app-dir/ppr-navigations/next.config.js b/test/e2e/app-dir/ppr-navigations/next.config.js new file mode 100644 index 0000000000000..016ac8833b57f --- /dev/null +++ b/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 diff --git a/test/e2e/app-dir/ppr-navigations/ppr-navigations.test.ts b/test/e2e/app-dir/ppr-navigations/ppr-navigations.test.ts new file mode 100644 index 0000000000000..ab1b0735ca803 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/ppr-navigations.test.ts @@ -0,0 +1,329 @@ +import { createNext } from 'e2e-utils' +import { findPort } from 'next-test-utils' +import http from 'http' + +describe('ppr-navigations', () => { + if ((global as any).isNextDev) { + test('prefetching is disabled in dev', () => {}) + return + } + + let server + let next + afterEach(async () => { + await next?.destroy() + server?.close() + }) + + test('when PPR is enabled, loading.tsx boundaries do not cause a partial prefetch', async () => { + const TestLog = createTestLog() + let pendingRequests = new Map() + server = createTestDataServer(async (key, res) => { + TestLog.log('REQUEST: ' + key) + if (pendingRequests.has(key)) { + throw new Error('Request already pending for ' + key) + } + pendingRequests.set(key, res) + }) + const port = await findPort() + server.listen(port) + next = await createNext({ + files: __dirname, + env: { TEST_DATA_SERVICE_URL: `http://localhost:${port}` }, + }) + + // There should have been no data requests during build + TestLog.assert([]) + + const browser = await next.browser( + '/loading-tsx-no-partial-rendering/start' + ) + + // Use a text input to set the target URL. + const input = await browser.elementByCss('input') + await input.fill('/loading-tsx-no-partial-rendering/yay') + + // This causes a to appear. (We create the Link after initial render + // so we can control when the prefetch happens.) + const link = await browser.elementByCss('a') + expect(await link.getAttribute('href')).toBe( + '/loading-tsx-no-partial-rendering/yay' + ) + + // The triggers a prefetch. Even though this route has a loading.tsx + // boundary, we're still able to prefetch the static data in the page. + // Without PPR, we would have stopped prefetching at the loading.tsx + // boundary. (The dynamic data is not fetched until navigation.) + await TestLog.waitFor(['REQUEST: yay [static]']) + + // Navigate. This will trigger the dynamic fetch. + await link.click() + + // TODO: Even though the prefetch request hasn't resolved yet, we should + // have already started fetching the dynamic data. Currently, the dynamic + // is fetched lazily during rendering, creating a waterfall. The plan is to + // remove this waterfall by initiating the fetch directly inside the + // router navigation handler, not during render. + TestLog.assert([]) + + // Finish loading the static data + pendingRequests.get('yay [static]').resolve() + + // The static UI appears + await browser.elementById('static') + const container = await browser.elementById('container') + expect(await container.innerHTML()).toEqual( + 'Loading dynamic...
yay [static]
' + ) + + // The dynamic data is fetched + TestLog.assert(['REQUEST: yay [dynamic]']) + + // Finish loading and render the full UI + pendingRequests.get('yay [dynamic]').resolve() + await browser.elementById('dynamic') + expect(await container.innerHTML()).toEqual( + '
yay [dynamic]
yay [static]
' + ) + + // Now we'll demonstrate that even though loading.tsx wasn't activated + // during initial render, it still acts as a regular Suspense boundary. + // Trigger a "bad" Suspense fallback by intentionally suspending without + // startTransition. + await browser.elementById('trigger-bad-suspense-fallback').click() + const loading = await browser.elementById('loading-tsx') + expect(await loading.innerHTML()).toEqual('Loading [inner loading.tsx]...') + }) +}) + +// NOTE: I've intentionally not yet moved these helpers into a separate +// 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. + +type TestDataResponse = { + _res: http.ServerResponse + resolve: (value: string) => any + reject: (value: any) => any +} + +type TestDataServer = { + _server: http.Server + listen: (port: number) => void + close: () => void +} + +// Creates a lightweight HTTP server for use in e2e testing. This simulates the +// data service that would be used in a real Next.js application, whether it's +// direct database access, an ORM, or a higher-level data access layer. The e2e +// test can observe when individual requests are received, and control the +// timing of when the data is fulfilled, without needing to mock any lower +// level I/O. +// +// Receives requests of the form: /?key=foo +// +// Responds in plain text. By default, the response is the key itself, but the +// e2e test can respond with any text it wants. +// +// Examples: +// response.resolve() // Responds with the key itself +// response.resolve('custom') // Responds with custom text +// response.reject(new Error('oops!')) // Responds with a 500 error +// +// Based on the AsyncText pattern used in the React repo. +function createTestDataServer( + onRequest: (key: string, response: TestDataResponse) => any +): TestDataServer { + const httpServer = http.createServer(async (req, res) => { + const searchParams = new URL(req.url, 'http://n').searchParams + const key = searchParams.get('key') + + if (typeof key !== 'string') { + res.statusCode = 400 + const msg = 'Missing key parameter' + res.end(msg) + return + } + + const response: TestDataResponse = { + _res: res, + resolve(value: string | void) { + res.end(value === undefined ? key : value) + }, + reject(error: Error, status?: number) { + res.statusCode = status ?? 500 + res.end(error.message ?? `Failed to fetch data for "${key}"`) + }, + } + + try { + const result = await onRequest(key, response) + if (typeof result === 'string') { + response.resolve(result) + } + } catch (error) { + response.reject(error) + } + }) + return { + _server: httpServer, + listen(port: number) { + httpServer.listen(port) + }, + close() { + httpServer.close() + }, + } +} + +// Creates an event log. You can write to this during testing and then assert +// on the result. +// +// The main use case is for asynchronous e2e tests. It provides a `waitFor` +// method that resolves when the log matches some expected asynchronous sequence +// of events. This is an alternative to setting up a timer loop. It helps catch +// subtle mistakes where the order of events is not expected, or the same +// event happens more than it should. +// +// Based on the Scheduler.log pattern used in the React repo. +function createTestLog() { + let events = [] + + // Represents a pending waitFor call. + let pendingExpectation: null | { + resolve: () => void + reject: (error: Error) => void + expectedEvents: Array + error: Error + } = null + + function log(value: any) { + // Add to the event log. + events.push(value) + + // Check if we've reached the end of the expected log. If there's a + // pending waitFor, and we've reached the last of the expected events, this + // will resolve the promise. + pingExpectation() + } + + function assert(expectedEvents: any[]) { + const actualEvents = events + + if (pendingExpectation !== null) { + const error = new Error('Cannot assert while a waitFor() is pending.') + Error.captureStackTrace(error, assert) + throw error + } + + if (!areLogsEqual(expectedEvents, actualEvents)) { + // Capture the stack trace of `assert` so that Jest will report the + // error as originating from the `assert` call instead of here. + const error = new Error( + 'Expected sequence of events did not occur.\n\n' + + createDiff(expectedEvents, actualEvents) + ) + Error.captureStackTrace(error, assert) + throw error + } + } + + function waitFor(expectedEvents: any[], timeout: number = 5000) { + // Returns a promise that resolves when the event log matches the + // expected sequence. + + // Capture the stack trace of `waitFor` so that if an inner assertion fails, + // Jest will report the error as originating from the `waitFor` call instead + // of inside this module's implementation. + const error = new Error() + Error.captureStackTrace(error, waitFor) + + if (pendingExpectation !== null) { + error.message = 'A previous waitFor() is still pending.' + throw error + } + + let resolve + let reject + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + + const thisExpectation = { + resolve, + reject, + expectedEvents, + error, + } + pendingExpectation = thisExpectation + + setTimeout(() => { + if (pendingExpectation === thisExpectation) { + error.message = `waitFor timed out after ${timeout}ms` + reject(error) + } + }, timeout) + + return promise + } + + function pingExpectation() { + if (pendingExpectation !== null) { + const expectedEvents = pendingExpectation.expectedEvents + if (events.length < expectedEvents.length) { + return + } + + if (areLogsEqual(expectedEvents, events)) { + // We've reached the end of the expected log. Resolve the promise and + // reset the log. + events = [] + pendingExpectation.resolve() + pendingExpectation = null + } else { + // The log does not match what was expected by the test. Reject the + // promise and reset the log. + + // Use the error object that we captured at the start of the `waitFor` + // call. Jest will show that the error originated from `waitFor` call + // instead of inside this internal function. + const error = pendingExpectation.error + error.message = + 'Expected sequence of events did not occur.\n\n' + + createDiff(expectedEvents, events) + + events = [] + pendingExpectation.reject(error) + pendingExpectation = null + } + } + } + + function createDiff(expected, actual) { + // TODO: Jest exposes the diffing utility that it uses for `expect`. + // We could use that here for nicer output. + return ` +Expected: ${JSON.stringify(expected)} +Actual: ${JSON.stringify(actual)} +` + } + + function areLogsEqual(a, b) { + if (a.length !== b.length) { + return false + } + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) { + return false + } + } + return true + } + + return { + log, + waitFor, + assert, + } +}