diff --git a/packages/common/src/directives/ng_optimized_image/lcp_image_observer.ts b/packages/common/src/directives/ng_optimized_image/lcp_image_observer.ts index 0de0e1a7573..0164e132218 100644 --- a/packages/common/src/directives/ng_optimized_image/lcp_image_observer.ts +++ b/packages/common/src/directives/ng_optimized_image/lcp_image_observer.ts @@ -25,6 +25,7 @@ interface ObservedImageState { modified: boolean; alreadyWarnedPriority: boolean; alreadyWarnedModified: boolean; + count: number; } /** @@ -94,29 +95,77 @@ export class LCPImageObserver implements OnDestroy { registerImage(rewrittenSrc: string, isPriority: boolean) { if (!this.observer) return; - const newObservedImageState: ObservedImageState = { - priority: isPriority, - modified: false, - alreadyWarnedModified: false, - alreadyWarnedPriority: false, - }; - this.images.set(getUrl(rewrittenSrc, this.window!).href, newObservedImageState); + const url = getUrl(rewrittenSrc, this.window!).href; + const existingState = this.images.get(url); + + if (existingState) { + // If any instance has priority, the URL is considered to have priority + existingState.priority = existingState.priority || isPriority; + existingState.count++; + } else { + const newObservedImageState: ObservedImageState = { + priority: isPriority, + modified: false, + alreadyWarnedModified: false, + alreadyWarnedPriority: false, + count: 1, + }; + this.images.set(url, newObservedImageState); + } } unregisterImage(rewrittenSrc: string) { if (!this.observer) return; - this.images.delete(getUrl(rewrittenSrc, this.window!).href); + const url = getUrl(rewrittenSrc, this.window!).href; + const existingState = this.images.get(url); + + if (existingState) { + existingState.count--; + if (existingState.count <= 0) { + this.images.delete(url); + } + } } updateImage(originalSrc: string, newSrc: string) { if (!this.observer) return; const originalUrl = getUrl(originalSrc, this.window!).href; - const img = this.images.get(originalUrl); - if (img) { - img.modified = true; - this.images.set(getUrl(newSrc, this.window!).href, img); + const newUrl = getUrl(newSrc, this.window!).href; + + // URL hasn't changed + if (originalUrl === newUrl) return; + + const originalState = this.images.get(originalUrl); + if (!originalState) return; + + // Decrement count for original URL + originalState.count--; + if (originalState.count <= 0) { this.images.delete(originalUrl); } + + // Add or update entry for new URL + const newState = this.images.get(newUrl); + if (newState) { + // Merge if original had priority, new should too + newState.priority = newState.priority || originalState.priority; + newState.modified = true; + // Preserve warning flags from the original state to avoid duplicate warnings + newState.alreadyWarnedPriority = + newState.alreadyWarnedPriority || originalState.alreadyWarnedPriority; + newState.alreadyWarnedModified = + newState.alreadyWarnedModified || originalState.alreadyWarnedModified; + newState.count++; + } else { + // Create new entry, preserving state from the image that moved + this.images.set(newUrl, { + priority: originalState.priority, + modified: true, + alreadyWarnedModified: originalState.alreadyWarnedModified, + alreadyWarnedPriority: originalState.alreadyWarnedPriority, + count: 1, + }); + } } ngOnDestroy() { diff --git a/packages/core/test/bundling/image-directive/BUILD.bazel b/packages/core/test/bundling/image-directive/BUILD.bazel index 2a1f9db1217..e4ead4036c4 100644 --- a/packages/core/test/bundling/image-directive/BUILD.bazel +++ b/packages/core/test/bundling/image-directive/BUILD.bazel @@ -12,6 +12,7 @@ ng_project( "e2e/image-perf-warnings-lazy/image-perf-warnings-lazy.ts", "e2e/image-perf-warnings-oversized/image-perf-warnings-oversized.ts", "e2e/image-perf-warnings-oversized/svg-no-perf-oversized-warnings.ts", + "e2e/lcp-check-duplicate/lcp-check-duplicate.ts", "e2e/lcp-check/lcp-check.ts", "e2e/oversized-image/oversized-image.ts", "e2e/preconnect-check/preconnect-check.ts", diff --git a/packages/core/test/bundling/image-directive/e2e/lcp-check-duplicate/lcp-check-duplicate.e2e-spec.ts b/packages/core/test/bundling/image-directive/e2e/lcp-check-duplicate/lcp-check-duplicate.e2e-spec.ts new file mode 100644 index 00000000000..e01ea9879aa --- /dev/null +++ b/packages/core/test/bundling/image-directive/e2e/lcp-check-duplicate/lcp-check-duplicate.e2e-spec.ts @@ -0,0 +1,35 @@ +/** + * @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.dev/license + */ + +/* tslint:disable:no-console */ +import {browser, by, element} from 'protractor'; +import {logging} from 'selenium-webdriver'; + +import {collectBrowserLogs} from '../browser-logs-util'; + +describe('NgOptimizedImage directive', () => { + it('should log a warning when a `priority` is missing on an LCP image', async () => { + await browser.get('/e2e/lcp-check-duplicate'); + // 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); + let srcA = await imgs.get(1).getAttribute('src'); + expect(srcA.endsWith('a.png')).toBe(true); + // The `b.png` and `a.png` images are used twice in a template. + srcB = await imgs.get(2).getAttribute('src'); + expect(srcB.endsWith('b.png')).toBe(true); + srcA = await imgs.get(3).getAttribute('src'); + expect(srcA.endsWith('a.png')).toBe(true); + + // Make sure that no warnings are in the console for image `a.png`, + // since the first instance has the `priority` attribute, and is the LCP element. + const logs = await collectBrowserLogs(logging.Level.SEVERE); + expect(logs.length).toEqual(0); + }); +}); diff --git a/packages/core/test/bundling/image-directive/e2e/lcp-check-duplicate/lcp-check-duplicate.ts b/packages/core/test/bundling/image-directive/e2e/lcp-check-duplicate/lcp-check-duplicate.ts new file mode 100644 index 00000000000..d4ef8a38590 --- /dev/null +++ b/packages/core/test/bundling/image-directive/e2e/lcp-check-duplicate/lcp-check-duplicate.ts @@ -0,0 +1,41 @@ +/** + * @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} from '@angular/common'; +import {Component} from '@angular/core'; + +@Component({ + selector: 'lcp-check', + imports: [NgOptimizedImage], + template: ` + + + +
+ + + + +
+ + + + + + + +
+ `, +}) +export class LcpCheckDuplicate {} diff --git a/packages/core/test/bundling/image-directive/e2e/oversized-image/oversized-image.e2e-spec.ts b/packages/core/test/bundling/image-directive/e2e/oversized-image/oversized-image.e2e-spec.ts index 4f396b04aeb..bf4fce9c2de 100644 --- a/packages/core/test/bundling/image-directive/e2e/oversized-image/oversized-image.e2e-spec.ts +++ b/packages/core/test/bundling/image-directive/e2e/oversized-image/oversized-image.e2e-spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {browser, by, element, ExpectedConditions} from 'protractor'; +import {browser} from 'protractor'; import {logging} from 'selenium-webdriver'; import {collectBrowserLogs} from '../browser-logs-util'; diff --git a/packages/core/test/bundling/image-directive/index.ts b/packages/core/test/bundling/image-directive/index.ts index 639db0294d2..98b79b8c8cd 100644 --- a/packages/core/test/bundling/image-directive/index.ts +++ b/packages/core/test/bundling/image-directive/index.ts @@ -27,6 +27,7 @@ import { } from './e2e/oversized-image/oversized-image'; import {PreconnectCheckComponent} from './e2e/preconnect-check/preconnect-check'; import {PlaygroundComponent} from './playground'; +import {LcpCheckDuplicate} from './e2e/lcp-check-duplicate/lcp-check-duplicate'; @Component({ selector: 'app-root', @@ -42,6 +43,7 @@ const ROUTES = [ // Paths below are used for e2e testing: {path: 'e2e/basic', component: BasicComponent}, {path: 'e2e/lcp-check', component: LcpCheckComponent}, + {path: 'e2e/lcp-check-duplicate', component: LcpCheckDuplicate}, {path: 'e2e/image-perf-warnings-lazy', component: ImagePerfWarningsLazyComponent}, {path: 'e2e/image-perf-warnings-oversized', component: ImagePerfWarningsOversizedComponent}, {path: 'e2e/svg-no-perf-oversized-warnings', component: SvgNoOversizedPerfWarningsComponent},