Skip to content

Commit

Permalink
fix redundant fetching of inactive default segments
Browse files Browse the repository at this point in the history
  • Loading branch information
ztanner committed May 18, 2024
1 parent 695aec9 commit 448098b
Show file tree
Hide file tree
Showing 16 changed files with 172 additions and 0 deletions.
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')
})
})

0 comments on commit 448098b

Please sign in to comment.