Skip to content

Commit 30fe57f

Browse files
alan-agius4alan-agius4
committed
fix(@angular/ssr): preserve preload metadata for nested empty-path routes
Previously, `handleRoute` yielded a parent route's metadata before appending `loadChildren` preloads or traversing child routes, while `traverseRoutesConfig` appended a trailing slash only to empty-path routes. Under a non-empty parent, the parent ('/pricing') and its empty-path child ('/pricing/') received distinct keys during `getRoutesFromAngularRouterConfig` deduplication (`seenRoutes`) and were only merged later by `RouteTree.insert`. However, when an empty-path route was nested under another empty-path route (such as at the root '/' or under '/es' -> '' -> ''), both the parent and child received the same key, causing `seenRoutes` to keep the outer route's metadata and drop the descendant's accumulated `preload` and redirect metadata. Update `handleRoute` to append `loadChildren` preloads upfront and allow the first matching empty-path child route to override its parent's metadata for the same path, and remove the redundant `addTrailingSlash` keying in `traverseRoutesConfig`. Closes #34320
1 parent 07845f0 commit 30fe57f

3 files changed

Lines changed: 156 additions & 36 deletions

File tree

‎packages/angular/ssr/src/routes/ng-routes.ts‎

Lines changed: 61 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { AngularAppManifest, getAngularAppManifest } from '../manifest';
3131
import { AngularBootstrap, isNgModule } from '../utils/ng';
3232
import { promiseWithAbort } from '../utils/promise';
3333
import { VALID_REDIRECT_RESPONSE_CODES, isValidRedirectResponseCode } from '../utils/redirect';
34-
import { addTrailingSlash, joinUrlParts, stripLeadingSlash } from '../utils/url';
34+
import { joinUrlParts, stripLeadingSlash } from '../utils/url';
3535
import {
3636
PrerenderFallback,
3737
RenderMode,
@@ -170,20 +170,28 @@ async function* handleRoute(options: {
170170
} = options;
171171

172172
const { redirectTo, loadChildren, loadComponent, children, ɵentryName } = route;
173-
if (ɵentryName && loadComponent) {
173+
if (ɵentryName && (loadComponent || loadChildren)) {
174174
appendPreloadToMetadata(ɵentryName, entryPointToBrowserMapping, metadata);
175175
}
176176

177+
const parentRoutes = new Map<string, RouteTreeNodeMetadata>();
178+
177179
if (metadata.renderMode === RenderMode.Prerender) {
178-
yield* handleSSGRoute(
180+
for await (const result of handleSSGRoute(
179181
serverConfigRouteTree,
180182
typeof redirectTo === 'string' ? redirectTo : undefined,
181183
parentRoute,
182184
metadata,
183185
parentInjector,
184186
invokeGetPrerenderParams,
185187
includePrerenderFallbackRoutes,
186-
);
188+
)) {
189+
if ('error' in result) {
190+
yield result;
191+
} else {
192+
parentRoutes.set(result.route, result);
193+
}
194+
}
187195
} else if (redirectTo !== undefined) {
188196
if (metadata.status && !isValidRedirectResponseCode(metadata.status)) {
189197
yield {
@@ -192,34 +200,23 @@ async function* handleRoute(options: {
192200
`Please use one of the following redirect response codes: ${[...VALID_REDIRECT_RESPONSE_CODES.values()].join(', ')}.`,
193201
};
194202
} else if (typeof redirectTo === 'string') {
195-
yield {
203+
parentRoutes.set(metadata.route, {
196204
...metadata,
197205
redirectTo: resolveRedirectTo(parentRoute, redirectTo),
198206
relativeRedirect: redirectTo[0] !== '/' ? true : undefined,
199-
};
207+
});
200208
} else {
201-
yield metadata;
209+
parentRoutes.set(metadata.route, metadata);
202210
}
203211
} else {
204-
yield metadata;
212+
parentRoutes.set(metadata.route, metadata);
205213
}
206214

207-
// Recursively process child routes
208-
if (children?.length) {
209-
yield* traverseRoutesConfig({
210-
...options,
211-
routes: children,
212-
parentRoute: currentRoutePath,
213-
parentPreloads: metadata.preload,
214-
});
215-
}
216-
217-
// Load and process lazy-loaded child routes
218-
if (loadChildren) {
219-
if (ɵentryName) {
220-
appendPreloadToMetadata(ɵentryName, entryPointToBrowserMapping, metadata);
221-
}
215+
// Recursively process child and lazy-loaded routes
216+
let childRoutes: Route[] | undefined = children;
217+
let childInjector = parentInjector;
222218

219+
if (!childRoutes?.length && loadChildren) {
223220
const routeInjector = route.providers
224221
? createEnvironmentInjector(
225222
route.providers,
@@ -230,16 +227,48 @@ async function* handleRoute(options: {
230227

231228
const loadedChildRoutes = await loadChildrenHelper(route, compiler, routeInjector);
232229
if (loadedChildRoutes) {
233-
const { routes: childRoutes, injector = routeInjector } = loadedChildRoutes;
234-
yield* traverseRoutesConfig({
235-
...options,
236-
routes: childRoutes,
237-
parentInjector: injector,
238-
parentRoute: currentRoutePath,
239-
parentPreloads: metadata.preload,
240-
});
230+
childRoutes = loadedChildRoutes.routes;
231+
childInjector = loadedChildRoutes.injector ?? routeInjector;
241232
}
242233
}
234+
235+
if (!childRoutes?.length) {
236+
yield* parentRoutes.values();
237+
238+
return;
239+
}
240+
241+
const childResults: (RouteTreeNodeMetadata | { error: string })[] = [];
242+
const overriddenParentRoutes = new Set<string>();
243+
const traverseChildRoutes = traverseRoutesConfig({
244+
...options,
245+
routes: childRoutes,
246+
parentInjector: childInjector,
247+
parentRoute: currentRoutePath,
248+
parentPreloads: metadata.preload,
249+
});
250+
251+
for await (const result of traverseChildRoutes) {
252+
if ('error' in result) {
253+
childResults.push(result);
254+
continue;
255+
}
256+
257+
// If a child route resolves to the exact same path as the parent (i.e., an empty-path child),
258+
// let the first matching child override the parent's metadata so accumulated preloads and
259+
// redirects from the activated descendant chain are preserved.
260+
if (parentRoutes.has(result.route)) {
261+
if (!overriddenParentRoutes.has(result.route)) {
262+
overriddenParentRoutes.add(result.route);
263+
parentRoutes.set(result.route, result);
264+
}
265+
} else {
266+
childResults.push(result);
267+
}
268+
}
269+
270+
yield* parentRoutes.values();
271+
yield* childResults;
243272
} catch (error) {
244273
yield {
245274
error: `Error in handleRoute for '${options.currentRoutePath}': ${(error as Error).message}`,
@@ -344,10 +373,7 @@ async function* traverseRoutesConfig(options: {
344373
renderMode: RenderMode.Prerender,
345374
...matchedMetaData,
346375
preload: parentPreloads,
347-
// Match Angular router behavior
348-
// ['one', 'two', ''] -> 'one/two/'
349-
// ['one', 'two', 'three'] -> 'one/two/three'
350-
route: path === '' ? addTrailingSlash(currentRoutePath) : currentRoutePath,
376+
route: currentRoutePath,
351377
presentInClientRouter: undefined,
352378
},
353379
currentRoutePath,

‎packages/angular/ssr/test/routes/ng-routes_spec.ts‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,4 +1044,95 @@ describe('extractRoutesAndCreateRouteTree', () => {
10441044

10451045
expect(isDiscoveringRoutes).toBeTrue();
10461046
});
1047+
1048+
it('should preserve preload metadata for routes under nested empty-path routes', async () => {
1049+
setAngularAppTestingManifest(
1050+
[
1051+
{
1052+
path: '',
1053+
children: [
1054+
{
1055+
path: '',
1056+
pathMatch: 'full',
1057+
loadChildren: () => [
1058+
{
1059+
path: '',
1060+
loadComponent: () => DummyComponent,
1061+
ɵentryName: 'home-page',
1062+
} as Route,
1063+
],
1064+
ɵentryName: 'home-routes',
1065+
} as Route,
1066+
{
1067+
path: '',
1068+
children: [
1069+
{
1070+
path: 'pricing',
1071+
loadChildren: () => [
1072+
{
1073+
path: '',
1074+
loadComponent: () => DummyComponent,
1075+
ɵentryName: 'pricing-page',
1076+
} as Route,
1077+
],
1078+
ɵentryName: 'pricing-routes',
1079+
} as Route,
1080+
],
1081+
},
1082+
],
1083+
},
1084+
{
1085+
path: 'es',
1086+
children: [
1087+
{
1088+
path: '',
1089+
pathMatch: 'full',
1090+
loadChildren: () => [
1091+
{
1092+
path: '',
1093+
loadComponent: () => DummyComponent,
1094+
ɵentryName: 'home-page',
1095+
} as Route,
1096+
],
1097+
ɵentryName: 'home-routes',
1098+
} as Route,
1099+
],
1100+
},
1101+
],
1102+
[{ path: '**', renderMode: RenderMode.Prerender }],
1103+
undefined,
1104+
undefined,
1105+
undefined,
1106+
undefined,
1107+
undefined,
1108+
undefined,
1109+
undefined,
1110+
{
1111+
'home-routes': ['chunk-home-routes.js'],
1112+
'home-page': ['chunk-home-page.js'],
1113+
'pricing-routes': ['chunk-pricing-routes.js'],
1114+
'pricing-page': ['chunk-pricing-page.js'],
1115+
},
1116+
);
1117+
1118+
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({ url });
1119+
expect(errors).toHaveSize(0);
1120+
expect(routeTree.toObject()).toEqual([
1121+
{
1122+
route: '/',
1123+
renderMode: RenderMode.Prerender,
1124+
preload: ['chunk-home-routes.js', 'chunk-home-page.js'],
1125+
},
1126+
{
1127+
route: '/pricing',
1128+
renderMode: RenderMode.Prerender,
1129+
preload: ['chunk-pricing-routes.js', 'chunk-pricing-page.js'],
1130+
},
1131+
{
1132+
route: '/es',
1133+
renderMode: RenderMode.Prerender,
1134+
preload: ['chunk-home-routes.js', 'chunk-home-page.js'],
1135+
},
1136+
]);
1137+
});
10471138
});

‎packages/angular/ssr/test/testing-utils.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { bootstrapApplication } from '@angular/platform-browser';
1717
import { RouterOutlet, Routes, provideRouter } from '@angular/router';
1818
import { type CompactPlan, compileSheet, encodePlan } from 'beasties/compiler';
1919
import { destroyAngularServerApp } from '../src/app';
20-
import { ServerAsset, setAngularAppManifest } from '../src/manifest';
20+
import { AngularAppManifest, ServerAsset, setAngularAppManifest } from '../src/manifest';
2121
import { ServerRoute, provideServerRendering, withRoutes } from '../src/routes/route-config';
2222

2323
@Component({
@@ -44,6 +44,7 @@ class AppComponent {}
4444
* root component and all its children.
4545
* @param inlineCriticalCss - An optional record of stylesheet paths to CSS content to be compiled into critical CSS plans.
4646
* @param nonce - An optional Content Security Policy (CSP) nonce to be used for inlined critical CSS.
47+
* @param entryPointToBrowserMapping - An optional mapping of entry-point names to their associated browser bundles.
4748
*/
4849
export function setAngularAppTestingManifest(
4950
routes: Routes,
@@ -55,6 +56,7 @@ export function setAngularAppTestingManifest(
5556
extraProviders: Array<Provider | EnvironmentProviders> = [],
5657
inlineCriticalCss?: Record<string, string>,
5758
nonce?: string,
59+
entryPointToBrowserMapping?: AngularAppManifest['entryPointToBrowserMapping'],
5860
): void {
5961
destroyAngularServerApp();
6062

@@ -70,6 +72,7 @@ export function setAngularAppTestingManifest(
7072
setAngularAppManifest({
7173
criticalCssPlans,
7274
nonce,
75+
entryPointToBrowserMapping,
7376
baseHref,
7477
locale,
7578
assets: {

0 commit comments

Comments
 (0)