feat(common): add a density cap for image srcsets (#47082)

kara · Pawel Kozlowski · commit b380fdd59e36 · 2022-08-16T17:36:54.000Z
With this commit, the NgOptimizedImage directive will throw a runtime error if it detects that one of the density descriptors in rawSrcset is higher than 3x. It's generally not recommended to use density descriptors higher than ~2, as it causes image to download at very large sizes on mobile screens (thus slowing down LCP). The density max is set conservatively to 3 in case apps expect users to zoom in. In future commits, we may want to throw even at densities > than 2 and provide a configuration override for the zoom case. PR Close #47082
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
@@ -36,6 +36,19 @@ const VALID_WIDTH_DESCRIPTOR_SRCSET = /^((\s*\d+w\s*(,|$)){1,})$/;
  */
 const VALID_DENSITY_DESCRIPTOR_SRCSET = /^((\s*\d(\.\d)?x\s*(,|$)){1,})$/;
 
+/**
+ * Srcset values with a density descriptor higher than this value will actively
+ * throw an error. Such densities are not permitted as they cause image sizes
+ * to be unreasonably large and slow down LCP.
+ */
+export const ABSOLUTE_SRCSET_DENSITY_CAP = 3;
+
+/**
+ * Used only in error message text to communicate best practices, as we will
+ * only throw based on the slightly more conservative ABSOLUTE_SRCSET_DENSITY_CAP.
+ */
+export const RECOMMENDED_SRCSET_DENSITY_CAP = 2;
+
 /**
  * Used to determine whether two aspect ratios are similar in value.
  */
@@ -359,15 +372,37 @@ function assertNonEmptyInput(name: string, value: unknown) {
 export function assertValidRawSrcset(value: unknown) {
   if (value == null) return;
   assertNonEmptyInput('rawSrcset', value);
-  const isValidSrcset = VALID_WIDTH_DESCRIPTOR_SRCSET.test(value as string) ||
-      VALID_DENSITY_DESCRIPTOR_SRCSET.test(value as string);
+  const stringVal = value as string;
+  const isValidWidthDescriptor = VALID_WIDTH_DESCRIPTOR_SRCSET.test(stringVal);
+  const isValidDensityDescriptor = VALID_DENSITY_DESCRIPTOR_SRCSET.test(stringVal);
 
+  if (isValidDensityDescriptor) {
+    assertUnderDensityCap(stringVal);
+  }
+
+  const isValidSrcset = isValidWidthDescriptor || isValidDensityDescriptor;
   if (!isValidSrcset) {
     throw new RuntimeError(
         RuntimeErrorCode.INVALID_INPUT,
         `The NgOptimizedImage directive has detected that the \`rawSrcset\` has an invalid value: ` +
             `expecting width descriptors (e.g. "100w, 200w") or density descriptors (e.g. "1x, 2x"), ` +
-            `but got: \`${value}\``);
+            `but got: \`${stringVal}\``);
+  }
+}
+
+function assertUnderDensityCap(value: string) {
+  const underDensityCap =
+      value.split(',').every(num => num === '' || parseFloat(num) <= ABSOLUTE_SRCSET_DENSITY_CAP);
+  if (!underDensityCap) {
+    throw new RuntimeError(
+        RuntimeErrorCode.INVALID_INPUT,
+        `The NgOptimizedImage directive has detected that the \`rawSrcset\` contains an unsupported image density:` +
+            `\`${value}\`. NgOptimizedImage generally recommends a max image density of ` +
+            `${RECOMMENDED_SRCSET_DENSITY_CAP}x but supports image densities up to ` +
+            `${ABSOLUTE_SRCSET_DENSITY_CAP}x. The human eye cannot distinguish between image densities ` +
+            `greater than ${RECOMMENDED_SRCSET_DENSITY_CAP}x - which makes them unnecessary for ` +
+            `most use cases. Images that will be pinch-zoomed are typically the primary use case for` +
+            `${ABSOLUTE_SRCSET_DENSITY_CAP}x images. Please remove the high density descriptor and try again.`);
   }
 }
 
diff --git a/packages/common/test/directives/ng_optimized_image_spec.ts b/packages/common/test/directives/ng_optimized_image_spec.ts
@@ -14,7 +14,7 @@ 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';
+import {ABSOLUTE_SRCSET_DENSITY_CAP, assertValidRawSrcset, NgOptimizedImageModule, RECOMMENDED_SRCSET_DENSITY_CAP} from '../../src/directives/ng_optimized_image/ng_optimized_image';
 import {PRECONNECT_CHECK_BLOCKLIST} from '../../src/directives/ng_optimized_image/preconnect_link_checker';
 
 describe('Image directive', () => {
@@ -322,6 +322,33 @@ describe('Image directive', () => {
                 `but got: \`100q, 200q\``);
       });
 
+      it('should throw if rawSrcset exceeds the density cap', () => {
+        const imageLoader = (config: ImageLoaderConfig) => {
+          const width = config.width ? `-${config.width}` : ``;
+          return window.location.origin + `/path/${config.src}${width}.png`;
+        };
+        setupTestingModule({imageLoader});
+
+        const template = `
+            <img rawSrc="img" rawSrcset="1x, 2x, 3x, 4x, 5x" width="100" height="50">
+          `;
+        expect(() => {
+          const fixture = createTestComponent(template);
+          fixture.detectChanges();
+        })
+            .toThrowError(
+                `NG0${
+                    RuntimeErrorCode
+                        .INVALID_INPUT}: The NgOptimizedImage directive has detected that the \`rawSrcset\` contains an unsupported image density:` +
+                `\`1x, 2x, 3x, 4x, 5x\`. NgOptimizedImage generally recommends a max image density of ` +
+                `${RECOMMENDED_SRCSET_DENSITY_CAP}x but supports image densities up to ` +
+                `${ABSOLUTE_SRCSET_DENSITY_CAP}x. The human eye cannot distinguish between image densities ` +
+                `greater than ${
+                    RECOMMENDED_SRCSET_DENSITY_CAP}x - which makes them unnecessary for ` +
+                `most use cases. Images that will be pinch-zoomed are typically the primary use case for` +
+                `${ABSOLUTE_SRCSET_DENSITY_CAP}x images. Please remove the high density descriptor and try again.`);
+      });
+
       it('should throw if width srcset is missing a comma', () => {
         expect(() => {
           assertValidRawSrcset('100w 200w');
