From 2e0d09354f2456098eada65a11c6a88abdeaf80b Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Tue, 3 May 2022 18:06:20 -0700 Subject: [PATCH] refactor(common): throw an error if `NgOptimizeImage` inputs change (#47082) This commit updates the `NgOptimizeImage` directive to add asserts to make sure no inputs are changed after directive initialization. PR Close #47082 --- goldens/public-api/common/errors.md | 2 + .../src/directives/ng_optimized_image.ts | 59 ++++++++++++++++-- packages/common/src/errors.ts | 1 + .../directives/ng_optimized_image_spec.ts | 62 +++++++++++++++++++ 4 files changed, 120 insertions(+), 4 deletions(-) diff --git a/goldens/public-api/common/errors.md b/goldens/public-api/common/errors.md index ba2928599a0..9d3b1a9e8df 100644 --- a/goldens/public-api/common/errors.md +++ b/goldens/public-api/common/errors.md @@ -15,6 +15,8 @@ export const enum RuntimeErrorCode { // (undocumented) PARENT_NG_SWITCH_NOT_FOUND = 2000, // (undocumented) + UNEXPECTED_INPUT_CHANGE = 2952, + // (undocumented) UNEXPECTED_SRC_ATTR = 2950 } diff --git a/packages/common/src/directives/ng_optimized_image.ts b/packages/common/src/directives/ng_optimized_image.ts index 7629cc64b53..05a7a29802a 100644 --- a/packages/common/src/directives/ng_optimized_image.ts +++ b/packages/common/src/directives/ng_optimized_image.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive, ElementRef, Inject, InjectionToken, Input, NgModule, OnInit, Renderer2, ɵRuntimeError as RuntimeError} from '@angular/core'; +import {Directive, ElementRef, Inject, InjectionToken, Input, NgModule, OnChanges, OnInit, Renderer2, SimpleChanges, ɵRuntimeError as RuntimeError} from '@angular/core'; import {RuntimeErrorCode} from '../errors'; @@ -63,7 +63,7 @@ export const IMAGE_LOADER = new InjectionToken('ImageLoader', { standalone: true, selector: 'img[rawSrc]', }) -export class NgOptimizedImage implements OnInit { +export class NgOptimizedImage implements OnInit, OnChanges { constructor( @Inject(IMAGE_LOADER) private imageLoader: ImageLoader, private renderer: Renderer2, private imgElement: ElementRef) {} @@ -104,6 +104,9 @@ export class NgOptimizedImage implements OnInit { return this._height; } + /** + * Indicates whether this image should have a high priority. + */ @Input() set priority(value: string|boolean|undefined) { this._priority = inputToBoolean(value); @@ -116,15 +119,23 @@ export class NgOptimizedImage implements OnInit { * Get a value of the `src` if it's set on a host element. * This input is needed to verify that there are no `src` and `rawSrc` provided * at the same time (thus causing an ambiguity on which src to use). + * @internal */ @Input() src?: string; ngOnInit() { if (ngDevMode) { - assertExistingSrc(this); + assertValidRawSrc(this.rawSrc); + assertNoConflictingSrc(this); assertNotBase64Image(this); assertNotBlobURL(this); } + } + + ngOnChanges(changes: SimpleChanges) { + if (ngDevMode) { + assertNoPostInitInputChange(this, changes, ['rawSrc', 'width', 'height', 'priority']); + } this.setHostAttribute('loading', this.getLoadingBehavior()); this.setHostAttribute('fetchpriority', this.getFetchPriority()); // The `src` attribute should be set last since other attributes @@ -175,7 +186,7 @@ function imgDirectiveDetails(dir: NgOptimizedImage) { /***** Assert functions *****/ // Verifies that there is no `src` set on a host element. -function assertExistingSrc(dir: NgOptimizedImage) { +function assertNoConflictingSrc(dir: NgOptimizedImage) { if (dir.src) { throw new RuntimeError( RuntimeErrorCode.UNEXPECTED_SRC_ATTR, @@ -215,3 +226,43 @@ function assertNotBlobURL(dir: NgOptimizedImage) { `for this element.`); } } + +// Verifies that the `rawSrc` is set to a non-empty string. +function assertValidRawSrc(value: unknown) { + const isString = typeof value === 'string'; + const isEmptyString = isString && value.trim() === ''; + if (!isString || isEmptyString) { + const extraMessage = isEmptyString ? ' (empty string)' : ''; + throw new RuntimeError( + RuntimeErrorCode.INVALID_INPUT, + `The NgOptimizedImage directive detected that the \`rawSrc\` has invalid value: ` + + `expecting a non-empty string, but got: \`${value}\`${extraMessage}.`); + } +} + +// Creates a `RuntimeError` instance to represent a situation when an input is set after +// the directive has initialized. +function postInitInputChangeError(dir: NgOptimizedImage, inputName: string): {} { + return new RuntimeError( + RuntimeErrorCode.UNEXPECTED_INPUT_CHANGE, + `${imgDirectiveDetails(dir)} detected that the \`${inputName}\` is updated after the ` + + `initialization. The NgOptimizedImage directive will not react to this input change.`); +} + +// Verify that none of the listed inputs has changed. +function assertNoPostInitInputChange( + dir: NgOptimizedImage, changes: SimpleChanges, inputs: string[]) { + inputs.forEach(input => { + const isUpdated = changes.hasOwnProperty(input); + if (isUpdated && !changes[input].isFirstChange()) { + if (input === 'rawSrc') { + // When the `rawSrc` input changes, we detect that only in the + // `ngOnChanges` hook, thus the `rawSrc` is already set. We use + // `rawSrc` in the error message, so we use a previous value, but + // not the updated one in it. + dir = {rawSrc: changes[input].previousValue} as NgOptimizedImage; + } + throw postInitInputChangeError(dir, input); + } + }); +} diff --git a/packages/common/src/errors.ts b/packages/common/src/errors.ts index dff334c4e0f..a1726ebc0c6 100644 --- a/packages/common/src/errors.ts +++ b/packages/common/src/errors.ts @@ -23,4 +23,5 @@ export const enum RuntimeErrorCode { // Image directive errors UNEXPECTED_SRC_ATTR = 2950, INVALID_INPUT = 2951, + UNEXPECTED_INPUT_CHANGE = 2952, } diff --git a/packages/common/test/directives/ng_optimized_image_spec.ts b/packages/common/test/directives/ng_optimized_image_spec.ts index 0e97c4853fd..4873cf7997a 100644 --- a/packages/common/test/directives/ng_optimized_image_spec.ts +++ b/packages/common/test/directives/ng_optimized_image_spec.ts @@ -147,6 +147,64 @@ describe('Image directive', () => { done(); }); + + it('should throw if `rawSrc` value is not provided', () => { + setupTestingModule(); + + const template = ''; + expect(() => { + const fixture = createTestComponent(template); + fixture.detectChanges(); + }) + .toThrowError( + 'NG02951: The NgOptimizedImage directive detected that the `rawSrc` ' + + 'has invalid value: expecting a non-empty string, but got: `` (empty string).'); + }); + + it('should throw if `rawSrc` value is set to an empty string', () => { + setupTestingModule(); + + const template = ''; + expect(() => { + const fixture = createTestComponent(template); + fixture.detectChanges(); + }) + .toThrowError( + 'NG02951: The NgOptimizedImage directive detected that the `rawSrc` ' + + 'has invalid value: expecting a non-empty string, but got: ` ` (empty string).'); + }); + + const inputs = [ + ['rawSrc', 'new-img.png'], // + ['width', 10], // + ['height', 20], // + ['priority', true] + ]; + inputs.forEach(([inputName, value]) => { + it(`should throw if inputs got changed after directive init (the \`${inputName}\` input)`, + () => { + setupTestingModule(); + + const template = + ''; + expect(() => { + // Initial render + const fixture = createTestComponent(template); + fixture.detectChanges(); + + // Update input (expect to throw) + (fixture.componentInstance as unknown as + {[key: string]: unknown})[inputName as string] = value; + fixture.detectChanges(); + }) + .toThrowError( + `NG02952: The NgOptimizedImage directive (activated on an element ` + + `with the \`rawSrc="img.png"\`) detected that the \`${ + inputName}\` is updated ` + + `after the initialization. The NgOptimizedImage directive will not react ` + + `to this input change.`); + }); + }); }); }); @@ -211,6 +269,10 @@ const ANGULAR_LOGO_BASE64 = template: '', }) class TestComponent { + width = 100; + height = 50; + rawSrc = 'img.png'; + priority = false; } function setupTestingModule(config?: {imageLoader: ImageLoader}) {