refactor(common): Log a warning when the priority attribute of NgOptimizedImage is used too often. (#56669)

JeanMeche · dylhunn · commit cc4cb9608a91 · 2024-06-26T12:43:00.000-07:00
When the DOM content is loaded, Angular will log a warning message if the `priority` attribute is applied to often on `NgOptimizedImage` directive instances. PR Close #56669
diff --git a/goldens/public-api/common/errors.api.md b/goldens/public-api/common/errors.api.md
@@ -37,6 +37,8 @@ export const enum RuntimeErrorCode {
     // (undocumented)
     TOO_MANY_PRELOADED_IMAGES = 2961,
     // (undocumented)
+    TOO_MANY_PRIORITY_ATTRIBUTES = 2966,
+    // (undocumented)
     UNEXPECTED_DEV_MODE_CHECK_IN_PROD_MODE = 2958,
     // (undocumented)
     UNEXPECTED_INPUT_CHANGE = 2953,
diff --git a/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts b/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts
@@ -137,6 +137,18 @@ export const BUILT_IN_LOADERS = [
   netlifyLoaderInfo,
 ];
 
+/**
+ * Threshold for the PRIORITY_TRUE_COUNT
+ */
+const PRIORITY_COUNT_THRESHOLD = 10;
+
+/**
+ * This count is used to log a devMode warning
+ * when the count of directive instances with priority=true
+ * exceeds the threshold PRIORITY_COUNT_THRESHOLD
+ */
+let IMGS_WITH_PRIORITY_ATTR_COUNT = 0;
+
 /**
  * Config options used in rendering placeholder images.
  *
@@ -430,6 +442,14 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
       if (this.priority) {
         const checker = this.injector.get(PreconnectLinkChecker);
         checker.assertPreconnect(this.getRewrittenSrc(), this.ngSrc);
+
+        // This leaves the Angular zone to avoid triggering unnecessary change detection cycles when
+        // document.addEventListener is invoked
+        if (!this.isServer) {
+          ngZone.runOutsideAngular(() => {
+            assetPriorityCountBelowThreshold();
+          });
+        }
       }
     }
     if (this.placeholder) {
@@ -1245,6 +1265,27 @@ function assertNoLoaderParamsWithoutLoader(dir: NgOptimizedImage, imageLoader: I
   }
 }
 
+/**
+ * Warns if the priority attribute is used too often on page load
+ */
+function assetPriorityCountBelowThreshold() {
+  if (IMGS_WITH_PRIORITY_ATTR_COUNT === 0) {
+    document.addEventListener('DOMContentLoaded', (event) => {
+      if (IMGS_WITH_PRIORITY_ATTR_COUNT > PRIORITY_COUNT_THRESHOLD) {
+        console.warn(
+          formatRuntimeError(
+            RuntimeErrorCode.TOO_MANY_PRIORITY_ATTRIBUTES,
+            `NgOptimizedImage: The "priority" attribute is set to true more than ${PRIORITY_COUNT_THRESHOLD} times (${IMGS_WITH_PRIORITY_ATTR_COUNT} times). ` +
+              `Marking too many images as "high" priority can hurt your application's LCP (https://web.dev/lcp).` +
+              `"Priority" should only be set on the image expected to be the page's LCP element.`,
+          ),
+        );
+      }
+    });
+  }
+  IMGS_WITH_PRIORITY_ATTR_COUNT++;
+}
+
 function round(input: number): number | string {
   return Number.isInteger(input) ? input : input.toFixed(2);
 }
diff --git a/packages/common/src/errors.ts b/packages/common/src/errors.ts
@@ -39,4 +39,5 @@ export const enum RuntimeErrorCode {
   MISSING_NECESSARY_LOADER = 2963,
   LCP_IMG_NGSRC_MODIFIED = 2964,
   OVERSIZED_PLACEHOLDER = 2965,
+  TOO_MANY_PRIORITY_ATTRIBUTES = 2966,
 }
diff --git a/packages/common/test/directives/ng_optimized_image_spec.ts b/packages/common/test/directives/ng_optimized_image_spec.ts
@@ -870,6 +870,51 @@ describe('Image directive', () => {
       const img = nativeElement.querySelector('img')!;
       expect(img.getAttribute('fetchpriority')).toBe('auto');
     });
+
+    it('should log a warning if the priority attribute is used too often', async () => {
+      withHead('', () => {
+        const imageLoader = () => {
+          // We need something different from the `localhost` (as we don't want to produce
+          // a preconnect warning for local environments).
+          return 'https://angular.io/assets/images/logos/angular/angular.svg';
+        };
+
+        setupTestingModule({imageLoader});
+
+        // 11 priority attributes, threshold is 10
+        const template = `
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      <img ngSrc="path/img.png" width="150" height="50" priority>
+      `;
+        const consoleWarnSpy = spyOn(console, 'warn');
+
+        const fixture = createTestComponent(template);
+        fixture.detectChanges();
+
+        // We manually fire the event that is listened by the directive
+        // as it won't fire in the context of our unit test
+        document.dispatchEvent(new Event('DOMContentLoaded'));
+
+        if (isBrowser) {
+          expect(consoleWarnSpy.calls.count()).toBe(1);
+          expect(consoleWarnSpy.calls.argsFor(0)[0]).toMatch(
+            new RegExp(`NG0${RuntimeErrorCode.TOO_MANY_PRIORITY_ATTRIBUTES}`),
+          );
+        } else {
+          // The warning is only logged on browsers
+          expect(consoleWarnSpy.calls.count()).toBe(0);
+        }
+      });
+    });
   });
 
   describe('meta data', () => {

Original file line number	Diff line number	Diff line change
`@@ -39,4 +39,5 @@ export const enum RuntimeErrorCode {`
`39`	`39`	`MISSING_NECESSARY_LOADER = 2963,`
`40`	`40`	`LCP_IMG_NGSRC_MODIFIED = 2964,`
`41`	`41`	`OVERSIZED_PLACEHOLDER = 2965,`
	`42`	`+ TOO_MANY_PRIORITY_ATTRIBUTES = 2966,`
`42`	`43`	`}`