-
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 interception/detail routes being triggered by router refreshes #63263
Fix interception/detail routes being triggered by router refreshes #63263
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
996a32e
to
8c6245c
Compare
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
buildDuration | 13.7s | 14s | |
buildDurationCached | 8.4s | 6.1s | N/A |
nodeModulesSize | 198 MB | 198 MB | |
nextStartRea..uration (ms) | 438ms | 445ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
2453-HASH.js gzip | 30.7 kB | 30.8 kB | N/A |
3304.HASH.js gzip | 181 B | 181 B | ✓ |
3f784ff6-HASH.js gzip | 53.7 kB | 53.7 kB | ✓ |
8299-HASH.js gzip | 5.04 kB | 5.04 kB | N/A |
framework-HASH.js gzip | 45.2 kB | 45.2 kB | ✓ |
main-app-HASH.js gzip | 242 B | 242 B | ✓ |
main-HASH.js gzip | 32.2 kB | 32.2 kB | N/A |
webpack-HASH.js gzip | 1.68 kB | 1.68 kB | N/A |
Overall change | 99.3 kB | 99.3 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
_app-HASH.js gzip | 196 B | 197 B | N/A |
_error-HASH.js gzip | 184 B | 184 B | ✓ |
amp-HASH.js gzip | 505 B | 505 B | ✓ |
css-HASH.js gzip | 324 B | 325 B | N/A |
dynamic-HASH.js gzip | 2.5 kB | 2.5 kB | N/A |
edge-ssr-HASH.js gzip | 258 B | 258 B | ✓ |
head-HASH.js gzip | 352 B | 352 B | ✓ |
hooks-HASH.js gzip | 370 B | 371 B | N/A |
image-HASH.js gzip | 4.21 kB | 4.21 kB | ✓ |
index-HASH.js gzip | 259 B | 259 B | ✓ |
link-HASH.js gzip | 2.67 kB | 2.67 kB | N/A |
routerDirect..HASH.js gzip | 314 B | 312 B | N/A |
script-HASH.js gzip | 386 B | 386 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
Overall change | 6.57 kB | 6.57 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
_buildManifest.js gzip | 481 B | 484 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
index.html gzip | 529 B | 529 B | ✓ |
link.html gzip | 541 B | 541 B | ✓ |
withRouter.html gzip | 525 B | 523 B | N/A |
Overall change | 1.07 kB | 1.07 kB | ✓ |
Edge SSR bundle Size
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
edge-ssr.js gzip | 95.4 kB | 95.4 kB | N/A |
page.js gzip | 3.04 kB | 3.04 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 624 B | 624 B | ✓ |
middleware-r..fest.js gzip | 151 B | 151 B | ✓ |
middleware.js gzip | 25.5 kB | 25.5 kB | N/A |
edge-runtime..pack.js gzip | 839 B | 839 B | ✓ |
Overall change | 1.61 kB | 1.61 kB | ✓ |
Next Runtimes
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
app-page-exp...dev.js gzip | 170 kB | 170 kB | N/A |
app-page-exp..prod.js gzip | 96.9 kB | 96.9 kB | N/A |
app-page-tur..prod.js gzip | 98.6 kB | 98.7 kB | N/A |
app-page-tur..prod.js gzip | 93.1 kB | 93.1 kB | N/A |
app-page.run...dev.js gzip | 144 kB | 144 kB | N/A |
app-page.run..prod.js gzip | 91.6 kB | 91.6 kB | N/A |
app-route-ex...dev.js gzip | 21.4 kB | 21.4 kB | ✓ |
app-route-ex..prod.js gzip | 15.1 kB | 15.1 kB | ✓ |
app-route-tu..prod.js gzip | 15.1 kB | 15.1 kB | ✓ |
app-route-tu..prod.js gzip | 14.8 kB | 14.8 kB | ✓ |
app-route.ru...dev.js gzip | 21 kB | 21 kB | ✓ |
app-route.ru..prod.js gzip | 14.8 kB | 14.8 kB | ✓ |
pages-api-tu..prod.js gzip | 9.52 kB | 9.52 kB | ✓ |
pages-api.ru...dev.js gzip | 9.8 kB | 9.8 kB | ✓ |
pages-api.ru..prod.js gzip | 9.52 kB | 9.52 kB | ✓ |
pages-turbo...prod.js gzip | 22.5 kB | 22.5 kB | ✓ |
pages.runtim...dev.js gzip | 23.1 kB | 23.1 kB | ✓ |
pages.runtim..prod.js gzip | 22.5 kB | 22.5 kB | ✓ |
server.runti..prod.js gzip | 50.7 kB | 50.7 kB | ✓ |
Overall change | 250 kB | 250 kB | ✓ |
build cache Overall increase ⚠️
vercel/next.js canary | vercel/next.js 03-13-Fix_interception/detail_routes_being_triggered_by_router_refreshes | Change | |
---|---|---|---|
0.pack gzip | 1.56 MB | 1.57 MB | |
index.pack gzip | 106 kB | 106 kB | N/A |
Overall change | 1.56 MB | 1.57 MB |
Diff details
Diff for middleware.js
Diff too large to display
Diff for 2453-HASH.js
Diff too large to display
Diff for app-page-exp..ntime.dev.js
Diff too large to display
Diff for app-page-exp..time.prod.js
Diff too large to display
Diff for app-page-tur..time.prod.js
Diff too large to display
Diff for app-page-tur..time.prod.js
Diff too large to display
Diff for app-page.runtime.dev.js
Diff too large to display
Diff for app-page.runtime.prod.js
Diff too large to display
72d48ea
to
0400330
Compare
Tests Passed |
0400330
to
6687634
Compare
// Otherwise the server action might be intercepted with the wrong action id | ||
// (ie, one that corresponds with the intercepted route) | ||
const includeNextUrl = | ||
state.nextUrl && hasInterceptionRouteInCurrentTree(state.tree) |
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.
The previous implementation here solved a similar problem for server actions, but it relied on an incorrect assumption about nextUrl
. This more accurately determines if an interception marker is present in the tree.
Confirmed that the previous test that was added for this case still passes (test ref)
6687634
to
0d364f8
Compare
What
Actions that revalidate the router state by kicking off a refetch (such as
router.refresh()
or dev fast refresh) would incorrectly trigger an interception route if one matched the current URL, or in the case of already being on an intercepted route, would trigger the full detail page instead.Why
Interception rewrites use the
nextUrl
header to determine if the requested path should be rewritten to a different path. We currently forward that header indiscriminately, which means that if you were on a non-intercepted route and calledrouter.refresh()
, the UI would change to the intercepted content instead (since it would treat it the same as a soft-navigation).How
This updates various reducers to only forward the
nextUrl
header if there's an interception route present in the tree. If there is, we want to refresh its data, rather than the data for the underlying page. The reverse is also true: if we were on the "full" page, and triggered arouter.refresh()
, we won't forwardnextUrl
meaning it won't fetch the interception data.In order to determine if an interception route is present in the tree, I had to add a new segment type for dynamic interception routes, as by the time they reach the client they are stripped of their interception marker.
Note: There are a series of bugs related to
router.refresh
with parallel/interception routes, such as the previous page/slot content disappearing when triggering a refresh. This does not address all of those cases, but I'm working through them!Fixes #60844
Fixes #62470
Closes NEXT-2737