fix inconsistent scroll restoration behavior

[ztanner]

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.

[ztanner]

Current dependencies on/for this PR:

• canary
  • PR fix inconsistent scroll restoration behavior #59366 👈

This stack of pull requests is managed by Graphite.

[ijjk]

Tests Passed

[ijjk]

Stats from current PR

Default Build

General Overall increase ⚠️

	vercel/next.js canary	vercel/next.js 12-07-fix_inconsistent_scroll_restoration_behavior	Change
buildDuration	10.9s	11.1s	⚠️ +195ms
buildDurationCached	6s	6.1s	N/A
nodeModulesSize	199 MB	199 MB	⚠️ +2.43 kB
nextStartRea..uration (ms)	420ms	424ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js 12-07-fix_inconsistent_scroll_restoration_behavior	Change
170-HASH.js gzip	26.7 kB	26.7 kB	N/A
199.HASH.js gzip	181 B	185 B	N/A
3f784ff6-HASH.js gzip	53.3 kB	53.3 kB	✓
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	242 B	241 B	N/A
main-HASH.js gzip	31.7 kB	31.7 kB	N/A
webpack-HASH.js gzip	1.7 kB	1.7 kB	N/A
Overall change	98.5 kB	98.5 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js 12-07-fix_inconsistent_scroll_restoration_behavior	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js 12-07-fix_inconsistent_scroll_restoration_behavior	Change
_app-HASH.js gzip	195 B	194 B	N/A
_error-HASH.js gzip	183 B	182 B	N/A
amp-HASH.js gzip	501 B	501 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.5 kB	2.5 kB	N/A
edge-ssr-HASH.js gzip	255 B	255 B	✓
head-HASH.js gzip	349 B	350 B	N/A
hooks-HASH.js gzip	368 B	369 B	N/A
image-HASH.js gzip	4.27 kB	4.27 kB	N/A
index-HASH.js gzip	255 B	256 B	N/A
link-HASH.js gzip	2.6 kB	2.6 kB	N/A
routerDirect..HASH.js gzip	311 B	309 B	N/A
script-HASH.js gzip	384 B	384 B	✓
withRouter-HASH.js gzip	307 B	306 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	1.57 kB	1.57 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js 12-07-fix_inconsistent_scroll_restoration_behavior	Change
_buildManifest.js gzip	483 B	483 B	✓
Overall change	483 B	483 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js 12-07-fix_inconsistent_scroll_restoration_behavior	Change
index.html gzip	527 B	530 B	N/A
link.html gzip	541 B	542 B	N/A
withRouter.html gzip	523 B	525 B	N/A
Overall change	0 B	0 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js 12-07-fix_inconsistent_scroll_restoration_behavior	Change
edge-ssr.js gzip	93.5 kB	93.5 kB	N/A
page.js gzip	146 kB	146 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js 12-07-fix_inconsistent_scroll_restoration_behavior	Change
middleware-b..fest.js gzip	622 B	625 B	N/A
middleware-r..fest.js gzip	151 B	151 B	✓
middleware.js gzip	37.4 kB	37.4 kB	N/A
edge-runtime..pack.js gzip	1.92 kB	1.92 kB	✓
Overall change	2.07 kB	2.07 kB	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js 12-07-fix_inconsistent_scroll_restoration_behavior	Change
app-page-exp...dev.js gzip	168 kB	168 kB	✓
app-page-exp..prod.js gzip	93.9 kB	93.9 kB	✓
app-page-tur..prod.js gzip	94.7 kB	94.7 kB	✓
app-page-tur..prod.js gzip	89.2 kB	89.2 kB	✓
app-page.run...dev.js gzip	138 kB	138 kB	✓
app-page.run..prod.js gzip	88.6 kB	88.6 kB	✓
app-route-ex...dev.js gzip	24 kB	24 kB	✓
app-route-ex..prod.js gzip	16.7 kB	16.7 kB	✓
app-route-tu..prod.js gzip	16.7 kB	16.7 kB	✓
app-route-tu..prod.js gzip	16.3 kB	16.3 kB	✓
app-route.ru...dev.js gzip	23.4 kB	23.4 kB	✓
app-route.ru..prod.js gzip	16.3 kB	16.3 kB	✓
pages-api-tu..prod.js gzip	9.37 kB	9.37 kB	✓
pages-api.ru...dev.js gzip	9.64 kB	9.64 kB	✓
pages-api.ru..prod.js gzip	9.37 kB	9.37 kB	✓
pages-turbo...prod.js gzip	21.9 kB	21.9 kB	✓
pages.runtim...dev.js gzip	22.6 kB	22.6 kB	✓
pages.runtim..prod.js gzip	21.9 kB	21.9 kB	✓
server.runti..prod.js gzip	49.4 kB	49.4 kB	✓
Overall change	930 kB	930 kB	✓

Diff details

Diff for page.js

Diff too large to display

Diff for 170-HASH.js

Diff too large to display

Commit: b56d6c0

[alvarlagerlof]

Nice to see this making it all the way to the test. Could potentially be interesting to turn this on for some other tests once to see if any of them break unexpectedly.

[acdlite]

I'm confused, where is the setState if this is an ACTION_RESTORE action?

[ztanner]

This is initialized to setState here:

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

Line 121 in b56d6c0

} = { resolve: setState, reject: () => {} }

And called in the action runner:

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

Line 99 in b56d6c0

action.resolve(nextState)