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 aae7776 commit b81e020
Show file tree
Hide file tree
Showing 17 changed files with 187 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
18 changes: 18 additions & 0 deletions packages/next/src/shared/lib/router/utils/route-regex.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,24 @@ describe('getNamedRouteRegex', () => {
expect(regex.re.test('/photos/1/2/3')).toBe(true)
expect(regex.re.test('/photos')).toBe(true)
})

it('should not match `.rsc` or `.prefetch.rsc` or `.action` suffixes in the capture group', async () => {
const regex = getNamedRouteRegex('/[[...kind]]', true)
expect(regex.routeKeys).toEqual({
nxtPkind: 'nxtPkind',
})

expect(regex.groups['kind']).toEqual({
pos: 1,
repeat: true,
optional: true,
})

let namedRegexp = new RegExp(regex.namedRegex)

const match = '/index.rsc'.match(namedRegexp)
expect(match?.groups?.['nxtPkind']).toBe('index')
})
})

describe('parseParameter', () => {
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, 1000))

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, 1000))
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,55 @@
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.
expect(rscRequests).toHaveLength(2)
})

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

0 comments on commit b81e020

Please sign in to comment.