Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 44 additions & 7 deletions packages/angular/ssr/src/routes/ng-routes.ts
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. */
presentInClientRouter?: 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.presentInClientRouter = true;
}

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

delete metadata.presentInClientRouter;

// 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,8 +246,8 @@ async function* handleSSGRoute(
if (!getPrerenderParams) {
yield {
error:
`The '${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'.`,
};

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 @@ -433,13 +442,41 @@ 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, presentInClientRouter } of serverConfigRouteTree.traverse()) {
if (presentInClientRouter || route === '**') {
// Skip if matched or it's the catch-all route.
continue;
}

errors.push(
`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.',
);
}
}
} else {
routesResults.push({ route: '', renderMode: RenderMode.Prerender });
}
Expand Down
2 changes: 1 addition & 1 deletion packages/angular/ssr/src/routes/route-tree.ts
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
103 changes: 85 additions & 18 deletions packages/angular/ssr/test/routes/ng-routes_spec.ts
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,89 @@ 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 'invalid' server route 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`,
);
});

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'.`,
);
});
});