Skip to content

Commit

Permalink
fix loading issue when navigating to page with async metadata (#61687)
Browse files Browse the repository at this point in the history
### What
Client-side transitioning to a page that triggered a loading boundary
with async metadata would cause the transition to stall, potentially
getting stuck in a refetch loop.

### Why
In layout-router, we trigger a "lazy fetch" when we encounter a segment
that we don't have cache nodes for. This calls out to the server and
suspends until the data fetch is resolved, and applied to the router
tree. However after suspending but before updating the client router, we
set `childNode.lazyData` to null. When we unsuspend from the server
patch action, `childNode.rsc` might still be missing and clearing
`lazyData` means we've blown away the reference to the fetch we already
had pending, triggering a refetch loop.

### How
This removes the logic that mutates the cache node in render, as this is
not concurrent safe, and doesn't appear to be needed for anything.

Fixes #61117
Closes NEXT-2361

---------

Co-authored-by: Jiachi Liu <inbox@huozhi.im>
  • Loading branch information
ztanner and huozhi committed Feb 28, 2024
1 parent 05b972b commit 8fd3d7e
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 5 deletions.
3 changes: 0 additions & 3 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,6 @@ function InnerLayoutRouter({
// When the data has not resolved yet `use` will suspend here.
const [flightData, overrideCanonicalUrl] = use(lazyData)

// segmentPath from the server does not match the layout's segmentPath
childNode.lazyData = null

// setTimeout is used to start a new transition during render, this is an intentional hack around React.
setTimeout(() => {
startTransition(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function SubmitButton() {
)
}

export default async function Page() {
export default function Page() {
return (
<>
<h1>Add to cart</h1>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Loading() {
return <div id="loading">Loading</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from 'react'

// ensure this page is dynamically rendered so we always trigger a loading state
export const dynamic = 'force-dynamic'

export default function page() {
return <div id="page-content">Content</div>
}

async function getTitle() {
return await new Promise((resolve) =>
setTimeout(() => resolve('Async Title'), 1000)
)
}

export async function generateMetadata() {
return { title: await getTitle() }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Link from 'next/link'

export default function page() {
return (
<div>
<Link href="/metadata-await-promise/nested">Link to nested</Link>
</div>
)
}
31 changes: 30 additions & 1 deletion test/e2e/app-dir/navigation/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ createNextDescribe(
{
files: __dirname,
},
({ next, isNextDev, isNextDeploy }) => {
({ next, isNextDev, isNextDeploy, isNextStart }) => {
describe('query string', () => {
it('should set query correctly', async () => {
const browser = await next.browser('/')
Expand Down Expand Up @@ -788,5 +788,34 @@ createNextDescribe(
expect(newScrollPosition).toEqual(scrollPosition)
})
})

describe('navigating to a page with async metadata', () => {
it('should render the final state of the page with correct metadata', async () => {
const browser = await next.browser('/metadata-await-promise')

// dev + PPR doesn't trigger the loading boundary as it's not prefetched
if (isNextDev && process.env.__NEXT_EXPERIMENTAL_PPR) {
await browser
.elementByCss("[href='/metadata-await-promise/nested']")
.click()
} else {
const loadingText = await browser
.elementByCss("[href='/metadata-await-promise/nested']")
.click()
.waitForElementByCss('#loading')
.text()

expect(loadingText).toBe('Loading')
}

await retry(async () => {
expect(await browser.elementById('page-content').text()).toBe(
'Content'
)

expect(await browser.elementByCss('title').text()).toBe('Async Title')
})
})
})
}
)

0 comments on commit 8fd3d7e

Please sign in to comment.