From 64c52521d052f850aa7ea1aaadfd8a9fcee9c387 Mon Sep 17 00:00:00 2001
From: Alan Agius <alanagius@google.com>
Date: Thu, 26 Sep 2024 20:35:07 +0000
Subject: [PATCH] fix(@angular/ssr): show error when multiple routes are set
 with `RenderMode.AppShell`

This change introduces error handling to ensure that when multiple routes are configured with `RenderMode.AppShell`, an error message is displayed. This prevents misconfiguration and enhances clarity in route management.
---
 packages/angular/ssr/src/routes/ng-routes.ts  | 32 +++++++++++++------
 .../angular/ssr/test/routes/ng-routes_spec.ts | 24 +++++++++++++-
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/packages/angular/ssr/src/routes/ng-routes.ts b/packages/angular/ssr/src/routes/ng-routes.ts
index dd66ee6ba7..a2680d5175 100644
--- a/packages/angular/ssr/src/routes/ng-routes.ts
+++ b/packages/angular/ssr/src/routes/ng-routes.ts
@@ -45,7 +45,7 @@ const VALID_REDIRECT_RESPONSE_CODES = new Set([301, 302, 303, 307, 308]);
  */
 type ServerConfigRouteTreeAdditionalMetadata = Partial<ServerRoute> & {
   /** Indicates if the route has been matched with the Angular router routes. */
-  matched?: boolean;
+  presentInClientRouter?: boolean;
 };
 
 /**
@@ -134,7 +134,7 @@ async function* traverseRoutesConfig(options: {
           continue;
         }
 
-        matchedMetaData.matched = true;
+        matchedMetaData.presentInClientRouter = true;
       }
 
       const metadata: ServerConfigRouteTreeNodeMetadata = {
@@ -142,7 +142,7 @@ async function* traverseRoutesConfig(options: {
         route: currentRoutePath,
       };
 
-      delete metadata.matched;
+      delete metadata.presentInClientRouter;
 
       // Handle redirects
       if (typeof redirectTo === 'string') {
@@ -246,8 +246,8 @@ async function* handleSSGRoute(
     if (!getPrerenderParams) {
       yield {
         error:
-          `The '${stripLeadingSlash(currentRoutePath)}' route uses prerendering and includes parameters, but 'getPrerenderParams' is missing. ` +
-          `Please define 'getPrerenderParams' function for this route in your server routing configuration ` +
+          `The '${stripLeadingSlash(currentRoutePath)}' route uses prerendering and includes parameters, but 'getPrerenderParams' ` +
+          `is missing. Please define 'getPrerenderParams' function for this route in your server routing configuration ` +
           `or specify a different 'renderMode'.`,
       };
 
@@ -442,24 +442,38 @@ export async function getRoutesFromAngularRouterConfig(
         includePrerenderFallbackRoutes,
       });
 
+      let seenAppShellRoute: string | undefined;
       for await (const result of traverseRoutes) {
         if ('error' in result) {
           errors.push(result.error);
         } else {
+          if (result.renderMode === RenderMode.AppShell) {
+            if (seenAppShellRoute !== undefined) {
+              errors.push(
+                `Error: Both '${seenAppShellRoute}' and '${stripLeadingSlash(result.route)}' routes have ` +
+                  `their 'renderMode' set to 'AppShell'. AppShell renderMode should only be assigned to one route. ` +
+                  `Please review your route configurations to ensure that only one route is set to 'RenderMode.AppShell'.`,
+              );
+            }
+
+            seenAppShellRoute = stripLeadingSlash(result.route);
+          }
+
           routesResults.push(result);
         }
       }
 
       if (serverConfigRouteTree) {
-        for (const { route, matched } of serverConfigRouteTree.traverse()) {
-          if (matched || route === '**') {
+        for (const { route, presentInClientRouter } of serverConfigRouteTree.traverse()) {
+          if (presentInClientRouter || route === '**') {
             // Skip if matched or it's the catch-all route.
             continue;
           }
 
           errors.push(
-            `The server route '${route}' does not match any routes defined in the Angular routing configuration. ` +
-              'Please verify and if unneeded remove this route from the server configuration.',
+            `The '${route}' server route does not match any routes defined in the Angular ` +
+              `routing configuration (typically provided as a part of the 'provideRouter' call). ` +
+              'Please make sure that the mentioned server route is present in the Angular routing configuration.',
           );
         }
       }
diff --git a/packages/angular/ssr/test/routes/ng-routes_spec.ts b/packages/angular/ssr/test/routes/ng-routes_spec.ts
index 75d79afc65..82ac362881 100644
--- a/packages/angular/ssr/test/routes/ng-routes_spec.ts
+++ b/packages/angular/ssr/test/routes/ng-routes_spec.ts
@@ -314,7 +314,7 @@ describe('extractRoutesAndCreateRouteTree', () => {
 
     expect(errors).toHaveSize(1);
     expect(errors[0]).toContain(
-      `The server route 'invalid' does not match any routes defined in the Angular routing configuration`,
+      `The 'invalid' server route does not match any routes defined in the Angular routing configuration`,
     );
   });
 
@@ -339,4 +339,26 @@ describe('extractRoutesAndCreateRouteTree', () => {
       `The 'invalid' route does not match any route defined in the server routing configuration`,
     );
   });
+
+  it(`should error when 'RenderMode.AppShell' is used on more than one route`, async () => {
+    setAngularAppTestingManifest(
+      [
+        { path: 'home', component: DummyComponent },
+        { path: 'shell', component: DummyComponent },
+      ],
+      [{ path: '**', renderMode: RenderMode.AppShell }],
+    );
+
+    const { errors } = await extractRoutesAndCreateRouteTree(
+      url,
+      /** manifest */ undefined,
+      /** invokeGetPrerenderParams */ false,
+      /** includePrerenderFallbackRoutes */ false,
+    );
+
+    expect(errors).toHaveSize(1);
+    expect(errors[0]).toContain(
+      `Both 'home' and 'shell' routes have their 'renderMode' set to 'AppShell'.`,
+    );
+  });
 });