Skip to content

Commit 723790c

Browse files
kon72kon72
committed
fix(common): do not warn about missing preconnect hints for same-origin images
When the `PreconnectLinkChecker` checks whether an image has a corresponding preconnect hint, it should not log a warning if the image location is the same origin as the page, even if there is no preconnect link, since the browser already establishes a connection to the origin of the page. Also replace the usage of `inject(DOCUMENT).defaultView.location` with `PlatformLocation` for a test with a mocked location.
1 parent 71f8d48 commit 723790c

4 files changed

Lines changed: 44 additions & 12 deletions

File tree

‎packages/common/src/directives/ng_optimized_image/lcp_image_observer.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {RuntimeErrorCode} from '../../errors';
1919
import {assertDevMode} from './asserts';
2020
import {imgDirectiveDetails} from './error_helper';
2121
import {getUrl} from './url';
22+
import {PlatformLocation} from '../../location';
2223

2324
interface ObservedImageState {
2425
priority: boolean;
@@ -43,7 +44,7 @@ export class LCPImageObserver implements OnDestroy {
4344
// Map of full image URLs -> original `ngSrc` values.
4445
private images = new Map<string, ObservedImageState>();
4546

46-
private window: Window | null = inject(DOCUMENT).defaultView;
47+
private platformLocation = inject(PlatformLocation);
4748
private observer: PerformanceObserver | null = null;
4849

4950
constructor() {
@@ -95,7 +96,7 @@ export class LCPImageObserver implements OnDestroy {
9596

9697
registerImage(rewrittenSrc: string, isPriority: boolean) {
9798
if (!this.observer) return;
98-
const url = getUrl(rewrittenSrc, this.window!).href;
99+
const url = getUrl(rewrittenSrc, this.platformLocation).href;
99100
const existingState = this.images.get(url);
100101

101102
if (existingState) {
@@ -116,7 +117,7 @@ export class LCPImageObserver implements OnDestroy {
116117

117118
unregisterImage(rewrittenSrc: string) {
118119
if (!this.observer) return;
119-
const url = getUrl(rewrittenSrc, this.window!).href;
120+
const url = getUrl(rewrittenSrc, this.platformLocation).href;
120121
const existingState = this.images.get(url);
121122

122123
if (existingState) {
@@ -129,8 +130,8 @@ export class LCPImageObserver implements OnDestroy {
129130

130131
updateImage(originalSrc: string, newSrc: string) {
131132
if (!this.observer) return;
132-
const originalUrl = getUrl(originalSrc, this.window!).href;
133-
const newUrl = getUrl(newSrc, this.window!).href;
133+
const originalUrl = getUrl(originalSrc, this.platformLocation).href;
134+
const newUrl = getUrl(newSrc, this.platformLocation).href;
134135

135136
// URL hasn't changed
136137
if (originalUrl === newUrl) return;

‎packages/common/src/directives/ng_optimized_image/preconnect_link_checker.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {RuntimeErrorCode} from '../../errors';
2020
import {assertDevMode} from './asserts';
2121
import {imgDirectiveDetails} from './error_helper';
2222
import {extractHostname, getUrl} from './url';
23+
import {PlatformLocation} from '../../location';
2324

2425
// Set of origins that are always excluded from the preconnect checks.
2526
const INTERNAL_PRECONNECT_CHECK_BLOCKLIST = new Set(['localhost', '127.0.0.1', '0.0.0.0', '[::1]']);
@@ -56,6 +57,7 @@ export const PRECONNECT_CHECK_BLOCKLIST = new InjectionToken<Array<string | stri
5657
@Service()
5758
export class PreconnectLinkChecker implements OnDestroy {
5859
private document = inject(DOCUMENT);
60+
private platformLocation = inject(PlatformLocation);
5961

6062
/**
6163
* Set of <link rel="preconnect"> tags found on this page.
@@ -68,8 +70,6 @@ export class PreconnectLinkChecker implements OnDestroy {
6870
*/
6971
private alreadySeen = new Set<string>();
7072

71-
private window: Window | null = this.document.defaultView;
72-
7373
private blocklist = new Set<string>(INTERNAL_PRECONNECT_CHECK_BLOCKLIST);
7474

7575
constructor() {
@@ -100,7 +100,12 @@ export class PreconnectLinkChecker implements OnDestroy {
100100
assertPreconnect(rewrittenSrc: string, originalNgSrc: string): void {
101101
if (typeof ngServerMode !== 'undefined' && ngServerMode) return;
102102

103-
const imgUrl = getUrl(rewrittenSrc, this.window!);
103+
const imgUrl = getUrl(rewrittenSrc, this.platformLocation);
104+
105+
// Do not check preconnect hints for same-origin URLs as the browser already
106+
// establishes a connection to the origin of the page.
107+
if (imgUrl.origin === new URL(this.platformLocation.href).origin) return;
108+
104109
if (this.blocklist.has(imgUrl.hostname) || this.alreadySeen.has(imgUrl.origin)) return;
105110

106111
// Register this origin as seen, so we don't check it again later.
@@ -130,7 +135,7 @@ export class PreconnectLinkChecker implements OnDestroy {
130135
const preconnectUrls = new Set<string>();
131136
const links = this.document.querySelectorAll<HTMLLinkElement>('link[rel=preconnect]');
132137
for (const link of links) {
133-
const url = getUrl(link.href, this.window!);
138+
const url = getUrl(link.href, this.platformLocation);
134139
preconnectUrls.add(url.origin);
135140
}
136141
return preconnectUrls;

‎packages/common/src/directives/ng_optimized_image/url.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import {PlatformLocation} from '../../location';
10+
911
// Converts a string that represents a URL into a URL class instance.
10-
export function getUrl(src: string, win: Window): URL {
12+
export function getUrl(src: string, win: PlatformLocation): URL {
1113
// Don't use a base URL is the URL is absolute.
12-
return isAbsoluteUrl(src) ? new URL(src) : new URL(src, win.location.href);
14+
return isAbsoluteUrl(src) ? new URL(src) : new URL(src, win.href);
1315
}
1416

1517
// Checks whether a URL is absolute (i.e. starts with `http://` or `https://`).

‎packages/common/test/directives/ng_optimized_image_spec.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {ComponentFixture, TestBed} from '@angular/core/testing';
1111
import {DomSanitizer, SafeResourceUrl} from '@angular/platform-browser';
1212
import {isBrowser, isNode, withHead} from '@angular/private/testing';
1313
import {expect} from '@angular/private/testing/matchers';
14-
import {CommonModule, DOCUMENT, IMAGE_CONFIG, ImageConfig} from '../../index';
14+
import {CommonModule, DOCUMENT, IMAGE_CONFIG, ImageConfig, PlatformLocation} from '../../index';
1515
import {RuntimeErrorCode} from '../../src/errors';
1616
import {PLATFORM_SERVER_ID} from '../../src/platform_id';
1717

@@ -32,6 +32,7 @@ import {
3232
resetImagePriorityCount,
3333
} from '../../src/directives/ng_optimized_image/ng_optimized_image';
3434
import {PRECONNECT_CHECK_BLOCKLIST} from '../../src/directives/ng_optimized_image/preconnect_link_checker';
35+
import {MockPlatformLocation} from '../../testing';
3536

3637
describe('Image directive', () => {
3738
const PLACEHOLDER_BLUR_AMOUNT = 15;
@@ -1622,6 +1623,29 @@ describe('Image directive', () => {
16221623
}),
16231624
);
16241625

1626+
it(
1627+
'should not log a warning if there is no preconnect link, but the image is loaded from the same origin',
1628+
withHead('', () => {
1629+
setupTestingModule({
1630+
imageLoader,
1631+
extraProviders: [
1632+
{
1633+
provide: PlatformLocation,
1634+
useValue: new MockPlatformLocation({startUrl: 'https://angular.dev/some-page'}),
1635+
},
1636+
],
1637+
});
1638+
1639+
const consoleWarnSpy = spyOn(console, 'warn');
1640+
const template = '<img ngSrc="a.png" width="100" height="50" priority>';
1641+
const fixture = createTestComponent(template);
1642+
fixture.detectChanges();
1643+
1644+
// Expect no warnings in the console.
1645+
expect(consoleWarnSpy.calls.count()).toBe(0);
1646+
}),
1647+
);
1648+
16251649
['localhost', '127.0.0.1', '0.0.0.0', '[::1]'].forEach((blocklistedHostname) => {
16261650
it(
16271651
`should not log a warning if an origin domain is blocklisted ` +

0 commit comments

Comments
 (0)