Skip to content

Commit a55719f

Browse files
JeanMechethePunderWoman
JeanMeche
authored and
thePunderWoman
committed
fix(common): Don't run preconnect assertion on the server. (#56213)
The `window` global is patched by domino on the server but the value of `window.location.href` isn't a valid base. Before this change `getUrl()` would throw when running in devmode on the server. Fixes #56207 PR Close #56213
1 parent 64f6860 commit a55719f

4 files changed

Lines changed: 51 additions & 23 deletions

File tree

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,16 @@ import {
1111
Injectable,
1212
OnDestroy,
1313
ɵformatRuntimeError as formatRuntimeError,
14+
PLATFORM_ID,
1415
} from '@angular/core';
1516

16-
import {DOCUMENT} from '../../dom_tokens';
1717
import {RuntimeErrorCode} from '../../errors';
1818

1919
import {assertDevMode} from './asserts';
2020
import {imgDirectiveDetails} from './error_helper';
2121
import {getUrl} from './url';
22+
import {PlatformLocation} from '../../location';
23+
import {isPlatformBrowser} from '../../platform_id';
2224

2325
interface ObservedImageState {
2426
priority: boolean;
@@ -42,14 +44,14 @@ export class LCPImageObserver implements OnDestroy {
4244
// Map of full image URLs -> original `ngSrc` values.
4345
private images = new Map<string, ObservedImageState>();
4446

45-
private window: Window | null = null;
4647
private observer: PerformanceObserver | null = null;
48+
private readonly baseUrl = inject(PlatformLocation).href;
4749

4850
constructor() {
51+
const isBrowser = isPlatformBrowser(inject(PLATFORM_ID));
52+
4953
assertDevMode('LCP checker');
50-
const win = inject(DOCUMENT).defaultView;
51-
if (typeof win !== 'undefined' && typeof PerformanceObserver !== 'undefined') {
52-
this.window = win;
54+
if (isBrowser && typeof PerformanceObserver !== 'undefined') {
5355
this.observer = this.initPerformanceObserver();
5456
}
5557
}
@@ -98,20 +100,20 @@ export class LCPImageObserver implements OnDestroy {
98100
alreadyWarnedModified: false,
99101
alreadyWarnedPriority: false,
100102
};
101-
this.images.set(getUrl(rewrittenSrc, this.window!).href, newObservedImageState);
103+
this.images.set(getUrl(rewrittenSrc, this.baseUrl).href, newObservedImageState);
102104
}
103105

104106
unregisterImage(rewrittenSrc: string) {
105107
if (!this.observer) return;
106-
this.images.delete(getUrl(rewrittenSrc, this.window!).href);
108+
this.images.delete(getUrl(rewrittenSrc, this.baseUrl).href);
107109
}
108110

109111
updateImage(originalSrc: string, newSrc: string) {
110-
const originalUrl = getUrl(originalSrc, this.window!).href;
112+
const originalUrl = getUrl(originalSrc, this.baseUrl).href;
111113
const img = this.images.get(originalUrl);
112114
if (img) {
113115
img.modified = true;
114-
this.images.set(getUrl(newSrc, this.window!).href, img);
116+
this.images.set(getUrl(newSrc, this.baseUrl).href, img);
115117
this.images.delete(originalUrl);
116118
}
117119
}

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

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
Injectable,
1212
InjectionToken,
1313
ɵformatRuntimeError as formatRuntimeError,
14-
ɵRuntimeError as RuntimeError,
14+
PLATFORM_ID,
1515
} from '@angular/core';
1616

1717
import {DOCUMENT} from '../../dom_tokens';
@@ -20,6 +20,8 @@ import {RuntimeErrorCode} from '../../errors';
2020
import {assertDevMode} from './asserts';
2121
import {imgDirectiveDetails} from './error_helper';
2222
import {extractHostname, getUrl} from './url';
23+
import {isPlatformServer} from '../../platform_id';
24+
import {PlatformLocation} from '../../location';
2325

2426
// Set of origins that are always excluded from the preconnect checks.
2527
const INTERNAL_PRECONNECT_CHECK_BLOCKLIST = new Set(['localhost', '127.0.0.1', '0.0.0.0']);
@@ -56,6 +58,8 @@ export const PRECONNECT_CHECK_BLOCKLIST = new InjectionToken<Array<string | stri
5658
@Injectable({providedIn: 'root'})
5759
export class PreconnectLinkChecker {
5860
private document = inject(DOCUMENT);
61+
private readonly isServer = isPlatformServer(inject(PLATFORM_ID));
62+
private readonly platformLocation = inject(PlatformLocation);
5963

6064
/**
6165
* Set of <link rel="preconnect"> tags found on this page.
@@ -68,16 +72,10 @@ export class PreconnectLinkChecker {
6872
*/
6973
private alreadySeen = new Set<string>();
7074

71-
private window: Window | null = null;
72-
7375
private blocklist = new Set<string>(INTERNAL_PRECONNECT_CHECK_BLOCKLIST);
7476

7577
constructor() {
7678
assertDevMode('preconnect link checker');
77-
const win = this.document.defaultView;
78-
if (typeof win !== 'undefined') {
79-
this.window = win;
80-
}
8179
const blocklist = inject(PRECONNECT_CHECK_BLOCKLIST, {optional: true});
8280
if (blocklist) {
8381
this.populateBlocklist(blocklist);
@@ -102,9 +100,9 @@ export class PreconnectLinkChecker {
102100
* @param originalNgSrc ngSrc value
103101
*/
104102
assertPreconnect(rewrittenSrc: string, originalNgSrc: string): void {
105-
if (!this.window) return;
103+
if (this.isServer) return;
106104

107-
const imgUrl = getUrl(rewrittenSrc, this.window);
105+
const imgUrl = getUrl(rewrittenSrc, this.platformLocation.href);
108106
if (this.blocklist.has(imgUrl.hostname) || this.alreadySeen.has(imgUrl.origin)) return;
109107

110108
// Register this origin as seen, so we don't check it again later.
@@ -135,7 +133,7 @@ export class PreconnectLinkChecker {
135133
const selector = 'link[rel=preconnect]';
136134
const links: HTMLLinkElement[] = Array.from(this.document.querySelectorAll(selector));
137135
for (let link of links) {
138-
const url = getUrl(link.href, this.window!);
136+
const url = getUrl(link.href, this.platformLocation.href);
139137
preconnectUrls.add(url.origin);
140138
}
141139
return preconnectUrls;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
*/
88

99
// Converts a string that represents a URL into a URL class instance.
10-
export function getUrl(src: string, win: Window): URL {
10+
export function getUrl(src: string, href: string): URL {
1111
// Don't use a base URL is the URL is absolute.
12-
return isAbsoluteUrl(src) ? new URL(src) : new URL(src, win.location.href);
12+
return isAbsoluteUrl(src) ? new URL(src) : new URL(src, href);
1313
}
1414

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

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

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

9-
import {CommonModule, DOCUMENT, IMAGE_CONFIG, ImageConfig} from '@angular/common';
9+
import {CommonModule, DOCUMENT, IMAGE_CONFIG, ImageConfig, PlatformLocation} from '@angular/common';
1010
import {RuntimeErrorCode} from '@angular/common/src/errors';
1111
import {PLATFORM_SERVER_ID} from '@angular/common/src/platform_id';
1212
import {ChangeDetectionStrategy, Component, PLATFORM_ID, Provider, Type} from '@angular/core';
@@ -32,6 +32,7 @@ import {
3232
RECOMMENDED_SRCSET_DENSITY_CAP,
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 '@angular/common/testing';
3536

3637
describe('Image directive', () => {
3738
describe('preload <link> element on a server', () => {
@@ -1243,6 +1244,9 @@ describe('Image directive', () => {
12431244
it(
12441245
'should log a warning if there is no preconnect link for a priority image',
12451246
withHead('', () => {
1247+
// The warning is only logged on the client
1248+
if (!isBrowser) return;
1249+
12461250
setupTestingModule({imageLoader});
12471251

12481252
const consoleWarnSpy = spyOn(console, 'warn');
@@ -1281,6 +1285,9 @@ describe('Image directive', () => {
12811285
it(
12821286
"should log a warning if there is a preconnect, but it doesn't match the priority image",
12831287
withHead('<link rel="preconnect" href="http://angular.io">', () => {
1288+
// The warning is only logged on the client
1289+
if (!isBrowser) return;
1290+
12841291
setupTestingModule({imageLoader});
12851292

12861293
const consoleWarnSpy = spyOn(console, 'warn');
@@ -1305,6 +1312,9 @@ describe('Image directive', () => {
13051312
withHead(
13061313
'<link rel="preload" href="https://angular.io/assets/images/logos/angular/angular.svg" as="image">',
13071314
() => {
1315+
// The warning is only logged on the client
1316+
if (!isBrowser) return;
1317+
13081318
setupTestingModule({imageLoader});
13091319

13101320
const consoleWarnSpy = spyOn(console, 'warn');
@@ -2165,6 +2175,14 @@ function setupTestingModule(config?: {
21652175
{provide: DOCUMENT, useValue: window.document},
21662176
...(config?.noLoader ? [] : [{provide: IMAGE_LOADER, useValue: loader}]),
21672177
...extraProviders,
2178+
{
2179+
provide: PlatformLocation,
2180+
useFactory: () => {
2181+
return new MockPlatformLocation({
2182+
appBaseHref: 'https://angular.io/',
2183+
});
2184+
},
2185+
},
21682186
];
21692187
if (config?.imageConfig) {
21702188
providers.push({provide: IMAGE_CONFIG, useValue: config.imageConfig});
@@ -2185,7 +2203,17 @@ function setUpModuleNoLoader() {
21852203
TestBed.configureTestingModule({
21862204
declarations: [TestComponent],
21872205
imports: [CommonModule, NgOptimizedImage],
2188-
providers: [{provide: DOCUMENT, useValue: window.document}],
2206+
providers: [
2207+
{provide: DOCUMENT, useValue: window.document},
2208+
{
2209+
provide: PlatformLocation,
2210+
useFactory: () => {
2211+
return new MockPlatformLocation({
2212+
appBaseHref: 'https://angular.io/',
2213+
});
2214+
},
2215+
},
2216+
],
21892217
});
21902218
}
21912219

0 commit comments

Comments
 (0)