From b380fdd59e368e89908ea915f150cdc9f5a87a7f Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Tue, 12 Jul 2022 09:18:28 -0700 Subject: [PATCH] feat(common): add a density cap for image srcsets (#47082) 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 --- .../ng_optimized_image/ng_optimized_image.ts | 41 +++++++++++++++++-- .../directives/ng_optimized_image_spec.ts | 29 ++++++++++++- 2 files changed, 66 insertions(+), 4 deletions(-) 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 index adf898f6840ce..5b05426e731a7 100644 --- 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 index 91068934d43c7..fa07b424cd885 100644 --- 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 = ` + + `; + 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');