Fix parallel routes with server actions / revalidating router cache

[ztanner]

What?

There are a bunch of different bugs caused by the same underlying issue, but the common thread is that performing any sort of router cache update (either through router.refresh(), revalidatePath(), or redirect()) inside of a parallel route would break the router preventing subsequent actions, and not resolve any pending state such as from useFormState.

Why?

applyPatch is responsible for taking an update response from the server and merging it into the client router cache. However, there's specific bailout logic to skip over applying the patch to a __DEFAULT__ segment (which corresponds with a default.tsx page). When the router detects a cache node that is expected to be rendered on the page but contains no data, the router will trigger a lazy fetch to retrieve the data that's expected to be there (ref) and then update the router cache once the data resolves (ref).

This is causing the router to get stuck in a loop: it'll fetch the data for the cache node, send the data to the router reducer to merge it into the existing cache nodes, skip merging that data in for __DEFAULT__ segments, and repeat.

How?

We currently assign __DEFAULT__ to have notFound() behavior when there isn't a default.tsx component for a particular segment. This makes it so that when loading a page that renders a slot without slot content / a default, it 404s. But when performing a client-side navigation, the intended behavior is different: we keep whatever was in the default slots place, until the user refreshes the page, which would then 404.

However, this logic is incorrect when triggering any of the above mentioned cache node revalidation strategies: if we always skip applying to the __DEFAULT__ segment, slots will never properly handle reducer actions that rely on making changes to their cache nodes.

This splits these different applyPatch functions: one that will apply to the full tree, and another that'll apply to everything except the default segments with the existing bailout condition.

Fixes #54173
Fixes #58772
Fixes #54723
Fixes #57665

Closes NEXT-1706
Closes NEXT-1815
Closes NEXT-1812

[ztanner]

Current dependencies on/for this PR:

• canary
  • PR Fix parallel routes with server actions / revalidating router cache #59585 👈

This stack of pull requests is managed by Graphite.

[ijjk]

Stats from current PR

Default Build

General Overall increase ⚠️

	vercel/next.js canary	vercel/next.js 12-13-fix_action_revalidation_within_a_parallel_slot	Change
buildDuration	12.7s	12.8s	N/A
buildDurationCached	7.2s	6.2s	N/A
nodeModulesSize	200 MB	200 MB	⚠️ +11.1 kB
nextStartRea..uration (ms)	432ms	421ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js 12-13-fix_action_revalidation_within_a_parallel_slot	Change
170-HASH.js gzip	26.8 kB	26.9 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	240 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-13-fix_action_revalidation_within_a_parallel_slot	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-13-fix_action_revalidation_within_a_parallel_slot	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.61 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-13-fix_action_revalidation_within_a_parallel_slot	Change
_buildManifest.js gzip	484 B	482 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js 12-13-fix_action_revalidation_within_a_parallel_slot	Change
index.html gzip	530 B	526 B	N/A
link.html gzip	542 B	539 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 12-13-fix_action_revalidation_within_a_parallel_slot	Change
edge-ssr.js gzip	93.7 kB	93.7 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-13-fix_action_revalidation_within_a_parallel_slot	Change
middleware-b..fest.js gzip	626 B	621 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-13-fix_action_revalidation_within_a_parallel_slot	Change
app-page-exp...dev.js gzip	168 kB	168 kB	✓
app-page-exp..prod.js gzip	94.1 kB	94.1 kB	✓
app-page-tur..prod.js gzip	94.8 kB	94.8 kB	✓
app-page-tur..prod.js gzip	89.4 kB	89.4 kB	✓
app-page.run...dev.js gzip	138 kB	138 kB	✓
app-page.run..prod.js gzip	88.7 kB	88.7 kB	✓
app-route-ex...dev.js gzip	24 kB	24 kB	✓
app-route-ex..prod.js gzip	16.6 kB	16.6 kB	✓
app-route-tu..prod.js gzip	16.6 kB	16.6 kB	✓
app-route-tu..prod.js gzip	16.2 kB	16.2 kB	✓
app-route.ru...dev.js gzip	23.4 kB	23.4 kB	✓
app-route.ru..prod.js gzip	16.2 kB	16.2 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.5 kB	22.5 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: c5867f3

[ijjk]

Failing test suites

Commit: c5867f3

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

• app dir - navigation > query string > useParams identity between renders > should be stable in pages

Expand output

● app dir - navigation › query string › useParams identity between renders › should be stable in pages

TIMED OUT: success

fail

undefined

  626 |
  627 |   if (hardError) {
> 628 |     throw new Error('TIMED OUT: ' + regex + '\n\n' + content + '\n\n' + lastErr)
      |           ^
  629 |   }
  630 |   return false
  631 | }

  at check (lib/next-test-utils.ts:628:11)
  at runTests (e2e/app-dir/navigation/navigation.test.ts:88:11)
  at Object.<anonymous> (e2e/app-dir/navigation/navigation.test.ts:113:11)

Read more about building and testing Next.js in contributing.md.

[ztanner]

No change in behavior to this file, just split it out of a ternary to better annotate with comments

[feedthejim]

optional nit, otherwise lgtm