Skip to content

Commit

Permalink
feat(common): add a density cap for image srcsets (#47082)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kara authored and Pawel Kozlowski committed Aug 16, 2022
1 parent 59ea528 commit b380fdd
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 4 deletions.
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.`);
}
}

Expand Down
29 changes: 28 additions & 1 deletion packages/common/test/directives/ng_optimized_image_spec.ts
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit b380fdd

Please sign in to comment.