fix inconsistent scroll restoration behavior (#59366)

### What?
While scrolled on a page, and when following a link to a new page and
clicking the browser back button or using `router.back()`, the scroll
position would sometimes restore scroll to the incorrect spot (in the
case of the test added in this PR, it'd scroll you back to the top of
the list)

### Why?
The refactor in #56497 changed the way router actions are processed:
specifically, all actions were assumed to be async, even if they could
be handled synchronously. For most actions this is fine, as most are
currently async. However, `ACTION_RESTORE` (triggered when the
`popstate` event occurs) isn't async, and introducing a small amount of
delay in the handling of this action can cause the browser to not
properly restore the scroll position

### How?
This special-cases `ACTION_RESTORE` to synchronously process the action
and call `setState` when it's received, rather than creating a promise.
To consistently reproduce this behavior, I added an option to our
browser interface that'll allow us to programmatically trigger a CPU
slowdown.

h/t to @alvarlagerlof for isolating the offending commit and sharing a
minimal reproduction.

Closes NEXT-1819
Likely addresses #58899 but the reproduction was too complex to verify.

Author: ztanner

packages/next/src/shared/lib/router/action-queue.ts

@@ -6,6 +6,7 @@ import {
   ACTION_REFRESH,
   ACTION_SERVER_ACTION,
   ACTION_NAVIGATE,
+  ACTION_RESTORE,
 } from '../../../client/components/router-reducer/router-reducer-types'
 import type { ReduxDevToolsInstance } from '../../../client/components/use-reducer-with-devtools'
 import { reducer } from '../../../client/components/router-reducer/router-reducer'
@@ -26,7 +27,7 @@ export type AppRouterActionQueue = {
 export type ActionQueueNode = {
   payload: ReducerActions
   next: ActionQueueNode | null
-  resolve: (value: PromiseLike<AppRouterState> | AppRouterState) => void
+  resolve: (value: ReducerState) => void
   reject: (err: Error) => void
   discarded?: boolean
 }
@@ -115,14 +116,26 @@ function dispatchAction(
   setState: DispatchStatePromise
 ) {
   let resolvers: {
-    resolve: (value: AppRouterState | PromiseLike<AppRouterState>) => void
+    resolve: (value: ReducerState) => void
     reject: (reason: any) => void
-  }
+  } = { resolve: setState, reject: () => {} }
+
+  // most of the action types are async with the exception of restore
+  // it's important that restore is handled quickly since it's fired on the popstate event
+  // and we don't want to add any delay on a back/forward nav
+  // this only creates a promise for the async actions
+  if (payload.type !== ACTION_RESTORE) {
+    // Create the promise and assign the resolvers to the object.
+    const deferredPromise = new Promise<AppRouterState>((resolve, reject) => {
+      resolvers = { resolve, reject }
+    })
 
-  // Create the promise and assign the resolvers to the object.
-  const deferredPromise = new Promise<AppRouterState>((resolve, reject) => {
-    resolvers = { resolve, reject }
-  })
+    startTransition(() => {
+      // we immediately notify React of the pending promise -- the resolver is attached to the action node
+      // and will be called when the associated action promise resolves
+      setState(deferredPromise)
+    })
+  }
 
   const newAction: ActionQueueNode = {
     payload,
@@ -131,12 +144,6 @@ function dispatchAction(
     reject: resolvers!.reject,
   }
 
-  startTransition(() => {
-    // we immediately notify React of the pending promise -- the resolver is attached to the action node
-    // and will be called when the associated action promise resolves
-    setState(deferredPromise)
-  })
-
   // Check if the queue is empty
   if (actionQueue.pending === null) {
     // The queue is empty, so add the action and start it immediately

test/e2e/app-dir/navigation/app/scroll-restoration/context.ts

@@ -0,0 +1,10 @@
+import { createContext } from 'react'
+
+export interface Item {
+  id: number
+}
+
+export const ItemsContext = createContext<{
+  items: Item[]
+  loadMoreItems: () => void
+}>({ items: [], loadMoreItems: () => {} })

test/e2e/app-dir/navigation/app/scroll-restoration/layout.tsx

@@ -0,0 +1,27 @@
+'use client'
+import { useState } from 'react'
+import { ItemsContext, type Item } from './context'
+
+const createItems = (start: number, end: number): Item[] => {
+  const items: Item[] = []
+  for (let i = start; i <= end; i++) {
+    items.push({ id: i })
+  }
+  return items
+}
+
+export default function Layout({ children }: { children: React.ReactNode }) {
+  const [items, setItems] = useState<Item[]>(createItems(1, 50))
+
+  const loadMoreItems = () => {
+    const start = items.length + 1
+    const end = start + 50 - 1
+    setItems((prevItems) => [...prevItems, ...createItems(start, end)])
+  }
+
+  return (
+    <ItemsContext.Provider value={{ items, loadMoreItems }}>
+      {children}
+    </ItemsContext.Provider>
+  )
+}

test/e2e/app-dir/navigation/app/scroll-restoration/other/page.tsx

@@ -0,0 +1,13 @@
+'use client'
+import { useRouter } from 'next/navigation'
+
+export default function Page() {
+  const router = useRouter()
+  return (
+    <div>
+      <button onClick={() => router.back()} id="back-button">
+        Go Back
+      </button>
+    </div>
+  )
+}

test/e2e/app-dir/navigation/app/scroll-restoration/page.tsx

@@ -0,0 +1,23 @@
+'use client'
+import Link from 'next/link'
+import React, { useContext } from 'react'
+import { ItemsContext } from './context'
+
+export default function Page() {
+  const { items, loadMoreItems } = useContext(ItemsContext)
+
+  return (
+    <div>
+      <h1>Page</h1>
+      <ul>
+        {items.map((item) => (
+          <li key={item.id}>Item {item.id}</li>
+        ))}
+      </ul>
+      <button id="load-more" onClick={loadMoreItems}>
+        Load More
+      </button>
+      <Link href="/scroll-restoration/other">Go to Other</Link>
+    </div>
+  )
+}

test/e2e/app-dir/navigation/navigation.test.ts

@@ -732,5 +732,34 @@ createNextDescribe(
         }
       })
     })
