Skip to content

Commit

Permalink
backport: fix prefetch bailout detection for nested loading segments (#…
Browse files Browse the repository at this point in the history
…70618)

Backports:
- #67358

Fixes #70527

Note: Turbopack dev failures seem to be pre-existing on the base branch.
  • Loading branch information
ztanner authored Sep 30, 2024
1 parent e19d91c commit 50e41a2
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,16 @@ export async function walkTreeWithFlightRouterState({
// Explicit refresh
flightRouterState[3] === 'refetch'

// Pre-PPR, the `loading` component signals to the router how deep to render the component tree
// to ensure prefetches are quick and inexpensive. If there's no `loading` component anywhere in the tree being rendered,
// the prefetch will be short-circuited to avoid requesting a potentially very expensive subtree. If there's a `loading`
// somewhere in the tree, we'll recursively render the component tree up until we encounter that loading component, and then stop.
const shouldSkipComponentTree =
// loading.tsx has no effect on prefetching when PPR is enabled
!experimental.ppr &&
isPrefetch &&
!Boolean(components.loading) &&
(flightRouterState ||
// If there is no flightRouterState, we need to check the entire loader tree, as otherwise we'll be only checking the root
!hasLoadingComponentInTree(loaderTree))
!hasLoadingComponentInTree(loaderTree)

if (!parentRendered && renderComponentsOnThisLevel) {
const overriddenSegment =
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ export default function HomePage() {
<Link href="/static-page" id="to-static-page">
To Static Page
</Link>
<Link href="/prefetch-auto/foobar" id="to-dynamic-page">
To Dynamic Slug Page
</Link>
</>
)
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/prefetch-auto/[slug]/layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,26 @@ import Link from 'next/link'

export const dynamic = 'force-dynamic'

function getData() {
const res = new Promise((resolve) => {
setTimeout(() => {
resolve({ message: 'Layout Data!' })
}, 2000)
})
return res
}

export default async function Layout({ children }) {
const result = await getData()

return (
<div>
<h1>Layout</h1>
<Link prefetch={undefined} href="/prefetch-auto/justputit">
Prefetch Link
</Link>
{children}
<h3>{JSON.stringify(result)}</h3>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export default function Loading() {
return <h1>Loading Prefetch Auto</h1>
return <h1 id="loading-text">Loading Prefetch Auto</h1>
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export const dynamic = 'force-dynamic'
function getData() {
const res = new Promise((resolve) => {
setTimeout(() => {
resolve({ message: 'Hello World!' })
resolve({ message: 'Page Data!' })
}, 2000)
})
return res
Expand All @@ -13,9 +13,9 @@ export default async function Page({ params }) {
const result = await getData()

return (
<>
<div id="prefetch-auto-page-data">
<h3>{JSON.stringify(params)}</h3>
<h3>{JSON.stringify(result)}</h3>
</>
</div>
)
}
18 changes: 16 additions & 2 deletions test/e2e/app-dir/app-prefetch/prefetching.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ createNextDescribe(
})

const prefetchResponse = await response.text()
expect(prefetchResponse).not.toContain('Hello World')
expect(prefetchResponse).toContain('Page Data')
expect(prefetchResponse).not.toContain('Layout Data!')
expect(prefetchResponse).not.toContain('Loading Prefetch Auto')
})

Expand Down Expand Up @@ -297,10 +298,23 @@ createNextDescribe(
})

const prefetchResponse = await response.text()
expect(prefetchResponse).not.toContain('Hello World')
expect(prefetchResponse).not.toContain('Page Data!')
expect(prefetchResponse).toContain('Loading Prefetch Auto')
})

it('should immediately render the loading state for a dynamic segment when fetched from higher up in the tree', async () => {
const browser = await next.browser('/')
const loadingText = await browser
.elementById('to-dynamic-page')
.click()
.waitForElementByCss('#loading-text')
.text()

expect(loadingText).toBe('Loading Prefetch Auto')

await browser.waitForElementByCss('#prefetch-auto-page-data')
})

describe('dynamic rendering', () => {
describe.each(['/force-dynamic', '/revalidate-0'])('%s', (basePath) => {
it('should not re-render layout when navigating between sub-pages', async () => {
Expand Down

0 comments on commit 50e41a2

Please sign in to comment.