From 43b9a9f3ef35979e141b16de5b19a4b2e388d0b3 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Thu, 30 Jan 2025 10:09:47 +0000 Subject: [PATCH] fix(@angular/ssr): prioritize the first matching route over subsequent ones Ensures that the SSR router gives precedence to the first matching route, addressing the issue where later conflicting routes. This change prevents the incorrect prioritization of routes and ensures the intended route is matched first, aligning routing behavior. Closes: #29539 --- packages/angular/ssr/src/routes/ng-routes.ts | 22 ++++++++---- .../angular/ssr/test/routes/ng-routes_spec.ts | 34 +++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/packages/angular/ssr/src/routes/ng-routes.ts b/packages/angular/ssr/src/routes/ng-routes.ts index 2e104054a814..0f7c270e6830 100644 --- a/packages/angular/ssr/src/routes/ng-routes.ts +++ b/packages/angular/ssr/src/routes/ng-routes.ts @@ -557,7 +557,6 @@ export async function getRoutesFromAngularRouterConfig( // Wait until the application is stable. await applicationRef.whenStable(); - const routesResults: RouteTreeNodeMetadata[] = []; const errors: string[] = []; let baseHref = @@ -581,11 +580,12 @@ export async function getRoutesFromAngularRouterConfig( if (errors.length) { return { baseHref, - routes: routesResults, + routes: [], errors, }; } + const routesResults: RouteTreeNodeMetadata[] = []; if (router.config.length) { // Retrieve all routes from the Angular router configuration. const traverseRoutes = traverseRoutesConfig({ @@ -599,11 +599,19 @@ export async function getRoutesFromAngularRouterConfig( entryPointToBrowserMapping, }); - for await (const result of traverseRoutes) { - if ('error' in result) { - errors.push(result.error); - } else { - routesResults.push(result); + const seenRoutes: Set = new Set(); + for await (const routeMetadata of traverseRoutes) { + if ('error' in routeMetadata) { + errors.push(routeMetadata.error); + continue; + } + + // If a result already exists for the exact same route, subsequent matches should be ignored. + // This aligns with Angular's app router behavior, which prioritizes the first route. + const routePath = routeMetadata.route; + if (!seenRoutes.has(routePath)) { + routesResults.push(routeMetadata); + seenRoutes.add(routePath); } } diff --git a/packages/angular/ssr/test/routes/ng-routes_spec.ts b/packages/angular/ssr/test/routes/ng-routes_spec.ts index 44d246e60a2f..6691f54d41a6 100644 --- a/packages/angular/ssr/test/routes/ng-routes_spec.ts +++ b/packages/angular/ssr/test/routes/ng-routes_spec.ts @@ -570,4 +570,38 @@ describe('extractRoutesAndCreateRouteTree', () => { expect(errors).toHaveSize(0); expect(routeTree.toObject()).toHaveSize(2); }); + + it('should give precedence to the first matching route over subsequent ones', async () => { + setAngularAppTestingManifest( + [ + { + path: '', + children: [ + { path: 'home', component: DummyComponent }, + { path: '**', component: DummyComponent }, + ], + }, + // The following routes should be ignored due to Angular's routing behavior: + // - ['', '**'] and ['**'] are equivalent, and the first match takes precedence. + // - ['', 'home'] and ['home'] are equivalent, and the first match takes precedence. + { + path: 'home', + redirectTo: 'never', + }, + { + path: '**', + redirectTo: 'never', + }, + ], + [{ path: '**', renderMode: RenderMode.Server }], + ); + + const { routeTree, errors } = await extractRoutesAndCreateRouteTree({ url }); + expect(errors).toHaveSize(0); + expect(routeTree.toObject()).toEqual([ + { route: '/', renderMode: RenderMode.Server }, + { route: '/home', renderMode: RenderMode.Server }, + { route: '/**', renderMode: RenderMode.Server }, + ]); + }); });