refactor(@angular/ssr): bundle Critters

.prettierignore

@@ -12,6 +12,6 @@
 /CONTRIBUTING.md
 .yarn/
 dist/
-third_party/
+third_party/github.com/
 /tests/legacy-cli/e2e/assets/
-/tools/test/*.json
+/tools/test/*.json

package.json

@@ -83,6 +83,7 @@
     "@bazel/buildifier": "7.1.2",
     "@bazel/concatjs": "patch:@bazel/concatjs@npm%3A5.8.1#~/.yarn/patches/@bazel-concatjs-npm-5.8.1-1bf81df846.patch",
     "@bazel/jasmine": "patch:@bazel/jasmine@npm%3A5.8.1#~/.yarn/patches/@bazel-jasmine-npm-5.8.1-3370fee155.patch",
+    "@bazel/runfiles": "^5.8.1",
     "@discoveryjs/json-ext": "0.6.1",
     "@inquirer/confirm": "3.1.22",
     "@inquirer/prompts": "5.3.8",
@@ -199,6 +200,7 @@
     "tslib": "2.6.3",
     "typescript": "5.5.4",
     "undici": "6.19.8",
+    "unenv": "^1.10.0",
     "verdaccio": "5.32.1",
     "verdaccio-auth-memory": "^10.0.0",
     "vite": "5.4.2",

packages/angular/build/BUILD.bazel

@@ -79,6 +79,7 @@ ts_library(
         "@npm//browserslist",
         "@npm//critters",
         "@npm//esbuild",
+        "@npm//esbuild-wasm",
         "@npm//fast-glob",
         "@npm//https-proxy-agent",
         "@npm//listr2",

packages/angular/build/src/typings.d.ts

@@ -0,0 +1,19 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+// The `bundled_critters` causes issues with module mappings in Bazel,
+// leading to unexpected behavior with esbuild. Specifically, the problem occurs
+// when esbuild resolves to a different module or version than expected, due to
+// how Bazel handles module mappings.
+//
+// This change aims to resolve esbuild types correctly and maintain consistency
+// in the Bazel build process.
+
+declare module 'esbuild' {
+  export * from 'esbuild-wasm';
+}

packages/angular/build/src/utils/index-file/inline-critical-css.ts

@@ -20,36 +20,46 @@ const MEDIA_SET_HANDLER_PATTERN = /^this\.media=["'](.*)["'];?$/;
 const CSP_MEDIA_ATTR = 'ngCspMedia';
 
 /**
- * Script text used to change the media value of the link tags.
+ * Script that dynamically updates the `media` attribute of `<link>` tags based on a custom attribute (`CSP_MEDIA_ATTR`).
  *
  * NOTE:
  * We do not use `document.querySelectorAll('link').forEach((s) => s.addEventListener('load', ...)`
- * because this does not always fire on Chome.
+ * because load events are not always triggered reliably on Chrome.
  * See: https://github.com/angular/angular-cli/issues/26932 and https://crbug.com/1521256
+ *
+ * The script:
+ * - Ensures the event target is a `<link>` tag with the `CSP_MEDIA_ATTR` attribute.
+ * - Updates the `media` attribute with the value of `CSP_MEDIA_ATTR` and then removes the attribute.
+ * - Removes the event listener when all relevant `<link>` tags have been processed.
+ * - Uses event capturing (the `true` parameter) since load events do not bubble up the DOM.
  */
