diff --git a/goldens/public-api/common/errors.md b/goldens/public-api/common/errors.md index 5e7508b9cd2..5435046fc27 100644 --- a/goldens/public-api/common/errors.md +++ b/goldens/public-api/common/errors.md @@ -11,6 +11,8 @@ export const enum RuntimeErrorCode { // (undocumented) INVALID_PIPE_ARGUMENT = 2100, // (undocumented) + LCP_IMG_MISSING_PRIORITY = 2954, + // (undocumented) NG_FOR_MISSING_DIFFER = -2200, // (undocumented) PARENT_NG_SWITCH_NOT_FOUND = 2000, diff --git a/packages/common/src/directives/ng_optimized_image.ts b/packages/common/src/directives/ng_optimized_image.ts index 62e4203ddbf..af04ac245b1 100644 --- a/packages/common/src/directives/ng_optimized_image.ts +++ b/packages/common/src/directives/ng_optimized_image.ts @@ -86,9 +86,15 @@ class LCPImageObserver implements OnDestroy { // Based on https://web.dev/lcp/#measure-lcp-in-javascript private initPerformanceObserver(): PerformanceObserver { const observer = new PerformanceObserver((entryList) => { - for (const entry of entryList.getEntries()) { + const entries = entryList.getEntries(); + if (entries.length > 0) { + // Note: we use the latest entry produced by the `PerformanceObserver` as the best + // signal on which element is actually an LCP one. As an example, the first image to load on + // a page, by virtue of being the only thing on the page so far, is often a LCP candidate + // and gets reported by PerformanceObserver, but isn't necessarily the LCP element. + const lcpElement = entries[entries.length - 1]; // Cast to `any` due to missing `element` on observed type of entry. - const imgSrc = (entry as any).element?.src ?? ''; + const imgSrc = (lcpElement as any).element?.src ?? ''; const img = this.images.get(imgSrc); // Exclude `data:` and `blob:` URLs, since they are not supported by the directive. if (img && !img.priority && !imgSrc.startsWith('data:') && !imgSrc.startsWith('blob:')) { diff --git a/packages/core/test/bundling/image-directive/BUILD.bazel b/packages/core/test/bundling/image-directive/BUILD.bazel index 8cefde710f2..53555998fc6 100644 --- a/packages/core/test/bundling/image-directive/BUILD.bazel +++ b/packages/core/test/bundling/image-directive/BUILD.bazel @@ -8,6 +8,7 @@ ng_module( srcs = [ "basic/basic.ts", "index.ts", + "lcp-check/lcp-check.ts", ], deps = [ "//packages/common", @@ -71,18 +72,20 @@ http_server( ts_library( name = "img_dir_e2e_tests_lib", testonly = True, - srcs = glob(["**/*.e2e-spec.ts"]), - tsconfig = ":tsconfig-e2e.json", + srcs = ["e2e-util/util.ts"] + glob([ + "**/*.e2e-spec.ts", + ]), + tsconfig = ":e2e-util/tsconfig-e2e.json", deps = [ - "//packages/examples/test-utils", "//packages/private/testing", + "@npm//@types/selenium-webdriver", "@npm//protractor", ], ) protractor_web_test_suite( name = "protractor_tests", - on_prepare = ":start-server.js", + on_prepare = ":e2e-util/start-server.js", server = ":devserver", deps = [ ":img_dir_e2e_tests_lib", diff --git a/packages/core/test/bundling/image-directive/README.md b/packages/core/test/bundling/image-directive/README.md new file mode 100644 index 00000000000..bcdcf848e78 --- /dev/null +++ b/packages/core/test/bundling/image-directive/README.md @@ -0,0 +1,13 @@ +* NgOptimizedImage directive testing + +This folder contains a simple application that can be used as a playground for the `NgOptimizedImage` directive testing. You can run the following command to start the dev server: + +``` +yarn ibazel run packages/core/test/bundling/image-directive:devserver +``` + +There is also a set of e2e tests (powered by Protractor), which can be invoked by running: + +``` +yarn bazel test packages/core/test/bundling/image-directive:protractor_tests +``` diff --git a/packages/core/test/bundling/image-directive/basic/basic.e2e-spec.ts b/packages/core/test/bundling/image-directive/basic/basic.e2e-spec.ts index 84d9be1a636..f64ea3c9753 100644 --- a/packages/core/test/bundling/image-directive/basic/basic.e2e-spec.ts +++ b/packages/core/test/bundling/image-directive/basic/basic.e2e-spec.ts @@ -8,7 +8,7 @@ import {browser, by, element} from 'protractor'; -import {verifyNoBrowserErrors} from '../../../../../examples/test-utils'; +import {verifyNoBrowserErrors} from '../e2e-util/util'; describe('NgOptimizedImage directive', () => { afterEach(verifyNoBrowserErrors); diff --git a/packages/core/test/bundling/image-directive/basic/basic.ts b/packages/core/test/bundling/image-directive/basic/basic.ts index b72f55b8645..5f0eb2bd9ef 100644 --- a/packages/core/test/bundling/image-directive/basic/basic.ts +++ b/packages/core/test/bundling/image-directive/basic/basic.ts @@ -10,10 +10,10 @@ import {ɵIMAGE_LOADER as IMAGE_LOADER, ɵNgOptimizedImage as NgOptimizedImage} import {Component} from '@angular/core'; @Component({ - selector: 'app-root', + selector: 'basic', standalone: true, imports: [NgOptimizedImage], - template: ``, + template: ``, providers: [{provide: IMAGE_LOADER, useValue: () => 'b.png'}], }) export class BasicComponent { diff --git a/packages/core/test/bundling/image-directive/start-server.js b/packages/core/test/bundling/image-directive/e2e-util/start-server.js similarity index 100% rename from packages/core/test/bundling/image-directive/start-server.js rename to packages/core/test/bundling/image-directive/e2e-util/start-server.js diff --git a/packages/core/test/bundling/image-directive/tsconfig-e2e.json b/packages/core/test/bundling/image-directive/e2e-util/tsconfig-e2e.json similarity index 100% rename from packages/core/test/bundling/image-directive/tsconfig-e2e.json rename to packages/core/test/bundling/image-directive/e2e-util/tsconfig-e2e.json diff --git a/packages/core/test/bundling/image-directive/e2e-util/util.ts b/packages/core/test/bundling/image-directive/e2e-util/util.ts new file mode 100644 index 00000000000..ebc30a2c7ba --- /dev/null +++ b/packages/core/test/bundling/image-directive/e2e-util/util.ts @@ -0,0 +1,45 @@ +/** + * @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} from 'protractor'; +import {logging} from 'selenium-webdriver'; + +export async function collectBrowserLogs(minLoggingLevel: logging.Level): Promise { + const browserLog = await browser.manage().logs().get('browser'); + const collectedLogs: logging.Entry[] = []; + + browserLog.forEach(logEntry => { + const msg = logEntry.message; + + // Since we currently use the `ts_devserver` from the Bazel TypeScript rules, which does + // fallback to the "index.html" file for HTML5 pushState routing but does always serve the + // expected fallback with a 404 status code, the browser will print a message about the 404, + // while the page loaded properly. Ideally the "ts_devserver" would allow us to opt-in for + // just returning a 200 status code, but the devserver is intended to be kept manually, so + // we manually filter this error before ensuring there are no console errors. + // TODO: This is a current limitation of using the "ts_devserver" with Angular routing. + // Tracked with: TOOL-629 + if (msg.includes( + `Failed to load resource: the server responded with a status of 404 (Not Found)`)) { + return; + } + + console.log('>> ' + msg, logEntry); + + if (logEntry.level.value >= minLoggingLevel.value) { + collectedLogs.push(logEntry); + } + }); + return collectedLogs; +} + +export async function verifyNoBrowserErrors() { + const logs = await collectBrowserLogs(logging.Level.INFO); + expect(logs).toEqual([]); +} diff --git a/packages/core/test/bundling/image-directive/index.ts b/packages/core/test/bundling/image-directive/index.ts index 6973bd48ef6..e7453ca4757 100644 --- a/packages/core/test/bundling/image-directive/index.ts +++ b/packages/core/test/bundling/image-directive/index.ts @@ -7,10 +7,11 @@ */ import {Component, importProvidersFrom} from '@angular/core'; -import {bootstrapApplication} from '@angular/platform-browser'; +import {bootstrapApplication, provideProtractorTestingSupport} from '@angular/platform-browser'; import {RouterModule} from '@angular/router'; import {BasicComponent} from './basic/basic'; +import {LcpCheckComponent} from './lcp-check/lcp-check'; @Component({ selector: 'app-root', @@ -22,9 +23,13 @@ export class RootComponent { } const ROUTES = [ - {path: '', component: BasicComponent} // + {path: '', component: BasicComponent}, // + {path: 'lcp-check', component: LcpCheckComponent} ]; bootstrapApplication(RootComponent, { - providers: [importProvidersFrom(RouterModule.forRoot(ROUTES))], + providers: [ + provideProtractorTestingSupport(), // + importProvidersFrom(RouterModule.forRoot(ROUTES)) + ], }); diff --git a/packages/core/test/bundling/image-directive/lcp-check/lcp-check.e2e-spec.ts b/packages/core/test/bundling/image-directive/lcp-check/lcp-check.e2e-spec.ts new file mode 100644 index 00000000000..8b22bdfc554 --- /dev/null +++ b/packages/core/test/bundling/image-directive/lcp-check/lcp-check.e2e-spec.ts @@ -0,0 +1,36 @@ +/** + * @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} from 'protractor'; +import {logging} from 'selenium-webdriver'; + +import {collectBrowserLogs} from '../e2e-util/util'; + +describe('NgOptimizedImage directive', () => { + it('should log a warning when a `priority` is missing on an LCP image', async () => { + await browser.get('/lcp-check'); + + // Verify that both images were rendered. + const imgs = element.all(by.css('img')); + let srcB = await imgs.get(0).getAttribute('src'); + expect(srcB.endsWith('b.png')).toBe(true); + const srcA = await imgs.get(1).getAttribute('src'); + expect(srcA.endsWith('a.png')).toBe(true); + // The `b.png` image is used twice in a template. + srcB = await imgs.get(2).getAttribute('src'); + expect(srcB.endsWith('b.png')).toBe(true); + + // Make sure that only one warning is in the console for image `a.png`, + // since the `b.png` should be below the fold and not treated as an LCP element. + const logs = await collectBrowserLogs(logging.Level.WARNING); + expect(logs.length).toEqual(1); + // Verify that the error code and the image src are present in the error message. + expect(logs[0].message).toMatch(/NG02954.*?a\.png/); + }); +}); diff --git a/packages/core/test/bundling/image-directive/lcp-check/lcp-check.ts b/packages/core/test/bundling/image-directive/lcp-check/lcp-check.ts new file mode 100644 index 00000000000..0c49e059748 --- /dev/null +++ b/packages/core/test/bundling/image-directive/lcp-check/lcp-check.ts @@ -0,0 +1,38 @@ +/** + * @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 {ɵNgOptimizedImage as NgOptimizedImage} from '@angular/common'; +import {Component} from '@angular/core'; + +@Component({ + selector: 'lcp-check', + standalone: true, + imports: [NgOptimizedImage], + template: ` + + + +
+ + + + +
+ + + + `, +}) +export class LcpCheckComponent { +}