Skip to content

Commit

Permalink
fix(@angular/ssr): improve handling of route mismatches between Angul…
Browse files Browse the repository at this point in the history 
…ar server routes and Angular router

This commit resolves an issue where routes defined in the Angular server routing configuration did not match those in the Angular router. Previously, discrepancies between these routes went unnoticed by users. With this update, appropriate error messages are now displayed when mismatches occur, enhancing the developer experience and facilitating easier troubleshooting.
  • Loading branch information
@alan-agius4
alan-agius4 committed Sep 26, 2024
1 parent 8f038de commit 50df631
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 25 deletions.
35 changes: 29 additions & 6 deletions packages/angular/ssr/src/routes/ng-routes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { ServerAssets } from '../assets';
import { Console } from '../console';
import { AngularAppManifest, getAngularAppManifest } from '../manifest';
import { AngularBootstrap, isNgModule } from '../utils/ng';
import { joinUrlParts } from '../utils/url';
import { joinUrlParts, stripLeadingSlash } from '../utils/url';
import { PrerenderFallback, RenderMode, SERVER_ROUTES_CONFIG, ServerRoute } from './route-config';
import { RouteTree, RouteTreeNodeMetadata } from './route-tree';

Expand All @@ -43,7 +43,10 @@ const VALID_REDIRECT_RESPONSE_CODES = new Set([301, 302, 303, 307, 308]);
/**
* Additional metadata for a server configuration route tree.
*/
type ServerConfigRouteTreeAdditionalMetadata = Partial<ServerRoute>;
type ServerConfigRouteTreeAdditionalMetadata = Partial<ServerRoute> & {
/** Indicates if the route has been matched with the Angular router routes. */
matched?: boolean;
};

/**
* Metadata for a server configuration route tree node.
Expand Down Expand Up @@ -124,19 +127,23 @@ async function* traverseRoutesConfig(options: {
if (!matchedMetaData) {
yield {
error:
`The '${currentRoutePath}' route does not match any route defined in the server routing configuration. ` +
`The '${stripLeadingSlash(currentRoutePath)}' route does not match any route defined in the server routing configuration. ` +
'Please ensure this route is added to the server routing configuration.',
};

continue;
}

matchedMetaData.matched = true;
}

const metadata: ServerConfigRouteTreeNodeMetadata = {
...matchedMetaData,
route: currentRoutePath,
};

delete metadata.matched;

// Handle redirects
if (typeof redirectTo === 'string') {
const redirectToResolved = resolveRedirectTo(currentRoutePath, redirectTo);
Expand Down Expand Up @@ -189,7 +196,9 @@ async function* traverseRoutesConfig(options: {
}
}
} catch (error) {
yield { error: `Error processing route '${route.path}': ${(error as Error).message}` };
yield {
error: `Error processing route '${stripLeadingSlash(route.path ?? '')}': ${(error as Error).message}`,
};
}
}
}
Expand Down Expand Up @@ -237,7 +246,7 @@ async function* handleSSGRoute(
if (!getPrerenderParams) {
yield {
error:
`The '${currentRoutePath}' route uses prerendering and includes parameters, but 'getPrerenderParams' is missing. ` +
`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'.`,
};
Expand All @@ -253,7 +262,7 @@ async function* handleSSGRoute(
const value = params[parameterName];
if (typeof value !== 'string') {
throw new Error(
`The 'getPrerenderParams' function defined for the '${currentRoutePath}' route ` +
`The 'getPrerenderParams' function defined for the '${stripLeadingSlash(currentRoutePath)}' route ` +
`returned a non-string value for parameter '${parameterName}'. ` +
`Please make sure the 'getPrerenderParams' function returns values for all parameters ` +
'specified in this route.',
Expand Down Expand Up @@ -440,6 +449,20 @@ export async function getRoutesFromAngularRouterConfig(
routesResults.push(result);
}
}

if (serverConfigRouteTree) {
for (const { route, matched } of serverConfigRouteTree.traverse()) {
if (matched || 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.',
);
}
}
} else {
routesResults.push({ route: '', renderMode: RenderMode.Prerender });
}
Expand Down
2 changes: 1 addition & 1 deletion packages/angular/ssr/src/routes/route-tree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
*
* @param node - The current node to start the traversal from. Defaults to the root node of the tree.
*/
private *traverse(node = this.root): Generator<RouteTreeNodeMetadata & AdditionalMetadata> {
*traverse(node = this.root): Generator<RouteTreeNodeMetadata & AdditionalMetadata> {
if (node.metadata) {
yield node.metadata;
}
Expand Down
81 changes: 63 additions & 18 deletions packages/angular/ssr/test/routes/ng-routes_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,24 +66,6 @@ describe('extractRoutesAndCreateRouteTree', () => {
);
});

it('should handle route not matching server routing configuration', async () => {
setAngularAppTestingManifest(
[
{ path: 'home', component: DummyComponent },
{ path: 'about', component: DummyComponent }, // This route is not in the server configuration
],
[
{ path: 'home', renderMode: RenderMode.Client },
// 'about' route is missing here
],
);

const { errors } = await extractRoutesAndCreateRouteTree(url);
expect(errors[0]).toContain(
`The '/about' route does not match any route defined in the server routing configuration.`,
);
});

describe('when `invokeGetPrerenderParams` is true', () => {
it('should resolve parameterized routes for SSG and add a fallback route if fallback is Server', async () => {
setAngularAppTestingManifest(
Expand Down Expand Up @@ -294,4 +276,67 @@ describe('extractRoutesAndCreateRouteTree', () => {
{ route: '/user/:id/role/:role', renderMode: RenderMode.Client },
]);
});

it(`should not error when a catch-all route didn't match any Angular route.`, async () => {
setAngularAppTestingManifest(
[{ path: 'home', component: DummyComponent }],
[
{ path: 'home', renderMode: RenderMode.Server },
{ path: '**', renderMode: RenderMode.Server },
],
);

const { errors } = await extractRoutesAndCreateRouteTree(
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ false,
/** includePrerenderFallbackRoutes */ false,
);

expect(errors).toHaveSize(0);
});

it('should error when a route is not defined in the server routing configuration', async () => {
setAngularAppTestingManifest(
[{ path: 'home', component: DummyComponent }],
[
{ path: 'home', renderMode: RenderMode.Server },
{ path: 'invalid', renderMode: RenderMode.Server },
],
);

const { errors } = await extractRoutesAndCreateRouteTree(
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ false,
/** includePrerenderFallbackRoutes */ false,
);

expect(errors).toHaveSize(1);
expect(errors[0]).toContain(
`The server route 'invalid' does not match any routes defined in the Angular routing configuration`,
);
});

it('should error when a server route is not defined in the Angular routing configuration', async () => {
setAngularAppTestingManifest(
[
{ path: 'home', component: DummyComponent },
{ path: 'invalid', component: DummyComponent },
],
[{ path: 'home', renderMode: RenderMode.Server }],
);

const { errors } = await extractRoutesAndCreateRouteTree(
url,
/** manifest */ undefined,
/** invokeGetPrerenderParams */ false,
/** includePrerenderFallbackRoutes */ false,
);

expect(errors).toHaveSize(1);
expect(errors[0]).toContain(
`The 'invalid' route does not match any route defined in the server routing configuration`,
);
});
});

0 comments on commit 50df631

Please sign in to comment.