Skip to content

Commit 0ebbf3b

Browse files
wyattjohalii
authored andcommitted
fix: revert client segment route changes for sub shell generation (vercel#81740)
### What? Implements a new sortable routes system that distinguishes between source pages and rendered pages for sub-shell generation. ### Why? The existing route sorting logic in Next.js doesn't account for the distinction between the original page definition (`sourcePage`) and the final rendered route (`page`). This is critical for sub-shell generation, given this example: | Page | Source Page | |--------|--------| | `/en/[teamSlug]/[projectSlug]/monitoring` | `/[lang]/[teamSlug]/[projectSlug]/monitoring` | | `/fr/[teamSlug]/[projectSlug]/monitoring` | `/[lang]/[teamSlug]/[projectSlug]/monitoring` | | `/[lang]/[teamSlug]/~/monitoring` | `/[lang]/[teamSlug]/~/monitoring` | And a request to `/en/vercel/~/monitoring` that it will hit the `/[lang]/[teamSlug]/~/monitoring` page instead of the `/en/[teamSlug]/[projectSlug]/monitoring`. This is because we first need to sort by the Source Page and then the Page to ensure that the correct route ordering is respected. ### How? - Adds new `SortableRoute` type that tracks both `sourcePage` and `page` - Implements depth-first route comparison algorithm that properly handles segment specificity - Refactors build process to use the new sorting functions instead of legacy `getSortedRoutes` - Adds source page tracking for PPR-enabled routes with `clientSegmentEnabled` - Includes comprehensive test coverage for all route sorting scenarios The new sorting algorithm ensures consistent route ordering across builds and proper sub-shell generation behavior.
1 parent 46ea0ef commit 0ebbf3b

File tree

6 files changed

+1337
-39
lines changed

6 files changed

+1337
-39
lines changed

packages/next/src/build/index.ts

Lines changed: 73 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,7 @@ import {
8181
DYNAMIC_CSS_MANIFEST,
8282
TURBOPACK_CLIENT_MIDDLEWARE_MANIFEST,
8383
} from '../shared/lib/constants'
84-
import {
85-
getSortedRoutes,
86-
isDynamicRoute,
87-
getSortedRouteObjects,
88-
} from '../shared/lib/router/utils'
84+
import { isDynamicRoute } from '../shared/lib/router/utils'
8985
import type { __ApiPreviewProps } from '../server/api-utils'
9086
import loadConfig from '../server/config'
9187
import type { BuildManifest } from '../server/get-page-files'
@@ -210,6 +206,11 @@ import { extractNextErrorCode } from '../lib/error-telemetry-utils'
210206
import { runAfterProductionCompile } from './after-production-compile'
211207
import { generatePreviewKeys } from './preview-key-utils'
212208
import { handleBuildComplete } from './adapter/build-complete'
209+
import {
210+
sortPageObjects,
211+
sortPages,
212+
sortSortableRouteObjects,
213+
} from '../shared/lib/router/utils/sortable-routes'
213214

214215
type Fallback = null | boolean | string
215216

@@ -430,7 +431,7 @@ export type RoutesManifest = {
430431
}
431432
headers: Array<ManifestHeaderRoute>
432433
staticRoutes: Array<ManifestRoute>
433-
dynamicRoutes: Array<ManifestRoute>
434+
dynamicRoutes: ReadonlyArray<ManifestRoute>
434435
dataRoutes: Array<ManifestDataRoute>
435436
i18n?: {
436437
domains?: ReadonlyArray<{
@@ -1328,14 +1329,20 @@ export default async function build(
13281329
const isAppPPREnabled = checkIsAppPPREnabled(config.experimental.ppr)
13291330

13301331
const routesManifestPath = path.join(distDir, ROUTES_MANIFEST)
1332+
const dynamicRoutes: Array<ManifestRoute> = []
1333+
1334+
/**
1335+
* A map of all the pages to their sourcePage value. This is only used for
1336+
* routes that have PPR enabled and clientSegmentEnabled is true.
1337+
*/
1338+
const sourcePages = new Map<string, string>()
13311339
const routesManifest: RoutesManifest = nextBuildSpan
13321340
.traceChild('generate-routes-manifest')
13331341
.traceFn(() => {
1334-
const sortedRoutes = getSortedRoutes([
1342+
const sortedRoutes = sortPages([
13351343
...pageKeys.pages,
13361344
...(pageKeys.app ?? []),
13371345
])
1338-
const dynamicRoutes: Array<ManifestRoute> = []
13391346
const staticRoutes: Array<ManifestRoute> = []
13401347

13411348
for (const route of sortedRoutes) {
@@ -2466,7 +2473,7 @@ export default async function build(
24662473
if (serverPropsPages.size > 0 || ssgPages.size > 0) {
24672474
// We update the routes manifest after the build with the
24682475
// data routes since we can't determine these until after build
2469-
routesManifest.dataRoutes = getSortedRoutes([
2476+
routesManifest.dataRoutes = sortPages([
24702477
...serverPropsPages,
24712478
...ssgPages,
24722479
]).map((page) => {
@@ -2930,39 +2937,39 @@ export default async function build(
29302937
// route), any routes that were generated with unknown route params
29312938
// should be collected and included in the dynamic routes part
29322939
// of the manifest instead.
2933-
const routes: PrerenderedRoute[] = []
2934-
const dynamicRoutes: PrerenderedRoute[] = []
2940+
const staticPrerenderedRoutes: PrerenderedRoute[] = []
2941+
const dynamicPrerenderedRoutes: PrerenderedRoute[] = []
29352942

29362943
// Sort the outputted routes to ensure consistent output. Any route
29372944
// though that has unknown route params will be pulled and sorted
29382945
// independently. This is because the routes with unknown route
29392946
// params will contain the dynamic path parameters, some of which
29402947
// may conflict with the actual prerendered routes.
2941-
let unknownPrerenderRoutes: PrerenderedRoute[] = []
2942-
let knownPrerenderRoutes: PrerenderedRoute[] = []
2948+
const unsortedUnknownPrerenderRoutes: PrerenderedRoute[] = []
2949+
const unsortedKnownPrerenderRoutes: PrerenderedRoute[] = []
29432950
for (const prerenderedRoute of prerenderedRoutes) {
29442951
if (
29452952
prerenderedRoute.fallbackRouteParams &&
29462953
prerenderedRoute.fallbackRouteParams.length > 0
29472954
) {
2948-
unknownPrerenderRoutes.push(prerenderedRoute)
2955+
unsortedUnknownPrerenderRoutes.push(prerenderedRoute)
29492956
} else {
2950-
knownPrerenderRoutes.push(prerenderedRoute)
2957+
unsortedKnownPrerenderRoutes.push(prerenderedRoute)
29512958
}
29522959
}
29532960

2954-
unknownPrerenderRoutes = getSortedRouteObjects(
2955-
unknownPrerenderRoutes,
2961+
const sortedUnknownPrerenderRoutes = sortPageObjects(
2962+
unsortedUnknownPrerenderRoutes,
29562963
(prerenderedRoute) => prerenderedRoute.pathname
29572964
)
2958-
knownPrerenderRoutes = getSortedRouteObjects(
2959-
knownPrerenderRoutes,
2965+
const sortedKnownPrerenderRoutes = sortPageObjects(
2966+
unsortedKnownPrerenderRoutes,
29602967
(prerenderedRoute) => prerenderedRoute.pathname
29612968
)
29622969

29632970
prerenderedRoutes = [
2964-
...knownPrerenderRoutes,
2965-
...unknownPrerenderRoutes,
2971+
...sortedKnownPrerenderRoutes,
2972+
...sortedUnknownPrerenderRoutes,
29662973
]
29672974

29682975
for (const prerenderedRoute of prerenderedRoutes) {
@@ -2979,16 +2986,16 @@ export default async function build(
29792986
) {
29802987
// If the route has unknown params, then we need to add it to
29812988
// the list of dynamic routes.
2982-
dynamicRoutes.push(prerenderedRoute)
2989+
dynamicPrerenderedRoutes.push(prerenderedRoute)
29832990
} else {
29842991
// If the route doesn't have unknown params, then we need to
2985-
// add it to the list of routes.
2986-
routes.push(prerenderedRoute)
2992+
// add it to the list of static routes.
2993+
staticPrerenderedRoutes.push(prerenderedRoute)
29872994
}
29882995
}
29892996

29902997
// Handle all the static routes.
2991-
for (const route of routes) {
2998+
for (const route of staticPrerenderedRoutes) {
29922999
if (isDynamicRoute(page) && route.pathname === page) continue
29933000
if (route.pathname === UNDERSCORE_NOT_FOUND_ROUTE) continue
29943001

@@ -3075,7 +3082,7 @@ export default async function build(
30753082
// they are enabled, then it'll already be included in the
30763083
// prerendered routes.
30773084
if (!isRoutePPREnabled) {
3078-
dynamicRoutes.push({
3085+
dynamicPrerenderedRoutes.push({
30793086
params: {},
30803087
pathname: page,
30813088
encodedPathname: page,
@@ -3088,7 +3095,7 @@ export default async function build(
30883095
})
30893096
}
30903097

3091-
for (const route of dynamicRoutes) {
3098+
for (const route of dynamicPrerenderedRoutes) {
30923099
const normalizedRoute = normalizePagePath(route.pathname)
30933100

30943101
const metadata = exportResult.byPath.get(
@@ -3103,18 +3110,43 @@ export default async function build(
31033110
}
31043111

31053112
let prefetchDataRoute: string | undefined
3113+
let dynamicRoute = routesManifest.dynamicRoutes.find(
3114+
(r) => r.page === route.pathname
3115+
)
31063116
if (!isAppRouteHandler && isAppPPREnabled) {
31073117
prefetchDataRoute = path.posix.join(
31083118
`${normalizedRoute}${RSC_PREFETCH_SUFFIX}`
31093119
)
3120+
3121+
// If the dynamic route wasn't found, then we need to create
3122+
// it. This ensures that for each fallback shell there's an
3123+
// entry in the app routes manifest which enables routing for
3124+
// this fallback shell.
3125+
if (!dynamicRoute) {
3126+
dynamicRoute = pageToRoute(route.pathname)
3127+
sourcePages.set(route.pathname, page)
3128+
3129+
// This route is not for the internal router, but instead
3130+
// for external routers.
3131+
dynamicRoute.skipInternalRouting = true
3132+
3133+
// Push this to the end of the array. The dynamic routes are
3134+
// sorted by page later.
3135+
dynamicRoutes.push(dynamicRoute)
3136+
}
31103137
}
31113138

31123139
if (!isAppRouteHandler && metadata?.segmentPaths) {
3113-
const dynamicRoute = routesManifest.dynamicRoutes.find(
3114-
(r) => r.page === page
3115-
)
3140+
// If PPR isn't enabled, then we might not find the dynamic
3141+
// route by pathname. If that's the case, we need to find the
3142+
// route by page.
31163143
if (!dynamicRoute) {
3117-
throw new InvariantError('Dynamic route not found')
3144+
dynamicRoute = dynamicRoutes.find((r) => r.page === page)
3145+
3146+
// If it can't be found by page, we must throw an error.
3147+
if (!dynamicRoute) {
3148+
throw new InvariantError('Dynamic route not found')
3149+
}
31183150
}
31193151

31203152
dynamicRoute.prefetchSegmentDataRoutes ??= []
@@ -3573,11 +3605,16 @@ export default async function build(
35733605
}
35743606
})
35753607

3576-
// As we may have modified the routesManifest.dynamicRoutes, we need to
3577-
// sort the dynamic routes by page.
3578-
routesManifest.dynamicRoutes = getSortedRouteObjects(
3579-
routesManifest.dynamicRoutes,
3580-
(route) => route.page
3608+
// As we may have modified the dynamicRoutes, we need to sort the
3609+
// dynamic routes by page.
3610+
routesManifest.dynamicRoutes = sortSortableRouteObjects(
3611+
dynamicRoutes,
3612+
(route) => ({
3613+
// If the route is PPR enabled, and has an associated source page,
3614+
// use it. Otherwise fallback to the page which should be the same.
3615+
sourcePage: sourcePages.get(route.page) ?? route.page,
3616+
page: route.page,
3617+
})
35813618
)
35823619

35833620
// Now write the routes manifest out.

packages/next/src/server/config-shared.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type {
1818
ManifestHeaderRoute,
1919
ManifestRedirectRoute,
2020
RouteType,
21+
ManifestRoute,
2122
} from '../build'
2223
import { isStableBuild } from '../shared/lib/canary-only'
2324

@@ -63,7 +64,7 @@ export interface NextAdapter {
6364
afterFiles: Array<ManifestRewriteRoute>
6465
fallback: Array<ManifestRewriteRoute>
6566
}
66-
dynamicRoutes: Array<{}>
67+
dynamicRoutes: ReadonlyArray<ManifestRoute>
6768
}
6869
outputs: AdapterOutputs
6970
}) => Promise<void> | void

0 commit comments

Comments
 (0)