-const LINK_LOAD_SCRIPT_CONTENT = [
-  '(() => {',
-  `  const CSP_MEDIA_ATTR = '${CSP_MEDIA_ATTR}';`,
-  '  const documentElement = document.documentElement;',
-  '  const listener = (e) => {',
-  '    const target = e.target;',
-  `    if (!target || target.tagName !== 'LINK' || !target.hasAttribute(CSP_MEDIA_ATTR)) {`,
-  '     return;',
-  '    }',
-
-  '    target.media = target.getAttribute(CSP_MEDIA_ATTR);',
-  '    target.removeAttribute(CSP_MEDIA_ATTR);',
-
-  // Remove onload listener when there are no longer styles that need to be loaded.
-  '    if (!document.head.querySelector(`link[${CSP_MEDIA_ATTR}]`)) {',
-  `      documentElement.removeEventListener('load', listener);`,
-  '    }',
-  '  };',
-
-  //  We use an event with capturing (the true parameter) because load events don't bubble.
-  `  documentElement.addEventListener('load', listener, true);`,
-  '})();',
-].join('\n');
+const LINK_LOAD_SCRIPT_CONTENT = `
+(() => {
+  const CSP_MEDIA_ATTR = '${CSP_MEDIA_ATTR}';
+  const documentElement = document.documentElement;
+
+  // Listener for load events on link tags.
+  const listener = (e) => {
+    const target = e.target;
+    if (
+      !target ||
+      target.tagName !== 'LINK' ||
+      !target.hasAttribute(CSP_MEDIA_ATTR)
+    ) {
+      return;
+    }
+
+    target.media = target.getAttribute(CSP_MEDIA_ATTR);
+    target.removeAttribute(CSP_MEDIA_ATTR);
+
+    if (!document.head.querySelector(\`link[\${CSP_MEDIA_ATTR}]\`)) {
+      documentElement.removeEventListener('load', listener);
+    }
+  };
+
+  documentElement.addEventListener('load', listener, true);
+})();
+`.trim();
 
 export interface InlineCriticalCssProcessOptions {
   outputPath: string;
@@ -85,22 +95,22 @@ interface PartialDocument {
   querySelector(selector: string): PartialHTMLElement | null;
 }
 
-/** Signature of the `Critters.embedLinkedStylesheet` method. */
-type EmbedLinkedStylesheetFn = (
-  link: PartialHTMLElement,
-  document: PartialDocument,
-) => Promise<unknown>;
+/* eslint-disable @typescript-eslint/no-unsafe-declaration-merging */
 
-class CrittersExtended extends Critters {
+// We use Typescript declaration merging because `embedLinkedStylesheet` it's not declared in
+// the `Critters` types which means that we can't call the `super` implementation.
+interface CrittersBase {
+  embedLinkedStylesheet(link: PartialHTMLElement, document: PartialDocument): Promise<unknown>;
+}
+class CrittersBase extends Critters {}
+/* eslint-enable @typescript-eslint/no-unsafe-declaration-merging */
+
+class CrittersExtended extends CrittersBase {
   readonly warnings: string[] = [];
   readonly errors: string[] = [];
-  private initialEmbedLinkedStylesheet: EmbedLinkedStylesheetFn;
   private addedCspScriptsDocuments = new WeakSet<PartialDocument>();
   private documentNonces = new WeakMap<PartialDocument, string | null>();
 
-  // Inherited from `Critters`, but not exposed in the typings.
-  protected declare embedLinkedStylesheet: EmbedLinkedStylesheetFn;
-
   constructor(
     private readonly optionsExtended: InlineCriticalCssProcessorOptions &
       InlineCriticalCssProcessOptions,
@@ -119,17 +129,11 @@ class CrittersExtended extends Critters {
       reduceInlineStyles: false,
       mergeStylesheets: false,
       // Note: if `preload` changes to anything other than `media`, the logic in
-      // `embedLinkedStylesheetOverride` will have to be updated.
+      // `embedLinkedStylesheet` will have to be updated.
       preload: 'media',
       noscriptFallback: true,
       inlineFonts: true,
     });
-
-    // We can't use inheritance to override `embedLinkedStylesheet`, because it's not declared in
-    // the `Critters` .d.ts which means that we can't call the `super` implementation. TS doesn't
-    // allow for `super` to be cast to a different type.
-    this.initialEmbedLinkedStylesheet = this.embedLinkedStylesheet;
-    this.embedLinkedStylesheet = this.embedLinkedStylesheetOverride;
   }
 
   public override readFile(path: string): Promise<string> {
@@ -142,7 +146,10 @@ class CrittersExtended extends Critters {
    * Override of the Critters `embedLinkedStylesheet` method
    * that makes it work with Angular's CSP APIs.
    */
-  private embedLinkedStylesheetOverride: EmbedLinkedStylesheetFn = async (link, document) => {
+  override async embedLinkedStylesheet(
+    link: PartialHTMLElement,
+    document: PartialDocument,
+  ): Promise<unknown> {
     if (link.getAttribute('media') === 'print' && link.next?.name === 'noscript') {
       // Workaround for https://github.com/GoogleChromeLabs/critters/issues/64
       // NB: this is only needed for the webpack based builders.
@@ -154,7 +161,7 @@ class CrittersExtended extends Critters {
       }
     }
 
-    const returnValue = await this.initialEmbedLinkedStylesheet(link, document);
+    const returnValue = await super.embedLinkedStylesheet(link, document);
     const cspNonce = this.findCspNonce(document);
 
     if (cspNonce) {
@@ -182,7 +189,7 @@ class CrittersExtended extends Critters {
     }
 
     return returnValue;
-  };
+  }
 
   /**
    * Finds the CSP nonce for a specific document.

packages/angular/ssr/BUILD.bazel

@@ -18,12 +18,12 @@ ts_library(
     ),
     module_name = "@angular/ssr",
     deps = [
+        "//packages/angular/ssr/third_party/critters:bundled_critters_lib",
         "@npm//@angular/common",
         "@npm//@angular/core",
         "@npm//@angular/platform-server",
         "@npm//@angular/router",
         "@npm//@types/node",
-        "@npm//critters",
     ],
 )
 
@@ -32,8 +32,15 @@ ng_package(
     package_name = "@angular/ssr",
     srcs = [
         ":package.json",
+        "//packages/angular/ssr/third_party/critters:bundled_critters_lib",
+    ],
+    externals = [
+        "express",
+        "../../third_party/critters",
+    ],
+    nested_packages = [
+        "//packages/angular/ssr/schematics:npm_package",
     ],
-    nested_packages = ["//packages/angular/ssr/schematics:npm_package"],
     tags = ["release-package"],
     deps = [
         ":ssr",

packages/angular/ssr/package.json

@@ -13,7 +13,6 @@
     "save": "dependencies"
   },
   "dependencies": {
-    "critters": "0.0.24",
     "tslib": "^2.3.0"
   },
   "peerDependencies": {
@@ -29,6 +28,7 @@
     "@angular/platform-browser": "19.0.0-next.1",
     "@angular/platform-server": "19.0.0-next.1",
     "@angular/router": "19.0.0-next.1",
+    "@bazel/runfiles": "^5.8.1",
     "zone.js": "^0.15.0"
   },
   "schematics": "./schematics/collection.json",

packages/angular/ssr/src/app.ts

@@ -14,6 +14,7 @@ import { Hooks } from './hooks';
 import { getAngularAppManifest } from './manifest';
 import { ServerRouter } from './routes/router';
 import { REQUEST, REQUEST_CONTEXT, RESPONSE_INIT } from './tokens';
+import { InlineCriticalCssProcessor } from './utils/inline-critical-css';
 import { renderAngular } from './utils/ng';
 
 /**
@@ -52,6 +53,11 @@ export class AngularServerApp {
    */
   private router: ServerRouter | undefined;
 
+  /**
+   * The `inlineCriticalCssProcessor` is responsible for handling critical CSS inlining.
+   */
+  private inlineCriticalCssProcessor: InlineCriticalCssProcessor | undefined;
+
   /**
    * Renders a response for the given HTTP request using the server application.
    *
@@ -177,10 +183,20 @@ export class AngularServerApp {
       html = await hooks.run('html:transform:pre', { html });
     }
 
-    return new Response(
-      await renderAngular(html, manifest.bootstrap(), new URL(request.url), platformProviders),
-      responseInit,
-    );
+    html = await renderAngular(html, manifest.bootstrap(), new URL(request.url), platformProviders);
+
+    if (manifest.inlineCriticalCss) {
+      // Optionally inline critical CSS.
+      this.inlineCriticalCssProcessor ??= new InlineCriticalCssProcessor((path: string) => {
+        const fileName = path.split('/').pop() ?? path;
+
+        return this.assets.getServerAsset(fileName);
+      });
+
+      html = await this.inlineCriticalCssProcessor.process(html);
+    }
+
+    return new Response(html, responseInit);
   }
 }
 

packages/angular/ssr/src/common-engine/common-engine.ts

@@ -11,7 +11,7 @@ import { renderApplication, renderModule, ɵSERVER_CONTEXT } from '@angular/plat
 import * as fs from 'node:fs';
 import { dirname, join, normalize, resolve } from 'node:path';
 import { URL } from 'node:url';
-import { InlineCriticalCssProcessor, InlineCriticalCssResult } from './inline-css-processor';
+import { CommonEngineInlineCriticalCssProcessor } from './inline-css-processor';
 import {
   noopRunMethodAndMeasurePerf,
   printPerformanceLogs,
@@ -55,14 +55,10 @@ export interface CommonEngineRenderOptions {
 
 export class CommonEngine {
   private readonly templateCache = new Map<string, string>();
-  private readonly inlineCriticalCssProcessor: InlineCriticalCssProcessor;
+  private readonly inlineCriticalCssProcessor = new CommonEngineInlineCriticalCssProcessor();
   private readonly pageIsSSG = new Map<string, boolean>();
 
-  constructor(private options?: CommonEngineOptions) {
-    this.inlineCriticalCssProcessor = new InlineCriticalCssProcessor({
-      minify: false,
-    });
-  }
+  constructor(private options?: CommonEngineOptions) {}
 
   /**
    * Render an HTML document for a specific URL with specified
@@ -81,17 +77,12 @@ export class CommonEngine {
       html = await runMethod('Render Page', () => this.renderApplication(opts));
 
       if (opts.inlineCriticalCss !== false) {
-        const { content, errors, warnings } = await runMethod('Inline Critical CSS', () =>
+        const content = await runMethod('Inline Critical CSS', () =>
           // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
           this.inlineCriticalCss(html!, opts),
         );
 
         html = content;
-
-        // eslint-disable-next-line no-console
-        warnings?.forEach((m) => console.warn(m));
-        // eslint-disable-next-line no-console
-        errors?.forEach((m) => console.error(m));
       }
     }
 
@@ -102,13 +93,11 @@ export class CommonEngine {
     return html;
   }
 
-  private inlineCriticalCss(
-    html: string,
-    opts: CommonEngineRenderOptions,
-  ): Promise<InlineCriticalCssResult> {
-    return this.inlineCriticalCssProcessor.process(html, {
-      outputPath: opts.publicPath ?? (opts.documentFilePath ? dirname(opts.documentFilePath) : ''),
-    });
+  private inlineCriticalCss(html: string, opts: CommonEngineRenderOptions): Promise<string> {
+    const outputPath =
+      opts.publicPath ?? (opts.documentFilePath ? dirname(opts.documentFilePath) : '');
+
+    return this.inlineCriticalCssProcessor.process(html, outputPath);
   }
 
   private async retrieveSSGPage(opts: CommonEngineRenderOptions): Promise<string | undefined> {