diff --git a/goldens/public-api/common/errors.md b/goldens/public-api/common/errors.md index d96110edf70d0..695bfeb54f47c 100644 --- a/goldens/public-api/common/errors.md +++ b/goldens/public-api/common/errors.md @@ -17,6 +17,8 @@ export const enum RuntimeErrorCode { // (undocumented) PARENT_NG_SWITCH_NOT_FOUND = 2000, // (undocumented) + PRIORITY_IMG_MISSING_PRECONNECT_TAG = 2956, + // (undocumented) REQUIRED_INPUT_MISSING = 2954, // (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 index da5291a7ac96f..23ad3f4447cbb 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 @@ -12,6 +12,8 @@ import {DOCUMENT} from '../../dom_tokens'; import {RuntimeErrorCode} from '../../errors'; import {IMAGE_LOADER, ImageLoader} from './image_loaders/image_loader'; +import {PreconnectLinkChecker} from './preconnect_link_checker'; +import {getUrl, imgDirectiveDetails} from './util'; /** * When a Base64-encoded image is passed as an input to the `NgOptimizedImage` directive, @@ -44,10 +46,8 @@ const VALID_DENSITY_DESCRIPTOR_SRCSET = /^((\s*\d(\.\d)?x\s*(,|$)){1,})$/; * * Based on https://web.dev/lcp/#measure-lcp-in-javascript. */ -@Injectable({ - providedIn: 'root', -}) -class LCPImageObserver implements OnDestroy { +@Injectable({providedIn: 'root'}) +export class LCPImageObserver implements OnDestroy { // Map of full image URLs -> original `rawSrc` values. private images = new Map(); // Keep track of images for which `console.warn` was produced. @@ -64,11 +64,6 @@ class LCPImageObserver implements OnDestroy { } } - // Converts relative image URL to a full URL. - private getFullUrl(src: string) { - return new URL(src, this.window!.location.href).href; - } - // Inits PerformanceObserver and subscribes to LCP events. // Based on https://web.dev/lcp/#measure-lcp-in-javascript private initPerformanceObserver(): PerformanceObserver { @@ -103,12 +98,12 @@ class LCPImageObserver implements OnDestroy { registerImage(rewrittenSrc: string, rawSrc: string) { if (!this.observer) return; - this.images.set(this.getFullUrl(rewrittenSrc), rawSrc); + this.images.set(getUrl(rewrittenSrc, this.window!).href, rawSrc); } unregisterImage(rewrittenSrc: string) { if (!this.observer) return; - this.images.delete(this.getFullUrl(rewrittenSrc)); + this.images.delete(getUrl(rewrittenSrc, this.window!).href); } ngOnDestroy() { @@ -140,6 +135,12 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy { private _height?: number; private _priority = false; + // Calculate the rewritten `src` once and store it. + // This is needed to avoid repetitive calculations and make sure the directive cleanup in the + // `ngOnDestroy` does not rely on the `IMAGE_LOADER` logic (which in turn can rely on some other + // instance that might be already destroyed). + private _rewrittenSrc: string|null = null; + /** * Name of the source image. * Image name will be processed by the image loader and the final URL will be applied as the `src` @@ -223,7 +224,10 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy { assertRequiredNumberInput(this, this.width, 'width'); assertRequiredNumberInput(this, this.height, 'height'); assertValidLoadingInput(this); - if (!this.priority) { + if (this.priority) { + const checker = this.injector.get(PreconnectLinkChecker); + checker.check(this.getRewrittenSrc(), this.rawSrc); + } else { // Monitor whether an image is an LCP element only in case // the `priority` attribute is missing. Otherwise, an image // has the necessary settings and no extra checks are required. @@ -265,8 +269,12 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy { // ImageLoaderConfig supports setting a width property. However, we're not setting width here // because if the developer uses rendered width instead of intrinsic width in the HTML width // attribute, the image requested may be too small for 2x+ screens. - const imgConfig = {src: this.rawSrc}; - return this.imageLoader(imgConfig); + if (!this._rewrittenSrc) { + const imgConfig = {src: this.rawSrc}; + // Cache calculated image src to reuse it later in the code. + this._rewrittenSrc = this.imageLoader(imgConfig); + } + return this._rewrittenSrc; } private getRewrittenSrcset(): string { @@ -280,11 +288,12 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy { } ngOnDestroy() { - if (ngDevMode && !this.priority) { - // An image is only registered in dev mode, try to unregister only in dev mode as well. - withLCPImageObserver( - this.injector, - (observer: LCPImageObserver) => observer.unregisterImage(this.getRewrittenSrc())); + if (ngDevMode) { + if (!this.priority && this._rewrittenSrc !== null) { + withLCPImageObserver( + this.injector, + (observer: LCPImageObserver) => observer.unregisterImage(this._rewrittenSrc!)); + } } } @@ -346,11 +355,6 @@ function withLCPImageObserver( }); } -function imgDirectiveDetails(rawSrc: string) { - return `The NgOptimizedImage directive (activated on an element ` + - `with the \`rawSrc="${rawSrc}"\`)`; -} - /***** Assert functions *****/ // Verifies that there is no `src` set on a host element. diff --git a/packages/common/src/directives/ng_optimized_image/preconnect_link_checker.ts b/packages/common/src/directives/ng_optimized_image/preconnect_link_checker.ts new file mode 100644 index 0000000000000..9bc9bf9178abd --- /dev/null +++ b/packages/common/src/directives/ng_optimized_image/preconnect_link_checker.ts @@ -0,0 +1,84 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {Inject, Injectable, NgZone, ɵformatRuntimeError as formatRuntimeError} from '@angular/core'; + +import {DOCUMENT} from '../../dom_tokens'; +import {RuntimeErrorCode} from '../../errors'; + +import {getUrl, imgDirectiveDetails} from './util'; + +/** + * Contains the logic to detect whether an image, marked with the "priority" attribute + * has a corresponding `` tag in the `document.head`. + * + * Note: this is a dev-mode only class, which should not appear in prod bundles, + * thus there is no `ngDevMode` use in the code. + */ +@Injectable({providedIn: 'root'}) +export class PreconnectLinkChecker { + // Set of tags found on this page. + // The `null` value indicates that there was no DOM query operation performed. + private preconnectLinks: Set|null = null; + + // Keep track of all already seen origin URLs to avoid repeating the same check. + private alreadySeen = new Set(); + + private window: Window|null = null; + + constructor(@Inject(DOCUMENT) private doc: Document, private ngZone: NgZone) { + const win = doc.defaultView; + if (typeof win !== 'undefined') { + this.window = win; + } + } + + check(rewrittenSrc: string, rawSrc: string) { + if (!this.window) return; + + const imgUrl = getUrl(rewrittenSrc, this.window); + if (this.alreadySeen.has(imgUrl.origin)) return; + + // Register this origin as seen, so we don't check it again later. + this.alreadySeen.add(imgUrl.origin); + + if (this.preconnectLinks === null) { + // Note: we query for preconnect links only *once* and cache the results + // for the entire lifespan of an application, since it's unlikely that the + // list would change frequently. This allows to make sure there are no + // performance implications of making extra DOM lookups for each image. + this.preconnectLinks = this.queryPreconnectLinks(); + } + + if (!this.preconnectLinks.has(imgUrl.origin)) { + console.warn(formatRuntimeError( + RuntimeErrorCode.PRIORITY_IMG_MISSING_PRECONNECT_TAG, + `${imgDirectiveDetails(rawSrc)} has detected that this image ` + + `contains the "priority" attribute, but doesn't have a corresponding ` + + `preconnect tag. Please add the following element into ` + + `the of the document to optimize loading of this image:\n` + + ` `)); + } + } + + private queryPreconnectLinks(): Set { + const preconnectURLs = new Set(); + const selector = 'link[rel=preconnect]'; + const links = (this.doc.head?.querySelectorAll(selector) ?? []) as unknown as HTMLLinkElement[]; + links.forEach(link => { + const url = getUrl(link.href, this.window!); + preconnectURLs.add(url.origin); + }); + return preconnectURLs; + } + + ngOnDestroy() { + this.preconnectLinks?.clear(); + this.alreadySeen.clear(); + } +} diff --git a/packages/common/src/directives/ng_optimized_image/util.ts b/packages/common/src/directives/ng_optimized_image/util.ts new file mode 100644 index 0000000000000..556737f473fce --- /dev/null +++ b/packages/common/src/directives/ng_optimized_image/util.ts @@ -0,0 +1,20 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +// Converts a string that represents a URL into a URL class instance. +export function getUrl(src: string, win: Window): URL { + const isAbsolute = /^https?:\/\//.test(src); + // Don't use a base URL is the URL is absolute. + return isAbsolute ? new URL(src) : new URL(src, win.location.href); +} + +// Assembles directive details string, useful for error messages. +export function imgDirectiveDetails(rawSrc: string) { + return `The NgOptimizedImage directive (activated on an element ` + + `with the \`rawSrc="${rawSrc}"\`)`; +} diff --git a/packages/common/src/errors.ts b/packages/common/src/errors.ts index ce341b2f067d7..27b8de1a3d765 100644 --- a/packages/common/src/errors.ts +++ b/packages/common/src/errors.ts @@ -27,4 +27,5 @@ export const enum RuntimeErrorCode { UNEXPECTED_INPUT_CHANGE = 2953, REQUIRED_INPUT_MISSING = 2954, LCP_IMG_MISSING_PRIORITY = 2955, + PRIORITY_IMG_MISSING_PRECONNECT_TAG = 2956, } diff --git a/packages/common/test/directives/ng_optimized_image_spec.ts b/packages/common/test/directives/ng_optimized_image_spec.ts index 5059739ed784d..fd2d5d56393ad 100644 --- a/packages/common/test/directives/ng_optimized_image_spec.ts +++ b/packages/common/test/directives/ng_optimized_image_spec.ts @@ -7,12 +7,14 @@ */ import {CommonModule, DOCUMENT} from '@angular/common'; -import {IMAGE_LOADER, ImageLoader, ImageLoaderConfig} from '@angular/common/src/directives/ng_optimized_image/image_loaders/image_loader'; -import {assertValidRawSrcset, NgOptimizedImageModule} from '@angular/common/src/directives/ng_optimized_image/ng_optimized_image'; import {RuntimeErrorCode} from '@angular/common/src/errors'; -import {Component} from '@angular/core'; +import {Component, Provider} from '@angular/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; 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'; describe('Image directive', () => { it('should set `loading` and `fetchpriority` attributes before `src`', () => { @@ -247,7 +249,7 @@ describe('Image directive', () => { it('should throw for empty rawSrcSet', () => { const imageLoader = (config: ImageLoaderConfig) => { const width = config.width ? `-${config.width}` : ``; - return `path/${config.src}${width}.png`; + return window.location.origin + `/path/${config.src}${width}.png`; }; setupTestingModule({imageLoader}); @@ -268,7 +270,7 @@ describe('Image directive', () => { it('should throw for invalid rawSrcSet', () => { const imageLoader = (config: ImageLoaderConfig) => { const width = config.width ? `-${config.width}` : ``; - return `path/${config.src}${width}.png`; + return window.location.origin + `/path/${config.src}${width}.png`; }; setupTestingModule({imageLoader}); @@ -495,6 +497,94 @@ describe('Image directive', () => { }); }); + describe('preconnect detector', () => { + it('should log a warning if there is no preconnect link for a priority image', + withHead('', () => { + setupTestingModule(); + + const consoleWarnSpy = spyOn(console, 'warn'); + const template = ''; + const fixture = createTestComponent(template); + fixture.detectChanges(); + + expect(consoleWarnSpy.calls.count()).toBe(1); + expect(consoleWarnSpy.calls.argsFor(0)[0]) + .toBe( + 'NG02956: The NgOptimizedImage directive (activated on an ' + + 'element with the `rawSrc="a.png"`) has detected that this image ' + + 'contains the "priority" attribute, but doesn\'t have a corresponding ' + + 'preconnect tag. Please add the following element into the of ' + + 'the document to optimize loading of this image:' + + '\n '); + })); + + it('should not log a warning if there is no preconnect link, but the image is not set as a priority', + withHead('', () => { + setupTestingModule(); + + const consoleWarnSpy = spyOn(console, 'warn'); + const template = ''; + const fixture = createTestComponent(template); + fixture.detectChanges(); + + // Expect no warnings in the console. + expect(consoleWarnSpy.calls.count()).toBe(0); + })); + + it('should log a warning if there is a preconnect, but it doesn\'t match the priority image', + withHead('', () => { + setupTestingModule(); + + const consoleWarnSpy = spyOn(console, 'warn'); + const template = ''; + const fixture = createTestComponent(template); + fixture.detectChanges(); + + expect(consoleWarnSpy.calls.count()).toBe(1); + expect(consoleWarnSpy.calls.argsFor(0)[0]) + .toBe( + 'NG02956: The NgOptimizedImage directive (activated on an ' + + ' element with the `rawSrc="a.png"`) has detected that ' + + 'this image contains the "priority" attribute, but doesn\'t have ' + + 'a corresponding preconnect tag. Please add the following element ' + + 'into the of the document to optimize loading of this image:' + + '\n '); + })); + + it('should log a warning if there is no matching preconnect link for a priority image, but there is a preload tag', + withHead('', () => { + setupTestingModule(); + + const consoleWarnSpy = spyOn(console, 'warn'); + const template = ''; + const fixture = createTestComponent(template); + fixture.detectChanges(); + + expect(consoleWarnSpy.calls.count()).toBe(1); + expect(consoleWarnSpy.calls.argsFor(0)[0]) + .toBe( + 'NG02956: The NgOptimizedImage directive (activated on an ' + + ' element with the `rawSrc="a.png"`) has detected that ' + + 'this image contains the "priority" attribute, but doesn\'t have ' + + 'a corresponding preconnect tag. Please add the following element ' + + 'into the of the document to optimize loading of this image:' + + '\n '); + })); + + it('should not log a warning if there is a matching preconnect link for a priority image (with an extra `/` at the end)', + withHead('', () => { + setupTestingModule(); + + const consoleWarnSpy = spyOn(console, 'warn'); + const template = ''; + const fixture = createTestComponent(template); + fixture.detectChanges(); + + // Expect no warnings in the console. + expect(consoleWarnSpy.calls.count()).toBe(0); + })); + }); + describe('loaders', () => { it('should set `src` to match `rawSrc` if image loader is not provided', () => { setupTestingModule(); @@ -705,8 +795,16 @@ class TestComponent { } function setupTestingModule(config?: {imageLoader: ImageLoader}) { - const providers = - config?.imageLoader ? [{provide: IMAGE_LOADER, useValue: config?.imageLoader}] : []; + const defaultLoader = (config: ImageLoaderConfig) => { + const isAbsolute = /^https?:\/\//.test(config.src); + return isAbsolute ? config.src : window.location.origin + '/' + config.src; + }; + const loader = config?.imageLoader || defaultLoader; + const providers: Provider[] = [ + {provide: DOCUMENT, useValue: window.document}, + {provide: IMAGE_LOADER, useValue: loader}, + ]; + TestBed.configureTestingModule({ declarations: [TestComponent], // Note: the `NgOptimizedImage` directive is experimental and is not a part of the diff --git a/packages/core/test/bundling/image-directive/BUILD.bazel b/packages/core/test/bundling/image-directive/BUILD.bazel index 6b9d310fb7fcb..463e09bc63195 100644 --- a/packages/core/test/bundling/image-directive/BUILD.bazel +++ b/packages/core/test/bundling/image-directive/BUILD.bazel @@ -7,6 +7,7 @@ ng_module( srcs = [ "e2e/basic/basic.ts", "e2e/lcp-check/lcp-check.ts", + "e2e/preconnect-check/preconnect-check.ts", "index.ts", "playground.ts", ], diff --git a/packages/core/test/bundling/image-directive/e2e/basic/basic.e2e-spec.ts b/packages/core/test/bundling/image-directive/e2e/basic/basic.e2e-spec.ts index f45a6203cf4aa..e40076d2a907f 100644 --- a/packages/core/test/bundling/image-directive/e2e/basic/basic.e2e-spec.ts +++ b/packages/core/test/bundling/image-directive/e2e/basic/basic.e2e-spec.ts @@ -7,16 +7,23 @@ */ import {browser, by, element} from 'protractor'; +import {logging} from 'selenium-webdriver'; -import {verifyNoBrowserErrors} from '../util'; +import {collectBrowserLogs} from '../util'; describe('NgOptimizedImage directive', () => { - afterEach(verifyNoBrowserErrors); - it('should render an image with an updated `src`', async () => { await browser.get('/e2e/basic'); const imgs = element.all(by.css('img')); const src = await imgs.get(0).getAttribute('src'); expect(/b\.png/.test(src)).toBe(true); + + // Since there are no preconnect tags on a page, + // we expect a log in a console that mentions that. + const logs = await collectBrowserLogs(logging.Level.WARNING); + expect(logs.length).toEqual(1); + + // Verify that the error code and a raw image src are present. + expect(logs[0].message).toMatch(/NG02956.*?a\.png/); }); }); diff --git a/packages/core/test/bundling/image-directive/e2e/preconnect-check/preconnect-check.e2e-spec.ts b/packages/core/test/bundling/image-directive/e2e/preconnect-check/preconnect-check.e2e-spec.ts new file mode 100644 index 0000000000000..96d0ca781a809 --- /dev/null +++ b/packages/core/test/bundling/image-directive/e2e/preconnect-check/preconnect-check.e2e-spec.ts @@ -0,0 +1,50 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/* tslint:disable:no-console */ +import {browser, by, element, ElementHelper} from 'protractor'; +import {logging} from 'selenium-webdriver'; + +import {collectBrowserLogs} from '../util'; + +// Verifies that both images used in a component were rendered. +async function verifyImagesPresent(element: ElementHelper) { + const imgs = element.all(by.css('img')); + const srcA = await imgs.get(0).getAttribute('src'); + expect(srcA.endsWith('a.png')).toBe(true); + const srcB = await imgs.get(1).getAttribute('src'); + expect(srcB.endsWith('b.png')).toBe(true); +} + +describe('NgOptimizedImage directive', () => { + it('should log a warning when there is no preconnect for priority images', async () => { + await browser.get('/e2e/preconnect-check'); + + await verifyImagesPresent(element); + + // Make sure that only one warning is in the console for both images, + // because they both have the same base URL (which is used to look for + // corresponding `` tags). + const logs = await collectBrowserLogs(logging.Level.WARNING); + expect(logs.length).toEqual(1); + + // Verify that the error code and a raw image src are present in the + // error message. + expect(logs[0].message).toMatch(/NG02956.*?a\.png/); + }); + + it('should not produce any logs when a preconect tag is present', async () => { + await browser.get('/e2e/preconnect-check?preconnect'); + + await verifyImagesPresent(element); + + // Make sure there are no browser logs. + const logs = await collectBrowserLogs(logging.Level.WARNING); + expect(logs.length).toEqual(0); + }); +}); diff --git a/packages/core/test/bundling/image-directive/e2e/preconnect-check/preconnect-check.ts b/packages/core/test/bundling/image-directive/e2e/preconnect-check/preconnect-check.ts new file mode 100644 index 0000000000000..dc5fb7ef8c935 --- /dev/null +++ b/packages/core/test/bundling/image-directive/e2e/preconnect-check/preconnect-check.ts @@ -0,0 +1,52 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {DOCUMENT, ɵNgOptimizedImageModule as NgOptimizedImageModule} from '@angular/common'; +import {Component, Inject} from '@angular/core'; + +@Component({ + selector: 'preconnect-check', + standalone: true, + imports: [NgOptimizedImageModule], + template: ` + + + + `, +}) +export class PreconnectCheckComponent { + constructor(@Inject(DOCUMENT) private doc: Document) { + this.createRequestedLinkElements(); + } + + /** + * Setup an environment required for e2e testing: create the necessary `` elements in the + * `document.head`, so that the `NgOptimizedImage` logic can be verified in various scenarios. + */ + private createRequestedLinkElements() { + const win = this.doc.defaultView; + if (!win) return; + const url = new URL(win.location.href).searchParams; + const preconnect = url.get('preconnect'); + if (preconnect !== null) { + const link = this.createLinkElement('preconnect', win.location.origin); + this.doc.head.appendChild(link); + } + } + + /** + * Helper method to create a simple `` element based on inputs. + */ + private createLinkElement(rel: string, href: string, as?: string): HTMLLinkElement { + const link = this.doc.createElement('link'); + link.rel = rel; + link.href = href; + if (as) link.as = as; + return link; + } +} diff --git a/packages/core/test/bundling/image-directive/index.ts b/packages/core/test/bundling/image-directive/index.ts index af639c79f6c22..56a6cfe2212c1 100644 --- a/packages/core/test/bundling/image-directive/index.ts +++ b/packages/core/test/bundling/image-directive/index.ts @@ -12,6 +12,7 @@ import {RouterModule} from '@angular/router'; import {BasicComponent} from './e2e/basic/basic'; import {LcpCheckComponent} from './e2e/lcp-check/lcp-check'; +import {PreconnectCheckComponent} from './e2e/preconnect-check/preconnect-check'; import {PlaygroundComponent} from './playground'; @Component({ @@ -29,7 +30,8 @@ const ROUTES = [ // Paths below are used for e2e testing: {path: 'e2e/basic', component: BasicComponent}, - {path: 'e2e/lcp-check', component: LcpCheckComponent} + {path: 'e2e/lcp-check', component: LcpCheckComponent}, + {path: 'e2e/preconnect-check', component: PreconnectCheckComponent} ]; bootstrapApplication(RootComponent, { diff --git a/packages/core/test/bundling/image-directive/playground.ts b/packages/core/test/bundling/image-directive/playground.ts index efbd3f5af0ac7..5f6b8b4fde190 100644 --- a/packages/core/test/bundling/image-directive/playground.ts +++ b/packages/core/test/bundling/image-directive/playground.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ɵIMAGE_LOADER as IMAGE_LOADER, ɵImageLoaderConfig as ImageLoaderConfig, ɵNgOptimizedImageModule as NgOptimizedImageModule, ɵprovideImgixLoader as provideImgixLoader} from '@angular/common'; +import {ɵNgOptimizedImageModule as NgOptimizedImageModule, ɵprovideImgixLoader as provideImgixLoader} from '@angular/common'; import {Component} from '@angular/core'; @Component({