Skip to content

Commit

Permalink
parallel routes: fix incorrect optimistic tree when there are multipl…
Browse files Browse the repository at this point in the history
…e parallel routes (#48449)

This PR fixes parallel routes navigation with `prefetch={false}`. This
was broken because the optimistic tree created when navigating with
prefetching disabled resulted in a state where the router tree was
expecting an incorrect node to be rendered and suspended until the
imaginary data arrived.

The fix consists of updating the method that creates the optimistic tree
in order to bailout of the optimistic tree creation when there are
multiple parallel routes for the current node.

<!-- 

## For Contributors

### Improving Documentation or adding/fixing Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md



## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->

fix #48122
link NEXT-1020

---------

Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
  • Loading branch information
feedthejim and timneutkens committed Apr 17, 2023
1 parent ea8c427 commit 9f67638
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,26 @@ export function createOptimisticTree(

const segmentMatches =
existingSegment !== null && matchSegment(existingSegment, segment)
const shouldRefetchThisLevel = !flightRouterState || !segmentMatches

// if there are multiple parallel routes at this level, we need to refetch here
// to ensure we get the correct tree. This is because we don't know which
// parallel route will match the next segment.
const hasMultipleParallelRoutes =
Object.keys(existingParallelRoutes).length > 1
const shouldRefetchThisLevel =
!flightRouterState || !segmentMatches || hasMultipleParallelRoutes

let parallelRoutes: FlightRouterState[1] = {}
if (existingSegment !== null && segmentMatches) {
parallelRoutes = existingParallelRoutes
}

let childTree
if (!isLastSegment) {

// if there's multiple parallel routes at this level, we shouldn't create an
// optimistic tree for the next level because we don't know which one will
// match the next segment.
if (!isLastSegment && !hasMultipleParallelRoutes) {
const childItem = createOptimisticTree(
segments.slice(1),
parallelRoutes ? parallelRoutes.children : null,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import React from 'react'

export default function Page() {
return <div id="default-parallel">default view for parallel</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import React from 'react'

export default function Page() {
return <div id="parallel-foo">parallel for foo</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => null
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import React from 'react'

export default function Layout({ children, parallel }) {
return (
<>
{children}
{parallel}
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Link from 'next/link'
import React from 'react'

export default function Layout() {
return (
<div>
<Link prefetch={false} href="/parallel-prefetch-false/foo">
link
</Link>
</div>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,27 @@ createNextDescribe(
'slot catchall'
)
})

it('should navigate with a link with prefetch=false', async () => {
const browser = await next.browser('/parallel-prefetch-false')

// check if the default view loads
await check(
() => browser.waitForElementByCss('#default-parallel').text(),
'default view for parallel'
)

// check that navigating to /foo re-renders the layout to display @parallel/foo
await check(
() =>
browser
.elementByCss('[href="/parallel-prefetch-false/foo"]')
.click()
.waitForElementByCss('#parallel-foo')
.text(),
'parallel for foo'
)
})
})

describe('route intercepting', () => {
Expand Down

0 comments on commit 9f67638

Please sign in to comment.