-
Notifications
You must be signed in to change notification settings - Fork 26.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix parallel routes with server actions / revalidating router cache #59585
Fix parallel routes with server actions / revalidating router cache #59585
Conversation
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
Stats from current PRDefault BuildGeneral 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 | |
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
Failing test suitesCommit: c5867f3
Expand output● app dir - navigation › query string › useParams identity between renders › should be stable in pages
Read more about building and testing Next.js in contributing.md. |
ac1e464
to
898003d
Compare
: withoutSearchParameters && segment.startsWith(PAGE_SEGMENT_KEY) | ||
? PAGE_SEGMENT_KEY | ||
: segment | ||
// if the segment is an array, it means it's a dynamic segment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No change in behavior to this file, just split it out of a ternary to better annotate with comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional nit, otherwise lgtm
packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts
Outdated
Show resolved
Hide resolved
46f12df
to
8b6d34a
Compare
8b6d34a
to
c5867f3
Compare
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()
, orredirect()
) inside of a parallel route would break the router preventing subsequent actions, and not resolve any pending state such as fromuseFormState
.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 adefault.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 havenotFound()
behavior when there isn't adefault.tsx
component for a particular segment. This makes it so that when loading a page that renders a slot without slot content / adefault
, it 404s. But when performing a client-side navigation, the intended behavior is different: we keep whatever was in thedefault
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