Skip to content

Commit

Permalink
fix router crash on revalidate + popstate
Browse files Browse the repository at this point in the history
  • Loading branch information
ztanner committed Feb 27, 2024
1 parent 18200a8 commit 9f072c1
Show file tree
Hide file tree
Showing 10 changed files with 99 additions and 10 deletions.
2 changes: 1 addition & 1 deletion packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ export function createEmptyCacheNode(): CacheNode {
rsc: null,
prefetchRsc: null,
parallelRoutes: new Map(),
lazyDataResolved: false,
}
}

Expand Down Expand Up @@ -578,7 +579,6 @@ function Router({
return
}

// @ts-ignore useTransition exists
// TODO-APP: Ideally the back button should not use startTransition as it should apply the updates synchronously
// Without startTransition works if the cache is there for this path
startTransition(() => {
Expand Down
23 changes: 16 additions & 7 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ function InnerLayoutRouter({
prefetchRsc: null,
head: null,
parallelRoutes: new Map(),
lazyDataResolved: false,
}

/**
Expand Down Expand Up @@ -411,6 +412,7 @@ function InnerLayoutRouter({
context.nextUrl,
buildId
)
childNode.lazyDataResolved = false
}

/**
Expand All @@ -419,15 +421,22 @@ function InnerLayoutRouter({
// When the data has not resolved yet `use` will suspend here.
const serverResponse = use(lazyData)

// setTimeout is used to start a new transition during render, this is an intentional hack around React.
setTimeout(() => {
startTransition(() => {
changeByServerResponse({
previousTree: fullTree,
serverResponse,
if (!childNode.lazyDataResolved) {
// setTimeout is used to start a new transition during render, this is an intentional hack around React.
setTimeout(() => {
startTransition(() => {
changeByServerResponse({
previousTree: fullTree,
serverResponse,
})
})
})
})

// It's important that we mark this as resolved, in case this branch is replayed, we don't want to continously re-apply
// the patch to the tree.
childNode.lazyDataResolved = true
}

// Suspend infinitely as `changeByServerResponse` will cause a different part of the tree to be rendered.
use(createInfinitePromise()) as never
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ describe('navigateReducer', () => {
"buildId": "development",
"cache": {
"lazyData": null,
"lazyDataResolved": false,
"parallelRoutes": Map {
"children" => Map {
"linking" => {
Expand Down Expand Up @@ -402,6 +403,7 @@ describe('navigateReducer', () => {
"buildId": "development",
"cache": {
"lazyData": null,
"lazyDataResolved": false,
"parallelRoutes": Map {
"children" => Map {
"linking" => {
Expand Down Expand Up @@ -1142,6 +1144,7 @@ describe('navigateReducer', () => {
"buildId": "development",
"cache": {
"lazyData": null,
"lazyDataResolved": false,
"parallelRoutes": Map {
"children" => Map {
"linking" => {
Expand Down Expand Up @@ -1406,6 +1409,7 @@ describe('navigateReducer', () => {
"buildId": "development",
"cache": {
"lazyData": null,
"lazyDataResolved": false,
"parallelRoutes": Map {
"children" => Map {
"parallel-tab-bar" => {
Expand Down Expand Up @@ -1824,6 +1828,7 @@ describe('navigateReducer', () => {
"buildId": "development",
"cache": {
"lazyData": null,
"lazyDataResolved": false,
"parallelRoutes": Map {
"children" => Map {
"linking" => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ describe('serverPatchReducer', () => {
"buildId": "development",
"cache": {
"lazyData": null,
"lazyDataResolved": false,
"parallelRoutes": Map {
"children" => Map {
"linking" => {
Expand Down Expand Up @@ -378,6 +379,7 @@ describe('serverPatchReducer', () => {
"buildId": "development",
"cache": {
"lazyData": null,
"lazyDataResolved": false,
"parallelRoutes": Map {
"children" => Map {
"linking" => {
Expand Down
10 changes: 10 additions & 0 deletions packages/next/src/shared/lib/app-router-context.shared-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ export type ChildSegmentMap = Map<string, CacheNode>
export type CacheNode = ReadyCacheNode | LazyCacheNode

export type LazyCacheNode = {
/**
* Whether the lazy cache node data promise has been resolved.
* This value is only true after we've called `use` on the promise (and applied the data to the tree).
*/
lazyDataResolved?: boolean
/**
* When rsc is null, this is a lazily-initialized cache node.
*
Expand Down Expand Up @@ -59,6 +64,11 @@ export type LazyCacheNode = {
}

export type ReadyCacheNode = {
/**
* Whether the lazy cache node data promise has been resolved.
* This value is only true after we've called `use` on the promise (and applied the data to the tree).
*/
lazyDataResolved?: boolean
/**
* When rsc is not null, it represents the RSC data for the
* corresponding segment.
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/shared/lib/router/action-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ function dispatchAction(
const newAction: ActionQueueNode = {
payload,
next: null,
resolve: resolvers!.resolve,
reject: resolvers!.reject,
resolve: resolvers.resolve,
reject: resolvers.reject,
}

// Check if the queue is empty
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use server'

import { revalidatePath } from 'next/cache'

export async function action() {
revalidatePath('/', 'layout')
return true
}
20 changes: 20 additions & 0 deletions test/e2e/app-dir/navigation/app/popstate-revalidate/foo/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use client'

import { useFormState } from 'react-dom'
import { action } from './action'

export default function Page() {
const [submitted, formAction] = useFormState(action, false)
if (submitted) {
return <div>Form Submitted.</div>
}

return (
<div>
<h1>Form</h1>
<form action={formAction}>
<button id="submit-button">Push this button to submit the form</button>
</form>
</div>
)
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/navigation/app/popstate-revalidate/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Link from 'next/link'

export default async function Home() {
await new Promise((resolve) => setTimeout(resolve, 1500))

return (
<div>
<h1>Home</h1>
<Link href="/popstate-revalidate/foo">To /foo</Link>
</div>
)
}
23 changes: 23 additions & 0 deletions test/e2e/app-dir/navigation/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -862,5 +862,28 @@ createNextDescribe(
})
})
})

describe('browser back to a revalidated page', () => {
it('should load the page', async () => {
const browser = await next.browser('/popstate-revalidate')
expect(await browser.elementByCss('h1').text()).toBe('Home')
await browser.elementByCss("[href='/popstate-revalidate/foo']").click()
await browser.waitForElementByCss('#submit-button')
expect(await browser.elementByCss('h1').text()).toBe('Form')
await browser.elementById('submit-button').click()

await retry(async () => {
expect(await browser.elementByCss('body').text()).toContain(
'Form Submitted.'
)
})

await browser.back()

await retry(async () => {
expect(await browser.elementByCss('h1').text()).toBe('Home')
})
})
})
}
)

0 comments on commit 9f072c1

Please sign in to comment.