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

fix redundant fetching of inactive default segments #65935

Open
wants to merge 3 commits into
base: canary
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions packages/next/src/server/app-render/create-component-tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { getTracer } from '../lib/trace/tracer'
import { NextNodeServerSpan } from '../lib/trace/constants'
import { StaticGenBailoutError } from '../../client/components/static-generation-bailout'
import type { LoadingModuleData } from '../../shared/lib/app-router-context.shared-runtime'
import { DEFAULT_SEGMENT_KEY } from '../../shared/lib/segment'

type ComponentTree = {
seedData: CacheNodeSeedData
Expand Down Expand Up @@ -395,6 +396,10 @@ async function createComponentTreeInternal({
// This behavior predates PPR and is only relevant if the
// PPR flag is not enabled.
isPrefetch &&
// If we'd be rendering the default parallel route component, we include it in the tree.
// Otherwise, the prefetch would cause any parallel routes rendering the default state
// to be refetched on navigation, because the server will have cleared them.
parallelRoute[0] !== DEFAULT_SEGMENT_KEY &&
(Loading || !hasLoadingComponentInTree(parallelRoute)) &&
// The approach with PPR is different — loading.tsx behaves like a
// regular Suspense boundary and has no special behavior.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Loading() {
return <h1>Grid Loading</h1>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import Link from 'next/link'

const alphabet = Array.from('abcdefghijklmnopqrstuvwxyz')

function getShuffledAlphabet() {
return alphabet
.map((value) => ({ value, sort: Math.random() }))
.sort((a, b) => a.sort - b.sort)
.map(({ value }) => value)
}

export default async function Page({
params: { letter },
}: {
params: { letter: string }
}) {
await new Promise((r) => setTimeout(r, 250))

return (
<>
<span id="letter-parallel">{letter}</span>
<ul>
{getShuffledAlphabet().map((letter) => (
<li key={letter}>
<Link href={`/${letter}`}>
<div>{letter}</div>
</Link>
</li>
))}
</ul>
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { ReactNode } from 'react'

type LetterLayoutProps = {
params: { letter: string }
children: ReactNode
grid: ReactNode
grid2: ReactNode
grid3: ReactNode
grid4: ReactNode
grid5: ReactNode
}

export default function LetterLayout(props: LetterLayoutProps) {
return (
<div className="bg-gray-100">
{props.children}
<div className="bg-gray-400 p-4">
{props.grid}
{props.grid2}
{props.grid3}
{props.grid4}
{props.grid5}
</div>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Loading() {
return <h1>Loading</h1>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default async function LetterPage({
params: { letter },
}: {
params: { letter: string }
}) {
await new Promise((r) => setTimeout(r, 250))
return <h1 id="letter-page">{letter}</h1>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Root({ children }: { children: React.ReactNode }) {
return (
<html>
<body>{children}</body>
</html>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import Link from 'next/link'

export default function Page() {
return <Link href="/a">Go to a letter</Link>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}

module.exports = nextConfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { nextTestSetup } from 'e2e-utils'
import { retry } from 'next-test-utils'

describe('parallel-routes-prefetching', () => {
const { next } = nextTestSetup({
files: __dirname,
})

it('should not re-fetch data for the __DEFAULT__ segments when navigating within a dynamic segment', async () => {
let prefetchRequests = []
let rscRequests = []
const browser = await next.browser('/a', {
beforePageLoad(page) {
page.on('request', (request) => {
return request.allHeaders().then((headers) => {
if (headers['Next-Router-Prefetch'.toLowerCase()] === '1') {
prefetchRequests.push(request.url())
} else if (headers['RSC'.toLowerCase()] === '1') {
rscRequests.push(request.url())
}
})
})
},
})

expect(await browser.elementById('letter-page').text()).toBe('a')
expect(await browser.elementById('letter-parallel').text()).toBe('a')

await retry(() => {
// we should see 25 prefetch requests for each letter of the alphabet (excluding the current)
expect(prefetchRequests).toHaveLength(25)
// and 0 RSC requests since we're not navigating to a different dynamic segment
expect(rscRequests).toHaveLength(0)
})

prefetchRequests = []

await browser.elementByCss('[href="/b"]').click()

await retry(() => {
// Navigating to a new dynamic segment should trigger any additional prefetch requests
expect(prefetchRequests).toHaveLength(0)

// but we except 2 RSC requests for the new page:
// - one for the `children` slot
// - one for the active parallel slot
// TODO: It would be nice to achieve this in a single request, but it currently isn't possible,
// because the router will see missing data for both slots simultaneously and kick off two separate requests.
// PPR doesn't have this issue because it doesn't perform the "lazy fetch" in layout-router.
expect(rscRequests).toHaveLength(
process.env.__NEXT_EXPERIMENTAL_PPR ? 1 : 2
)
})

expect(await browser.elementById('letter-page').text()).toBe('b')
expect(await browser.elementById('letter-parallel').text()).toBe('b')
})
})
Loading