Skip to content

Commit

Permalink
fix(common): Don't run preconnect assertion on the server. (angular#5…
Browse files Browse the repository at this point in the history
…6213)

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 angular#56207

PR Close angular#56213
  • Loading branch information
JeanMeche authored and atscott committed Jul 16, 2024
1 parent e5e1f49 commit 2c4613a
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
Injectable,
OnDestroy,
ɵformatRuntimeError as formatRuntimeError,
PLATFORM_ID,
} from '@angular/core';

import {DOCUMENT} from '../../dom_tokens';
Expand All @@ -19,6 +20,7 @@ import {RuntimeErrorCode} from '../../errors';
import {assertDevMode} from './asserts';
import {imgDirectiveDetails} from './error_helper';
import {getUrl} from './url';
import {isPlatformBrowser} from '../../platform_id';

interface ObservedImageState {
priority: boolean;
Expand Down Expand Up @@ -46,9 +48,10 @@ export class LCPImageObserver implements OnDestroy {
private observer: PerformanceObserver | null = null;

constructor() {
const isBrowser = isPlatformBrowser(inject(PLATFORM_ID));
assertDevMode('LCP checker');
const win = inject(DOCUMENT).defaultView;
if (typeof win !== 'undefined' && typeof PerformanceObserver !== 'undefined') {
if (isBrowser && typeof PerformanceObserver !== 'undefined') {
this.window = win;
this.observer = this.initPerformanceObserver();
}
Expand Down Expand Up @@ -107,6 +110,7 @@ export class LCPImageObserver implements OnDestroy {
}

updateImage(originalSrc: string, newSrc: string) {
if (!this.observer) return;
const originalUrl = getUrl(originalSrc, this.window!).href;
const img = this.images.get(originalUrl);
if (img) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
Injectable,
InjectionToken,
ɵformatRuntimeError as formatRuntimeError,
ɵRuntimeError as RuntimeError,
PLATFORM_ID,
} from '@angular/core';

import {DOCUMENT} from '../../dom_tokens';
Expand All @@ -20,6 +20,7 @@ import {RuntimeErrorCode} from '../../errors';
import {assertDevMode} from './asserts';
import {imgDirectiveDetails} from './error_helper';
import {extractHostname, getUrl} from './url';
import {isPlatformServer} from '../../platform_id';

// 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']);
Expand Down Expand Up @@ -56,6 +57,7 @@ export const PRECONNECT_CHECK_BLOCKLIST = new InjectionToken<Array<string | stri
@Injectable({providedIn: 'root'})
export class PreconnectLinkChecker {
private document = inject(DOCUMENT);
private readonly isServer = isPlatformServer(inject(PLATFORM_ID));

/**
* Set of <link rel="preconnect"> tags found on this page.
Expand Down Expand Up @@ -102,9 +104,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.window!);
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.
Expand Down
9 changes: 9 additions & 0 deletions packages/common/test/directives/ng_optimized_image_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1360,6 +1360,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');
Expand Down Expand Up @@ -1398,6 +1401,9 @@ describe('Image directive', () => {
it(
"should log a warning if there is a preconnect, but it doesn't match the priority image",
withHead('<link rel="preconnect" href="http://angular.io">', () => {
// The warning is only logged on the client
if (!isBrowser) return;

setupTestingModule({imageLoader});

const consoleWarnSpy = spyOn(console, 'warn');
Expand All @@ -1422,6 +1428,9 @@ describe('Image directive', () => {
withHead(
'<link rel="preload" href="https://angular.io/assets/images/logos/angular/angular.svg" as="image">',
() => {
// The warning is only logged on the client
if (!isBrowser) return;

setupTestingModule({imageLoader});

const consoleWarnSpy = spyOn(console, 'warn');
Expand Down

0 comments on commit 2c4613a

Please sign in to comment.