Skip to content

Commit

Permalink
Restore RSC fetch error handling after navigating back (vercel#73985)
Browse files Browse the repository at this point in the history
As a follow-up to vercel#73975, we're now restoring the error handling for
failed RSC fetch calls when the user navigates back after a hard
navigation. In this case the browser uses the bfcache to restore the
previous JavaScript execution context, and thus the abort controller
will still be in the aborted state. To take this into account, we're now
creating a new `AbortController` instance on `'pageshow'` events.

In addition, the abort controller's `signal` is now actually passed to
the `fetch` call, and not only used for the error handling, so that the
requests are aborted on `'pagehide'`. This was an oversight in the
original PR. With that, it's even more important to create a fresh abort
controller, otherwise RSC fetching would be disabled after back
navigation.

The added e2e test can only run in headed mode unfortunately, as the
bfcache is not available in headless mode. (Using the same approach as
in vercel#54081.)
  • Loading branch information
unstubbable authored Dec 16, 2024
1 parent e9415fc commit e08fe27
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,23 @@ function doMpaNavigation(url: string): FetchServerResponseResult {
}
}

// TODO: Figure out why this module is included in server page bundles.
const win = typeof window === 'undefined' ? undefined : window
const abortController = new AbortController()
let abortController = new AbortController()

if (typeof window !== 'undefined') {
// Abort any in-flight requests when the page is unloaded, e.g. due to
// reloading the page or performing hard navigations. This allows us to ignore
// what would otherwise be a thrown TypeError when the browser cancels the
// requests.
window.addEventListener('pagehide', () => {
abortController.abort()
})

// Abort any in-flight requests when the page is unloaded, e.g. due to reloading
// the page or performing hard navigations. This allows us to ignore what would
// otherwise be a thrown TypeError when the browser cancels the requests.
win?.addEventListener('pagehide', () => {
abortController.abort()
})
// Use a fresh AbortController instance on pageshow, e.g. when navigating back
// and the JavaScript execution context is restored by the browser.
window.addEventListener('pageshow', () => {
abortController = new AbortController()
})
}

/**
* Fetch the flight data for the provided url. Takes in the current router state
Expand Down Expand Up @@ -152,7 +159,12 @@ export async function fetchServerResponse(
: 'low'
: 'auto'

const res = await createFetch(url, headers, fetchPriority)
const res = await createFetch(
url,
headers,
fetchPriority,
abortController.signal
)

const responseUrl = urlToUrlWithoutFlightMarker(res.url)
const canonicalUrl = res.redirected ? responseUrl : undefined
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ export default function HomePage() {
<Link href="/prefetch-auto/foobar" id="to-dynamic-page">
To Dynamic Slug Page
</Link>
<a href="/static-page" id="to-static-page-hard">
Hard Nav to Static Page
</a>
</>
)
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/static-page/back-button.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use client'

export function BackButton() {
return (
<button
type="button"
id="go-back"
onClick={() => {
window.history.back()
}}
>
Go back
</button>
)
}
4 changes: 4 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/static-page/page.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Link from 'next/link'
import { BackButton } from './back-button'

export default async function Page() {
return (
Expand All @@ -14,6 +15,9 @@ export default async function Page() {
To Same Page
</Link>
</p>
<p>
<BackButton />
</p>
</>
)
}
28 changes: 27 additions & 1 deletion test/e2e/app-dir/app-prefetch/prefetching.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe('app dir - prefetching', () => {

it('should not have prefetch error when reloading before prefetch request is finished', async () => {
const browser = await next.browser('/')
await browser.eval('window.nd.router.prefetch("/dashboard/123")')
await browser.eval('window.next.router.prefetch("/dashboard/123")')
await browser.refresh()
const logs = await browser.log()

Expand All @@ -92,6 +92,32 @@ describe('app dir - prefetching', () => {
)
})

it('should not suppress prefetches after navigating back', async () => {
if (!process.env.CI && process.env.HEADLESS) {
console.warn('This test can only run in headed mode. Skipping...')
return
}

// Force headed mode, as bfcache is not available in headless mode.
const browser = await next.browser('/', { headless: false })

// Trigger a hard navigation.
await browser.elementById('to-static-page-hard').click()

// Go back, utilizing the bfcache.
await browser.elementById('go-back').click()

let requests: string[] = []
browser.on('request', (req) => {
requests.push(new URL(req.url()).pathname)
})

await browser.evalAsync('window.next.router.prefetch("/dashboard/123")')
await browser.waitForIdleNetwork()

expect(requests).toInclude('/dashboard/123')
})

it('should not fetch again when a static page was prefetched', async () => {
const browser = await next.browser('/404', browserConfigWithFixedTime)
let requests: string[] = []
Expand Down

0 comments on commit e08fe27

Please sign in to comment.