feat(common): verify that priority images have preconnect links (#47082)

This commit updates the `NgOptimizedImage` directive to add a logic to detect whether an image, marked with the "priority" attribute has a corresponding `<link rel="preconnect">` tag in the `document.head` for better performance.

PR Close #47082

goldens/public-api/common/errors.md

@@ -17,6 +17,8 @@ export const enum RuntimeErrorCode {
     // (undocumented)
     PARENT_NG_SWITCH_NOT_FOUND = 2000,
     // (undocumented)
+    PRIORITY_IMG_MISSING_PRECONNECT_TAG = 2956,
+    // (undocumented)
     REQUIRED_INPUT_MISSING = 2954,
     // (undocumented)
     UNEXPECTED_INPUT_CHANGE = 2953,

packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts

@@ -12,6 +12,8 @@ import {DOCUMENT} from '../../dom_tokens';
 import {RuntimeErrorCode} from '../../errors';
 
 import {IMAGE_LOADER, ImageLoader} from './image_loaders/image_loader';
+import {PreconnectLinkChecker} from './preconnect_link_checker';
+import {getUrl, imgDirectiveDetails} from './util';
 
 /**
  * When a Base64-encoded image is passed as an input to the `NgOptimizedImage` directive,
@@ -44,10 +46,8 @@ const VALID_DENSITY_DESCRIPTOR_SRCSET = /^((\s*\d(\.\d)?x\s*(,|$)){1,})$/;
  *
  * Based on https://web.dev/lcp/#measure-lcp-in-javascript.
  */
-@Injectable({
-  providedIn: 'root',
-})
-class LCPImageObserver implements OnDestroy {
+@Injectable({providedIn: 'root'})
+export class LCPImageObserver implements OnDestroy {
   // Map of full image URLs -> original `rawSrc` values.
   private images = new Map<string, string>();
   // Keep track of images for which `console.warn` was produced.
@@ -64,11 +64,6 @@ class LCPImageObserver implements OnDestroy {
     }
   }
 
-  // Converts relative image URL to a full URL.
-  private getFullUrl(src: string) {
-    return new URL(src, this.window!.location.href).href;
-  }
-
   // Inits PerformanceObserver and subscribes to LCP events.
   // Based on https://web.dev/lcp/#measure-lcp-in-javascript
   private initPerformanceObserver(): PerformanceObserver {
@@ -103,12 +98,12 @@ class LCPImageObserver implements OnDestroy {
 
   registerImage(rewrittenSrc: string, rawSrc: string) {
     if (!this.observer) return;
-    this.images.set(this.getFullUrl(rewrittenSrc), rawSrc);
+    this.images.set(getUrl(rewrittenSrc, this.window!).href, rawSrc);
   }
 
   unregisterImage(rewrittenSrc: string) {
     if (!this.observer) return;
-    this.images.delete(this.getFullUrl(rewrittenSrc));
+    this.images.delete(getUrl(rewrittenSrc, this.window!).href);
   }
 
   ngOnDestroy() {
@@ -140,6 +135,12 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
   private _height?: number;
   private _priority = false;
 
+  // Calculate the rewritten `src` once and store it.
+  // This is needed to avoid repetitive calculations and make sure the directive cleanup in the
+  // `ngOnDestroy` does not rely on the `IMAGE_LOADER` logic (which in turn can rely on some other
+  // instance that might be already destroyed).
+  private _rewrittenSrc: string|null = null;
+
   /**
    * Name of the source image.
    * Image name will be processed by the image loader and the final URL will be applied as the `src`
@@ -223,7 +224,10 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
       assertRequiredNumberInput(this, this.width, 'width');
       assertRequiredNumberInput(this, this.height, 'height');
       assertValidLoadingInput(this);
-      if (!this.priority) {
+      if (this.priority) {
+        const checker = this.injector.get(PreconnectLinkChecker);
+        checker.check(this.getRewrittenSrc(), this.rawSrc);
+      } else {
         // Monitor whether an image is an LCP element only in case
         // the `priority` attribute is missing. Otherwise, an image
         // has the necessary settings and no extra checks are required.
@@ -265,8 +269,12 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
     // ImageLoaderConfig supports setting a width property. However, we're not setting width here
     // because if the developer uses rendered width instead of intrinsic width in the HTML width
     // attribute, the image requested may be too small for 2x+ screens.
-    const imgConfig = {src: this.rawSrc};
-    return this.imageLoader(imgConfig);
+    if (!this._rewrittenSrc) {
+      const imgConfig = {src: this.rawSrc};
+      // Cache calculated image src to reuse it later in the code.
+      this._rewrittenSrc = this.imageLoader(imgConfig);
+    }
+    return this._rewrittenSrc;
   }
 
   private getRewrittenSrcset(): string {
@@ -280,11 +288,12 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
   }
 
   ngOnDestroy() {
-    if (ngDevMode && !this.priority) {
-      // An image is only registered in dev mode, try to unregister only in dev mode as well.
-      withLCPImageObserver(
-          this.injector,
-          (observer: LCPImageObserver) => observer.unregisterImage(this.getRewrittenSrc()));
+    if (ngDevMode) {
+      if (!this.priority && this._rewrittenSrc !== null) {
+        withLCPImageObserver(
+            this.injector,
+            (observer: LCPImageObserver) => observer.unregisterImage(this._rewrittenSrc!));
+      }
     }
   }
 
@@ -346,11 +355,6 @@ function withLCPImageObserver(
   });
 }
 
-function imgDirectiveDetails(rawSrc: string) {
-  return `The NgOptimizedImage directive (activated on an <img> element ` +
-      `with the \`rawSrc="${rawSrc}"\`)`;
-}
-
 /***** Assert functions *****/
 
 // Verifies that there is no `src` set on a host element.

packages/common/src/directives/ng_optimized_image/preconnect_link_checker.ts

@@ -0,0 +1,84 @@
+/**
+ * @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.io/license
+ */
+
+import {Inject, Injectable, NgZone, ɵformatRuntimeError as formatRuntimeError} from '@angular/core';
+
+import {DOCUMENT} from '../../dom_tokens';
+import {RuntimeErrorCode} from '../../errors';
+
+import {getUrl, imgDirectiveDetails} from './util';
+
+/**
+ * Contains the logic to detect whether an image, marked with the "priority" attribute
+ * has a corresponding `<link rel="preconnect">` tag in the `document.head`.
+ *
+ * Note: this is a dev-mode only class, which should not appear in prod bundles,
+ * thus there is no `ngDevMode` use in the code.
+ */
+@Injectable({providedIn: 'root'})
+export class PreconnectLinkChecker {
+  // Set of <link rel="preconnect"> tags found on this page.
+  // The `null` value indicates that there was no DOM query operation performed.
+  private preconnectLinks: Set<string>|null = null;
+
+  // Keep track of all already seen origin URLs to avoid repeating the same check.
+  private alreadySeen = new Set<string>();
+
+  private window: Window|null = null;
+
+  constructor(@Inject(DOCUMENT) private doc: Document, private ngZone: NgZone) {
+    const win = doc.defaultView;
+    if (typeof win !== 'undefined') {
+      this.window = win;
+    }
+  }
+
+  check(rewrittenSrc: string, rawSrc: string) {
+    if (!this.window) return;
+
+    const imgUrl = getUrl(rewrittenSrc, this.window);
+    if (this.alreadySeen.has(imgUrl.origin)) return;
+
+    // Register this origin as seen, so we don't check it again later.
+    this.alreadySeen.add(imgUrl.origin);
+
+    if (this.preconnectLinks === null) {
+      // Note: we query for preconnect links only *once* and cache the results
+      // for the entire lifespan of an application, since it's unlikely that the
+      // list would change frequently. This allows to make sure there are no
+      // performance implications of making extra DOM lookups for each image.
+      this.preconnectLinks = this.queryPreconnectLinks();
+    }
+
+    if (!this.preconnectLinks.has(imgUrl.origin)) {
+      console.warn(formatRuntimeError(
+          RuntimeErrorCode.PRIORITY_IMG_MISSING_PRECONNECT_TAG,
+          `${imgDirectiveDetails(rawSrc)} has detected that this image ` +
+              `contains the "priority" attribute, but doesn't have a corresponding ` +
+              `preconnect tag. Please add the following element into ` +
+              `the <head> of the document to optimize loading of this image:\n` +
+              `  <link rel="preconnect" href="${imgUrl.origin}">`));
+    }
+  }
+
+  private queryPreconnectLinks(): Set<string> {
+    const preconnectURLs = new Set<string>();
+    const selector = 'link[rel=preconnect]';
+    const links = (this.doc.head?.querySelectorAll(selector) ?? []) as unknown as HTMLLinkElement[];
+    links.forEach(link => {
+      const url = getUrl(link.href, this.window!);
+      preconnectURLs.add(url.origin);
+    });
+    return preconnectURLs;
+  }
+
+  ngOnDestroy() {
+    this.preconnectLinks?.clear();
+    this.alreadySeen.clear();
+  }
+}

packages/common/src/directives/ng_optimized_image/util.ts

@@ -0,0 +1,20 @@
+/**
+ * @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.io/license
+ */
+
+// Converts a string that represents a URL into a URL class instance.
+export function getUrl(src: string, win: Window): URL {
+  const isAbsolute = /^https?:\/\//.test(src);
+  // Don't use a base URL is the URL is absolute.
+  return isAbsolute ? new URL(src) : new URL(src, win.location.href);
+}
+
+// Assembles directive details string, useful for error messages.
+export function imgDirectiveDetails(rawSrc: string) {
+  return `The NgOptimizedImage directive (activated on an <img> element ` +
+      `with the \`rawSrc="${rawSrc}"\`)`;
+}

packages/common/src/errors.ts

@@ -27,4 +27,5 @@ export const enum RuntimeErrorCode {
   UNEXPECTED_INPUT_CHANGE = 2953,
   REQUIRED_INPUT_MISSING = 2954,
   LCP_IMG_MISSING_PRIORITY = 2955,
+  PRIORITY_IMG_MISSING_PRECONNECT_TAG = 2956,
 }

packages/common/test/directives/ng_optimized_image_spec.ts

@@ -7,12 +7,14 @@
  */
 
 import {CommonModule, DOCUMENT} from '@angular/common';
-import {IMAGE_LOADER, ImageLoader, ImageLoaderConfig} from '@angular/common/src/directives/ng_optimized_image/image_loaders/image_loader';
-import {assertValidRawSrcset, NgOptimizedImageModule} from '@angular/common/src/directives/ng_optimized_image/ng_optimized_image';
 import {RuntimeErrorCode} from '@angular/common/src/errors';
-import {Component} from '@angular/core';
+import {Component, Provider} from '@angular/core';
 import {ComponentFixture, TestBed} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
+import {withHead} from '@angular/private/testing';
+
+import {IMAGE_LOADER, ImageLoader, ImageLoaderConfig} from '../../src/directives/ng_optimized_image/image_loaders/image_loader';
+import {assertValidRawSrcset, NgOptimizedImageModule} from '../../src/directives/ng_optimized_image/ng_optimized_image';
 
 describe('Image directive', () => {
   it('should set `loading` and `fetchpriority` attributes before `src`', () => {
@@ -247,7 +249,7 @@ describe('Image directive', () => {
       it('should throw for empty rawSrcSet', () => {
         const imageLoader = (config: ImageLoaderConfig) => {
           const width = config.width ? `-${config.width}` : ``;
-          return `path/${config.src}${width}.png`;
+          return window.location.origin + `/path/${config.src}${width}.png`;
         };
         setupTestingModule({imageLoader});
 
@@ -268,7 +270,7 @@ describe('Image directive', () => {
       it('should throw for invalid rawSrcSet', () => {
         const imageLoader = (config: ImageLoaderConfig) => {
           const width = config.width ? `-${config.width}` : ``;
-          return `path/${config.src}${width}.png`;
+          return window.location.origin + `/path/${config.src}${width}.png`;
         };
         setupTestingModule({imageLoader});
 
@@ -495,6 +497,94 @@ describe('Image directive', () => {
     });
   });
 
+  describe('preconnect detector', () => {
+    it('should log a warning if there is no preconnect link for a priority image',
+       withHead('', () => {
+         setupTestingModule();
+
+         const consoleWarnSpy = spyOn(console, 'warn');
+         const template = '<img rawSrc="a.png" width="100" height="50" priority>';
+         const fixture = createTestComponent(template);
+         fixture.detectChanges();
+
+         expect(consoleWarnSpy.calls.count()).toBe(1);
+         expect(consoleWarnSpy.calls.argsFor(0)[0])
+             .toBe(
+                 'NG02956: The NgOptimizedImage directive (activated on an <img> ' +
+                 'element with the `rawSrc="a.png"`) has detected that this image ' +
+                 'contains the "priority" attribute, but doesn\'t have a corresponding ' +
+                 'preconnect tag. Please add the following element into the <head> of ' +
+                 'the document to optimize loading of this image:' +
+                 '\n  <link rel="preconnect" href="http://localhost">');
+       }));
+
+    it('should not log a warning if there is no preconnect link, but the image is not set as a priority',
+       withHead('', () => {
+         setupTestingModule();
+
+         const consoleWarnSpy = spyOn(console, 'warn');
+         const template = '<img rawSrc="a.png" width="100" height="50">';
+         const fixture = createTestComponent(template);
+         fixture.detectChanges();
+
+         // Expect no warnings in the console.
+         expect(consoleWarnSpy.calls.count()).toBe(0);
+       }));
+
+    it('should log a warning if there is a preconnect, but it doesn\'t match the priority image',
+       withHead('<link rel="preconnect" href="https://localhost:443">', () => {
+         setupTestingModule();
+
+         const consoleWarnSpy = spyOn(console, 'warn');
+         const template = '<img rawSrc="a.png" width="100" height="50" priority>';
+         const fixture = createTestComponent(template);
+         fixture.detectChanges();
+
+         expect(consoleWarnSpy.calls.count()).toBe(1);
+         expect(consoleWarnSpy.calls.argsFor(0)[0])
+             .toBe(
+                 'NG02956: The NgOptimizedImage directive (activated on an ' +
+                 '<img> element with the `rawSrc="a.png"`) has detected that ' +
+                 'this image contains the "priority" attribute, but doesn\'t have ' +
+                 'a corresponding preconnect tag. Please add the following element ' +
+                 'into the <head> of the document to optimize loading of this image:' +
+                 '\n  <link rel="preconnect" href="http://localhost">');
+       }));
+
+    it('should log a warning if there is no matching preconnect link for a priority image, but there is a preload tag',
+       withHead('<link rel="preload" href="http://localhost/a.png" as="image">', () => {
+         setupTestingModule();
+
+         const consoleWarnSpy = spyOn(console, 'warn');
+         const template = '<img rawSrc="a.png" width="100" height="50" priority>';
+         const fixture = createTestComponent(template);
+         fixture.detectChanges();
+
+         expect(consoleWarnSpy.calls.count()).toBe(1);
+         expect(consoleWarnSpy.calls.argsFor(0)[0])
+             .toBe(
+                 'NG02956: The NgOptimizedImage directive (activated on an ' +
+                 '<img> element with the `rawSrc="a.png"`) has detected that ' +
+                 'this image contains the "priority" attribute, but doesn\'t have ' +
+                 'a corresponding preconnect tag. Please add the following element ' +
+                 'into the <head> of the document to optimize loading of this image:' +
+                 '\n  <link rel="preconnect" href="http://localhost">');
+       }));
+
+    it('should not log a warning if there is a matching preconnect link for a priority image (with an extra `/` at the end)',
+       withHead('<link rel="preconnect" href="http://localhost/">', () => {
+         setupTestingModule();
+
+         const consoleWarnSpy = spyOn(console, 'warn');
+         const template = '<img rawSrc="a.png" width="100" height="50" priority>';
+         const fixture = createTestComponent(template);
+         fixture.detectChanges();
+
+         // Expect no warnings in the console.
+         expect(consoleWarnSpy.calls.count()).toBe(0);
+       }));
+  });
+
   describe('loaders', () => {
     it('should set `src` to match `rawSrc` if image loader is not provided', () => {
       setupTestingModule();
@@ -705,8 +795,16 @@ class TestComponent {
 }
 
 function setupTestingModule(config?: {imageLoader: ImageLoader}) {
-  const providers =
-      config?.imageLoader ? [{provide: IMAGE_LOADER, useValue: config?.imageLoader}] : [];
+  const defaultLoader = (config: ImageLoaderConfig) => {
+    const isAbsolute = /^https?:\/\//.test(config.src);
+    return isAbsolute ? config.src : window.location.origin + '/' + config.src;
+  };
+  const loader = config?.imageLoader || defaultLoader;
+  const providers: Provider[] = [
+    {provide: DOCUMENT, useValue: window.document},
+    {provide: IMAGE_LOADER, useValue: loader},
+  ];
+
   TestBed.configureTestingModule({
     declarations: [TestComponent],
     // Note: the `NgOptimizedImage` directive is experimental and is not a part of the

packages/core/test/bundling/image-directive/BUILD.bazel

@@ -7,6 +7,7 @@ ng_module(
     srcs = [
         "e2e/basic/basic.ts",
         "e2e/lcp-check/lcp-check.ts",
+        "e2e/preconnect-check/preconnect-check.ts",
         "index.ts",
         "playground.ts",
     ],