Skip to content

fix(@angular/ssr): ensure wildcard RenderMode is applied when no Angular routes are defined #28718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 28, 2024
Merged

fix(@angular/ssr): ensure wildcard RenderMode is applied when no Angular routes are defined #28718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 22 additions & 17 deletions packages/angular/ssr/src/routes/ng-routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,26 +411,26 @@ export async function getRoutesFromAngularRouterConfig(
injector.get(APP_BASE_HREF, null, { optional: true }) ??
injector.get(PlatformLocation).getBaseHrefFromDOM();

if (router.config.length) {
const compiler = injector.get(Compiler);
const compiler = injector.get(Compiler);

const serverRoutesConfig = injector.get(SERVER_ROUTES_CONFIG, null, { optional: true });
let serverConfigRouteTree: RouteTree<ServerConfigRouteTreeAdditionalMetadata> | undefined;
const serverRoutesConfig = injector.get(SERVER_ROUTES_CONFIG, null, { optional: true });
let serverConfigRouteTree: RouteTree<ServerConfigRouteTreeAdditionalMetadata> | undefined;

if (serverRoutesConfig) {
const result = buildServerConfigRouteTree(serverRoutesConfig);
serverConfigRouteTree = result.serverConfigRouteTree;
errors.push(...result.errors);
}
if (serverRoutesConfig) {
const result = buildServerConfigRouteTree(serverRoutesConfig);
serverConfigRouteTree = result.serverConfigRouteTree;
errors.push(...result.errors);
}

if (errors.length) {
return {
baseHref,
routes: routesResults,
errors,
};
}
if (errors.length) {
return {
baseHref,
routes: routesResults,
errors,
};
}

if (router.config.length) {
// Retrieve all routes from the Angular router configuration.
const traverseRoutes = traverseRoutesConfig({
routes: router.config,
Expand Down Expand Up @@ -478,7 +478,12 @@ export async function getRoutesFromAngularRouterConfig(
}
}
} else {
routesResults.push({ route: '', renderMode: RenderMode.Prerender });
const renderMode = serverConfigRouteTree?.match('')?.renderMode ?? RenderMode.Prerender;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgp1130, I'd prefer if in this case it was a hard error both the old and new API. But this would require us to generate an additional component and add route when doing ng-new.

Is this something that we'll be willing to do?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the case where export const SERVER_ROUTES = [];? You think that should be a hard error?

I can see where you're coming from. This has bugged me in the past because we do the same thing for client-side routes and it means the <router-outlet> is a somewhat confusing no-op in ng new and it's not clear how to add a route, which leads to devs using component instead of loadComponent and deoptimizing their apps.

Could we generate { path: '/**', renderMode: RenderMode.Prerender } by default in SSR? Would that avoid the need for a defined route? Or do we need to add a route in the app router to get this to work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the case where export const SERVER_ROUTES = [];? You think that should be a hard error?

I was referring to Angular Router routes.

We do generate { path: '/**', renderMode: RenderMode.Prerender } already but we do not generate any route in the Angular router.


routesResults.push({
route: '',
renderMode,
});
}

return {
Expand Down
14 changes: 14 additions & 0 deletions packages/angular/ssr/test/routes/ng-routes_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,4 +361,18 @@ describe('extractRoutesAndCreateRouteTree', () => {
`Both 'home' and 'shell' routes have their 'renderMode' set to 'AppShell'.`,
);
});

it('should apply RenderMode matching the wildcard when no Angular routes are defined', async () => {
setAngularAppTestingManifest([], [{ path: '**', renderMode: RenderMode.Server }]);

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

expect(errors).toHaveSize(0);
expect(routeTree.toObject()).toEqual([{ route: '/', renderMode: RenderMode.Server }]);
});
});