From a55719f55eb9b75799dfe41bb56cacdd85b4bd9f Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Sat, 1 Jun 2024 13:18:10 +0200 Subject: [PATCH] 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 --- .../ng_optimized_image/lcp_image_observer.ts | 20 ++++++------ .../preconnect_link_checker.ts | 18 +++++------ .../src/directives/ng_optimized_image/url.ts | 4 +-- .../directives/ng_optimized_image_spec.ts | 32 +++++++++++++++++-- 4 files changed, 51 insertions(+), 23 deletions(-) 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 ca8e70b89d0..4a6c7b40fe1 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 @@ -11,14 +11,16 @@ import { Injectable, OnDestroy, ɵformatRuntimeError as formatRuntimeError, + PLATFORM_ID, } from '@angular/core'; -import {DOCUMENT} from '../../dom_tokens'; import {RuntimeErrorCode} from '../../errors'; import {assertDevMode} from './asserts'; import {imgDirectiveDetails} from './error_helper'; import {getUrl} from './url'; +import {PlatformLocation} from '../../location'; +import {isPlatformBrowser} from '../../platform_id'; interface ObservedImageState { priority: boolean; @@ -42,14 +44,14 @@ export class LCPImageObserver implements OnDestroy { // Map of full image URLs -> original `ngSrc` values. private images = new Map(); - private window: Window | null = null; private observer: PerformanceObserver | null = null; + private readonly baseUrl = inject(PlatformLocation).href; constructor() { + const isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); + assertDevMode('LCP checker'); - const win = inject(DOCUMENT).defaultView; - if (typeof win !== 'undefined' && typeof PerformanceObserver !== 'undefined') { - this.window = win; + if (isBrowser && typeof PerformanceObserver !== 'undefined') { this.observer = this.initPerformanceObserver(); } } @@ -98,20 +100,20 @@ export class LCPImageObserver implements OnDestroy { alreadyWarnedModified: false, alreadyWarnedPriority: false, }; - this.images.set(getUrl(rewrittenSrc, this.window!).href, newObservedImageState); + this.images.set(getUrl(rewrittenSrc, this.baseUrl).href, newObservedImageState); } unregisterImage(rewrittenSrc: string) { if (!this.observer) return; - this.images.delete(getUrl(rewrittenSrc, this.window!).href); + this.images.delete(getUrl(rewrittenSrc, this.baseUrl).href); } updateImage(originalSrc: string, newSrc: string) { - const originalUrl = getUrl(originalSrc, this.window!).href; + const originalUrl = getUrl(originalSrc, this.baseUrl).href; const img = this.images.get(originalUrl); if (img) { img.modified = true; - this.images.set(getUrl(newSrc, this.window!).href, img); + this.images.set(getUrl(newSrc, this.baseUrl).href, img); this.images.delete(originalUrl); } } 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 index 3f87fdabbaf..79ab544ac19 100644 --- a/packages/common/src/directives/ng_optimized_image/preconnect_link_checker.ts +++ b/packages/common/src/directives/ng_optimized_image/preconnect_link_checker.ts @@ -11,7 +11,7 @@ import { Injectable, InjectionToken, ɵformatRuntimeError as formatRuntimeError, - ɵRuntimeError as RuntimeError, + PLATFORM_ID, } from '@angular/core'; import {DOCUMENT} from '../../dom_tokens'; @@ -20,6 +20,8 @@ import {RuntimeErrorCode} from '../../errors'; import {assertDevMode} from './asserts'; import {imgDirectiveDetails} from './error_helper'; import {extractHostname, getUrl} from './url'; +import {isPlatformServer} from '../../platform_id'; +import {PlatformLocation} from '../../location'; // Set of origins that are always excluded from the preconnect checks. 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 tags found on this page. @@ -68,16 +72,10 @@ export class PreconnectLinkChecker { */ private alreadySeen = new Set(); - private window: Window | null = null; - private blocklist = new Set(INTERNAL_PRECONNECT_CHECK_BLOCKLIST); constructor() { assertDevMode('preconnect link checker'); - const win = this.document.defaultView; - if (typeof win !== 'undefined') { - this.window = win; - } const blocklist = inject(PRECONNECT_CHECK_BLOCKLIST, {optional: true}); if (blocklist) { this.populateBlocklist(blocklist); @@ -102,9 +100,9 @@ export class PreconnectLinkChecker { * @param originalNgSrc ngSrc value */ assertPreconnect(rewrittenSrc: string, originalNgSrc: string): void { - if (!this.window) return; + if (this.isServer) return; - const imgUrl = getUrl(rewrittenSrc, this.window); + const imgUrl = getUrl(rewrittenSrc, this.platformLocation.href); if (this.blocklist.has(imgUrl.hostname) || this.alreadySeen.has(imgUrl.origin)) return; // Register this origin as seen, so we don't check it again later. @@ -135,7 +133,7 @@ export class PreconnectLinkChecker { const selector = 'link[rel=preconnect]'; const links: HTMLLinkElement[] = Array.from(this.document.querySelectorAll(selector)); for (let link of links) { - const url = getUrl(link.href, this.window!); + const url = getUrl(link.href, this.platformLocation.href); preconnectUrls.add(url.origin); } return preconnectUrls; diff --git a/packages/common/src/directives/ng_optimized_image/url.ts b/packages/common/src/directives/ng_optimized_image/url.ts index 26f8224c5d8..6c7d8de11fa 100644 --- a/packages/common/src/directives/ng_optimized_image/url.ts +++ b/packages/common/src/directives/ng_optimized_image/url.ts @@ -7,9 +7,9 @@ */ // Converts a string that represents a URL into a URL class instance. -export function getUrl(src: string, win: Window): URL { +export function getUrl(src: string, href: string): URL { // Don't use a base URL is the URL is absolute. - return isAbsoluteUrl(src) ? new URL(src) : new URL(src, win.location.href); + return isAbsoluteUrl(src) ? new URL(src) : new URL(src, href); } // Checks whether a URL is absolute (i.e. starts with `http://` or `https://`). diff --git a/packages/common/test/directives/ng_optimized_image_spec.ts b/packages/common/test/directives/ng_optimized_image_spec.ts index 04496b19e55..f446e6922f4 100644 --- a/packages/common/test/directives/ng_optimized_image_spec.ts +++ b/packages/common/test/directives/ng_optimized_image_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {CommonModule, DOCUMENT, IMAGE_CONFIG, ImageConfig} from '@angular/common'; +import {CommonModule, DOCUMENT, IMAGE_CONFIG, ImageConfig, PlatformLocation} from '@angular/common'; import {RuntimeErrorCode} from '@angular/common/src/errors'; import {PLATFORM_SERVER_ID} from '@angular/common/src/platform_id'; import {ChangeDetectionStrategy, Component, PLATFORM_ID, Provider, Type} from '@angular/core'; @@ -32,6 +32,7 @@ import { RECOMMENDED_SRCSET_DENSITY_CAP, } from '../../src/directives/ng_optimized_image/ng_optimized_image'; import {PRECONNECT_CHECK_BLOCKLIST} from '../../src/directives/ng_optimized_image/preconnect_link_checker'; +import {MockPlatformLocation} from '@angular/common/testing'; describe('Image directive', () => { describe('preload element on a server', () => { @@ -1243,6 +1244,9 @@ describe('Image directive', () => { it( 'should log a warning if there is no preconnect link for a priority image', withHead('', () => { + // The warning is only logged on the client + if (!isBrowser) return; + setupTestingModule({imageLoader}); const consoleWarnSpy = spyOn(console, 'warn'); @@ -1281,6 +1285,9 @@ describe('Image directive', () => { it( "should log a warning if there is a preconnect, but it doesn't match the priority image", withHead('', () => { + // The warning is only logged on the client + if (!isBrowser) return; + setupTestingModule({imageLoader}); const consoleWarnSpy = spyOn(console, 'warn'); @@ -1305,6 +1312,9 @@ describe('Image directive', () => { withHead( '', () => { + // The warning is only logged on the client + if (!isBrowser) return; + setupTestingModule({imageLoader}); const consoleWarnSpy = spyOn(console, 'warn'); @@ -2165,6 +2175,14 @@ function setupTestingModule(config?: { {provide: DOCUMENT, useValue: window.document}, ...(config?.noLoader ? [] : [{provide: IMAGE_LOADER, useValue: loader}]), ...extraProviders, + { + provide: PlatformLocation, + useFactory: () => { + return new MockPlatformLocation({ + appBaseHref: 'https://angular.io/', + }); + }, + }, ]; if (config?.imageConfig) { providers.push({provide: IMAGE_CONFIG, useValue: config.imageConfig}); @@ -2185,7 +2203,17 @@ function setUpModuleNoLoader() { TestBed.configureTestingModule({ declarations: [TestComponent], imports: [CommonModule, NgOptimizedImage], - providers: [{provide: DOCUMENT, useValue: window.document}], + providers: [ + {provide: DOCUMENT, useValue: window.document}, + { + provide: PlatformLocation, + useFactory: () => { + return new MockPlatformLocation({ + appBaseHref: 'https://angular.io/', + }); + }, + }, + ], }); }