make router restore action resilient to a missing tree #62098

ztanner · 2024-02-15T13:31:09Z

What

Following an anchor link to a hash param, and then attempting to use history.pushState or history.replaceState, would result in an MPA navigation to the targeted URL.

Why

In #61822, a guard was added to prevent calling ACTION_RESTORE with a missing tree, to match other call-sites where we do the same. This was to prevent the app from crashing in the case where app router internals weren't available in the history state. The original assumption was that this is a rare / unlikely edge case. However the above scenario is a very probable case where this can happen, and triggering an MPA navigation isn't ideal.

How

This updates ACTION_RESTORE to be resilient to an undefined router state tree. When this happens, we'll still trigger the restore action to sync params, but use the existing flight router state.

Closes NEXT-2502

ztanner · 2024-02-15T13:31:22Z

canary
- make router restore action resilient to a missing tree #62098 👈

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ztanner and the rest of your teammates on Graphite

ijjk · 2024-02-15T13:40:30Z

Tests Passed

ijjk · 2024-02-15T13:41:46Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
buildDuration	20.1s	20.2s	⚠️ +102ms
buildDurationCached	7.8s	7.1s	N/A
nodeModulesSize	196 MB	196 MB	⚠️ +1.26 kB
nextStartRea..uration (ms)	438ms	430ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
1068-HASH.js gzip	30.2 kB	30.2 kB	N/A
3f784ff6-HASH.js gzip	53.5 kB	53.5 kB	N/A
4944-HASH.js gzip	5.04 kB	5.03 kB	N/A
8423.HASH.js gzip	181 B	181 B	✓
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	243 B	242 B	N/A
main-HASH.js gzip	32.1 kB	32.1 kB	N/A
webpack-HASH.js gzip	1.7 kB	1.7 kB	✓
Overall change	47.1 kB	47.1 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
_app-HASH.js gzip	196 B	196 B	✓
_error-HASH.js gzip	184 B	183 B	N/A
amp-HASH.js gzip	503 B	504 B	N/A
css-HASH.js gzip	323 B	324 B	N/A
dynamic-HASH.js gzip	2.5 kB	2.51 kB	N/A
edge-ssr-HASH.js gzip	258 B	259 B	N/A
head-HASH.js gzip	353 B	351 B	N/A
hooks-HASH.js gzip	370 B	370 B	✓
image-HASH.js gzip	4.21 kB	4.2 kB	N/A
index-HASH.js gzip	259 B	259 B	✓
link-HASH.js gzip	2.68 kB	2.67 kB	N/A
routerDirect..HASH.js gzip	313 B	314 B	N/A
script-HASH.js gzip	386 B	385 B	N/A
withRouter-HASH.js gzip	309 B	311 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	931 B	931 B	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
_buildManifest.js gzip	485 B	484 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
index.html gzip	529 B	527 B	N/A
link.html gzip	541 B	540 B	N/A
withRouter.html gzip	524 B	522 B	N/A
Overall change	0 B	0 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
edge-ssr.js gzip	94.4 kB	94.4 kB	N/A
page.js gzip	151 kB	151 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
middleware-b..fest.js gzip	624 B	625 B	N/A
middleware-r..fest.js gzip	151 B	151 B	✓
middleware.js gzip	44.4 kB	44.4 kB	N/A
edge-runtime..pack.js gzip	1.94 kB	1.94 kB	✓
Overall change	2.1 kB	2.1 kB	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
app-page-exp...dev.js gzip	166 kB	166 kB	✓
app-page-exp..prod.js gzip	95.5 kB	95.5 kB	✓
app-page-tur..prod.js gzip	97.3 kB	97.3 kB	✓
app-page-tur..prod.js gzip	91.7 kB	91.7 kB	✓
app-page.run...dev.js gzip	136 kB	136 kB	✓
app-page.run..prod.js gzip	90.3 kB	90.3 kB	✓
app-route-ex...dev.js gzip	22 kB	22 kB	✓
app-route-ex..prod.js gzip	14.9 kB	14.9 kB	✓
app-route-tu..prod.js gzip	14.9 kB	14.9 kB	✓
app-route-tu..prod.js gzip	14.6 kB	14.6 kB	✓
app-route.ru...dev.js gzip	21.7 kB	21.7 kB	✓
app-route.ru..prod.js gzip	14.6 kB	14.6 kB	✓
pages-api-tu..prod.js gzip	9.43 kB	9.43 kB	✓
pages-api.ru...dev.js gzip	9.7 kB	9.7 kB	✓
pages-api.ru..prod.js gzip	9.43 kB	9.43 kB	✓
pages-turbo...prod.js gzip	22 kB	22 kB	✓
pages.runtim...dev.js gzip	22.7 kB	22.7 kB	✓
pages.runtim..prod.js gzip	22 kB	22 kB	✓
server.runti..prod.js gzip	50 kB	50 kB	✓
Overall change	925 kB	925 kB	✓

build cache Overall increase ⚠️

	vercel/next.js canary	vercel/next.js 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree	Change
