Skip to content

fix(@angular/ssr): avoid preloading unnecessary dynamic bundles #30560

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
22 changes: 5 additions & 17 deletions packages/angular/build/src/utils/server-rendering/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,34 +201,22 @@ function generateLazyLoadedFilesMappings(
metafile: Metafile,
initialFiles: Set<string>,
publicPath = '',
): Record<string, FilesMapping[]> {
const entryPointToBundles: Record<string, FilesMapping[]> = {};
): Record<string, string[]> {
const entryPointToBundles: Record<string, string[]> = {};
for (const [fileName, { entryPoint, exports, imports }] of Object.entries(metafile.outputs)) {
// Skip files that don't have an entryPoint, no exports, or are not .js
if (!entryPoint || exports?.length < 1 || !fileName.endsWith('.js')) {
continue;
}

const importedPaths: FilesMapping[] = [
{
path: `${publicPath}${fileName}`,
dynamicImport: false,
},
];
const importedPaths: string[] = [`${publicPath}${fileName}`];

for (const { kind, external, path } of imports) {
if (
external ||
initialFiles.has(path) ||
(kind !== 'dynamic-import' && kind !== 'import-statement')
) {
if (external || initialFiles.has(path) || kind !== 'import-statement') {
continue;
}

importedPaths.push({
path: `${publicPath}${path}`,
dynamicImport: kind === 'dynamic-import',
});
importedPaths.push(`${publicPath}${path}`);
}

entryPointToBundles[entryPoint] = importedPaths;
Expand Down
11 changes: 3 additions & 8 deletions packages/angular/ssr/src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,21 +123,16 @@ export interface AngularAppManifest {
* Maps entry-point names to their corresponding browser bundles and loading strategies.
*
* - **Key**: The entry-point name, typically the value of `ɵentryName`.
* - **Value**: An array of objects, each representing a browser bundle with:
* - `path`: The filename or URL of the associated JavaScript bundle to preload.
* - `dynamicImport`: A boolean indicating whether the bundle is loaded via a dynamic `import()`.
* If `true`, the bundle is lazily loaded, impacting its preloading behavior.
* - **Value**: A readonly array of JavaScript bundle paths or `undefined` if no bundles are associated.
*
* ### Example
* ```ts
* {
* 'src/app/lazy/lazy.ts': [{ path: 'src/app/lazy/lazy.js', dynamicImport: true }]
* 'src/app/lazy/lazy.ts': ['src/app/lazy/lazy.js']
* }
* ```
*/
readonly entryPointToBrowserMapping?: Readonly<
Record<string, ReadonlyArray<{ path: string; dynamicImport: boolean }> | undefined>
>;
readonly entryPointToBrowserMapping?: Readonly<Record<string, readonly string[] | undefined>>;
}

/**
Expand Down
18 changes: 4 additions & 14 deletions packages/angular/ssr/src/routes/ng-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ async function* handleRoute(options: {

const { redirectTo, loadChildren, loadComponent, children, ɵentryName } = route;
if (ɵentryName && loadComponent) {
appendPreloadToMetadata(ɵentryName, entryPointToBrowserMapping, metadata, true);
appendPreloadToMetadata(ɵentryName, entryPointToBrowserMapping, metadata);
}

if (metadata.renderMode === RenderMode.Prerender) {
Expand Down Expand Up @@ -192,11 +192,7 @@ async function* handleRoute(options: {
// Load and process lazy-loaded child routes
if (loadChildren) {
if (ɵentryName) {
// When using `loadChildren`, the entire feature area (including multiple routes) is loaded.
// As a result, we do not want all dynamic-import dependencies to be preload, because it involves multiple dependencies
// across different child routes. In contrast, `loadComponent` only loads a single component, which allows
// for precise control over preloading, ensuring that the files preloaded are exactly those required for that specific route.
appendPreloadToMetadata(ɵentryName, entryPointToBrowserMapping, metadata, false);
appendPreloadToMetadata(ɵentryName, entryPointToBrowserMapping, metadata);
}

const loadedChildRoutes = await loadChildrenHelper(
Expand Down Expand Up @@ -336,7 +332,6 @@ function appendPreloadToMetadata(
entryName: string,
entryPointToBrowserMapping: EntryPointToBrowserMapping,
metadata: ServerConfigRouteTreeNodeMetadata,
includeDynamicImports: boolean,
): void {
const existingPreloads = metadata.preload ?? [];
if (!entryPointToBrowserMapping || existingPreloads.length >= MODULE_PRELOAD_MAX) {
Expand All @@ -350,13 +345,8 @@ function appendPreloadToMetadata(

// Merge existing preloads with new ones, ensuring uniqueness and limiting the total to the maximum allowed.
const combinedPreloads: Set<string> = new Set(existingPreloads);
for (const { dynamicImport, path } of preload) {
if (dynamicImport && !includeDynamicImports) {
continue;
}

combinedPreloads.add(path);

for (const href of preload) {
combinedPreloads.add(href);
if (combinedPreloads.size === MODULE_PRELOAD_MAX) {
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,15 @@ const RESPONSE_EXPECTS: Record<
matches: [
/<link rel="modulepreload" href="(ssg\.routes-[a-zA-Z0-9]{8}\.js)">/,
/<link rel="modulepreload" href="(ssg-one-[a-zA-Z0-9]{8}\.js)">/,
/<link rel="modulepreload" href="(cross-dep-[a-zA-Z0-9]{8}\.js)">/,
],
notMatches: [/home/, /ssr/, /csr/, /ssg-two/, /ssg\-component/],
notMatches: [/home/, /ssr/, /csr/, /ssg-two/, /ssg\-component/, /cross-dep-/],
},
'/ssg/two': {
matches: [
/<link rel="modulepreload" href="(ssg\.routes-[a-zA-Z0-9]{8}\.js)">/,
/<link rel="modulepreload" href="(ssg-two-[a-zA-Z0-9]{8}\.js)">/,
/<link rel="modulepreload" href="(cross-dep-[a-zA-Z0-9]{8}\.js)">/,
],
notMatches: [/home/, /ssr/, /csr/, /ssg-one/, /ssg\-component/],
notMatches: [/home/, /ssr/, /csr/, /ssg-one/, /ssg\-component/, /cross-dep-/],
},
'/ssr': {
matches: [/<link rel="modulepreload" href="(ssr-[a-zA-Z0-9]{8}\.js)">/],
Expand Down
Loading