+
+    describe('scroll restoration', () => {
+      it('should restore original scroll position when navigating back', async () => {
+        const browser = await next.browser('/scroll-restoration', {
+          // throttling the CPU to rule out flakiness based on how quickly the page loads
+          cpuThrottleRate: 6,
+        })
+        const body = await browser.elementByCss('body')
+        expect(await body.text()).toContain('Item 50')
+        await browser.elementById('load-more').click()
+        await browser.elementById('load-more').click()
+        await browser.elementById('load-more').click()
+        expect(await body.text()).toContain('Item 200')
+
+        // scroll to the bottom of the page
+        await browser.eval('window.scrollTo(0, document.body.scrollHeight)')
+
+        // grab the current position
+        const scrollPosition = await browser.eval('window.pageYOffset')
+
+        await browser.elementByCss("[href='/scroll-restoration/other']").click()
+        await browser.elementById('back-button').click()
+
+        const newScrollPosition = await browser.eval('window.pageYOffset')
+
+        // confirm that the scroll position was restored
+        await check(() => scrollPosition === newScrollPosition, true)
+      })
+    })
   }
 )

test/lib/browsers/base.ts

@@ -125,7 +125,15 @@ export abstract class BrowserInterface implements PromiseLike<any> {
   off(event: Event, cb: (...args: any[]) => void) {}
   async loadPage(
     url: string,
-    { disableCache: boolean, beforePageLoad: Function }
+    {
+      disableCache,
+      cpuThrottleRate,
+      beforePageLoad,
+    }: {
+      disableCache?: boolean
+      cpuThrottleRate?: number
+      beforePageLoad?: Function
+    }
   ): Promise<void> {}
   async get(url: string): Promise<void> {}
 

test/lib/browsers/playwright.ts

@@ -128,7 +128,11 @@ export class Playwright extends BrowserInterface {
 
   async loadPage(
     url: string,
-    opts?: { disableCache: boolean; beforePageLoad?: (...args: any[]) => void }
+    opts?: {
+      disableCache: boolean
+      cpuThrottleRate: number
+      beforePageLoad?: (...args: any[]) => void
+    }
   ) {
     if (this.activeTrace) {
       const traceDir = path.join(__dirname, '../../traces')
@@ -182,6 +186,14 @@ export class Playwright extends BrowserInterface {
       session.send('Network.setCacheDisabled', { cacheDisabled: true })
     }
 
+    if (opts?.cpuThrottleRate) {
+      const session = await context.newCDPSession(page)
+      // https://chromedevtools.github.io/devtools-protocol/tot/Emulation/#method-setCPUThrottlingRate
+      session.send('Emulation.setCPUThrottlingRate', {
+        rate: opts.cpuThrottleRate,
+      })
+    }
+
     page.on('websocket', (ws) => {
       if (tracePlaywright) {
         page

test/lib/next-webdriver.ts

@@ -68,6 +68,7 @@ export default async function webdriver(
     disableJavaScript?: boolean
     headless?: boolean
     ignoreHTTPSErrors?: boolean
+    cpuThrottleRate?: number
   }
 ): Promise<BrowserInterface> {
   let CurrentInterface: new () => BrowserInterface
@@ -87,6 +88,7 @@ export default async function webdriver(
     disableJavaScript,
     ignoreHTTPSErrors,
     headless,
+    cpuThrottleRate,
   } = options
 
   // we import only the needed interface
@@ -124,7 +126,11 @@ export default async function webdriver(
 
   console.log(`\n> Loading browser with ${fullUrl}\n`)
 
-  await browser.loadPage(fullUrl, { disableCache, beforePageLoad })
+  await browser.loadPage(fullUrl, {
+    disableCache,
+    cpuThrottleRate,
+    beforePageLoad,
+  })
   console.log(`\n> Loaded browser with ${fullUrl}\n`)
 
   // Wait for application to hydrate