0.pack gzip	1.55 MB	1.55 MB	⚠️ +254 B
index.pack gzip	104 kB	103 kB	N/A
Overall change	1.55 MB	1.55 MB	⚠️ +254 B

Diff details

Diff for page.js

Diff too large to display

Diff for 1068-HASH.js

Diff too large to display

Commit: d57e0e0

packages/next/src/client/components/app-router.tsx

franky47

LGTM, thanks @ztanner ! I'll report back here when it's available in a canary to try the nuqs test suite against.

franky47 · 2024-02-15T17:06:47Z

Confirming 14.1.1-canary.55 fixed the issue: https://github.com/47ng/nuqs/actions/runs/7919580461

franky47 · 2024-02-29T23:10:33Z

How come this PR wasn't included in next@14.1.1? A lot of canaries were skipped in that release..

Following an anchor link to a hash param, and then attempting to use `history.pushState` or `history.replaceState`, would result in an MPA navigation to the targeted URL. In #61822, a guard was added to prevent calling `ACTION_RESTORE` with a missing tree, to match other call-sites where we do the same. This was to prevent the app from crashing in the case where app router internals weren't available in the history state. The original assumption was that this is a rare / unlikely edge case. However the above scenario is a very probable case where this can happen, and triggering an MPA navigation isn't ideal. This updates `ACTION_RESTORE` to be resilient to an undefined router state tree. When this happens, we'll still trigger the restore action to sync params, but use the existing flight router state. Closes NEXT-2502

ggomaeng · 2024-03-05T05:53:14Z

How come this PR wasn't included in next@14.1.1? A lot of canaries were skipped in that release..

yeah this is a crucial bug. it breaks our service where it uses pushState replaceState...

I've been tracking this issue every day to see if it's included.

franky47 · 2024-03-05T05:55:10Z

It has been released in next@14.1.2.

ijjk added area: tests created-by: Next.js team PRs by the Next.js team. type: next labels Feb 15, 2024

ztanner force-pushed the 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree branch from 441ead6 to 937e905 Compare February 15, 2024 13:32

ztanner changed the title ~~make ACTION_RESTORE resilient to a missing tree~~ make router restore action resilient to a missing tree Feb 15, 2024

ztanner mentioned this pull request Feb 15, 2024

Guard against restoring router state with missing data #61822

Merged

ztanner marked this pull request as ready for review February 15, 2024 13:45

ztanner requested review from timneutkens, ijjk, shuding, huozhi, feedthejim and wyattjoh as code owners February 15, 2024 13:45

timneutkens reviewed Feb 15, 2024

View reviewed changes

packages/next/src/client/components/app-router.tsx Outdated Show resolved Hide resolved

timneutkens approved these changes Feb 15, 2024

View reviewed changes

make ACTION_RESTORE resilient to a missing tree

d57e0e0

ztanner force-pushed the 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree branch from 937e905 to d57e0e0 Compare February 15, 2024 13:54

ztanner enabled auto-merge (squash) February 15, 2024 13:57

franky47 approved these changes Feb 15, 2024

View reviewed changes

franky47 mentioned this pull request Feb 15, 2024

Updating url with hash 47ng/nuqs#498

Closed

ztanner merged commit 5309c30 into canary Feb 15, 2024
66 of 71 checks passed

ztanner deleted the 02-15-make_ACTION_RESTORE_resilient_to_a_missing_tree branch February 15, 2024 14:10

franky47 mentioned this pull request Feb 17, 2024

Known issues & solutions / Version compatibility table 47ng/nuqs#423

Open

franky47 mentioned this pull request Mar 4, 2024

Add experimental config for navigation raf test #62668

Merged

github-actions bot added the locked label Mar 19, 2024

github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make router restore action resilient to a missing tree #62098

make router restore action resilient to a missing tree #62098

ztanner commented Feb 15, 2024 •

edited

Loading

ztanner commented Feb 15, 2024

ijjk commented Feb 15, 2024 •

edited

Loading

ijjk commented Feb 15, 2024 •

edited

Loading

franky47 left a comment

franky47 commented Feb 15, 2024

franky47 commented Feb 29, 2024

ggomaeng commented Mar 5, 2024

franky47 commented Mar 5, 2024

make router restore action resilient to a missing tree #62098

make router restore action resilient to a missing tree #62098

Conversation

ztanner commented Feb 15, 2024 • edited Loading

What

Why

How

ztanner commented Feb 15, 2024

ijjk commented Feb 15, 2024 • edited Loading

Tests Passed

ijjk commented Feb 15, 2024 • edited Loading

Stats from current PR

franky47 left a comment

Choose a reason for hiding this comment

franky47 commented Feb 15, 2024

franky47 commented Feb 29, 2024

ggomaeng commented Mar 5, 2024

franky47 commented Mar 5, 2024

ztanner commented Feb 15, 2024 •

edited

Loading

ijjk commented Feb 15, 2024 •

edited

Loading

ijjk commented Feb 15, 2024 •

edited

Loading