From 7f961817380061c7b234318f45583a0b9fce68f5 Mon Sep 17 00:00:00 2001 From: johannesjo Date: Thu, 14 May 2026 12:40:51 +0200 Subject: [PATCH] fix(pwa): avoid startup stalls during network changes --- ngsw-config.json | 18 +-- .../initial-pwa-update-check.service.spec.ts | 128 ++++++++++++++++++ .../core/initial-pwa-update-check.service.ts | 14 +- .../ui/material-icons-loader.service.spec.ts | 75 +++++++++- src/app/ui/material-icons-loader.service.ts | 22 ++- 5 files changed, 240 insertions(+), 17 deletions(-) create mode 100644 src/app/core/initial-pwa-update-check.service.spec.ts diff --git a/ngsw-config.json b/ngsw-config.json index 027de8ea85..60ee244c93 100644 --- a/ngsw-config.json +++ b/ngsw-config.json @@ -9,14 +9,6 @@ "files": ["/favicon.ico", "/index.html", "/manifest.json", "/*.css", "/*.js"] } }, - { - "name": "assets", - "installMode": "lazy", - "updateMode": "prefetch", - "resources": { - "files": ["/assets/**"] - } - }, { "name": "criticalAssets", "installMode": "prefetch", @@ -27,11 +19,19 @@ "/assets/icons/icon-*.png", "/assets/icons/favicon-*.png", "/assets/i18n/en.json", - "/assets/fonts/MaterialIcons-Regular.ttf", + "/assets/fonts/material-symbols-outlined.woff2", "/*.(otf|ttf|woff|woff2)" ] } }, + { + "name": "assets", + "installMode": "lazy", + "updateMode": "prefetch", + "resources": { + "files": ["/assets/**"] + } + }, { "name": "externalAssets", "installMode": "lazy", diff --git a/src/app/core/initial-pwa-update-check.service.spec.ts b/src/app/core/initial-pwa-update-check.service.spec.ts new file mode 100644 index 0000000000..8fc84a2d45 --- /dev/null +++ b/src/app/core/initial-pwa-update-check.service.spec.ts @@ -0,0 +1,128 @@ +import { fakeAsync, flushMicrotasks, TestBed, tick } from '@angular/core/testing'; +import { InitialPwaUpdateCheckService } from './initial-pwa-update-check.service'; +import { SwUpdate } from '@angular/service-worker'; +import { TranslateService } from '@ngx-translate/core'; +import { firstValueFrom } from 'rxjs'; +import { Log } from './log'; + +const INITIAL_PWA_UPDATE_CHECK_TIMEOUT_MS = 8000; + +describe('InitialPwaUpdateCheckService', () => { + let originalOnLineDescriptor: PropertyDescriptor | undefined; + + beforeEach(() => { + originalOnLineDescriptor = Object.getOwnPropertyDescriptor(navigator, 'onLine'); + Object.defineProperty(navigator, 'onLine', { + value: true, + configurable: true, + }); + }); + + afterEach(() => { + restoreNavigatorOnLine(originalOnLineDescriptor); + TestBed.resetTestingModule(); + }); + + it('should defer the update check until subscribed', () => { + const { service, swUpdate } = setup(Promise.resolve(false)); + + expect(swUpdate.checkForUpdate).not.toHaveBeenCalled(); + + const sub = service.afterInitialUpdateCheck$.subscribe(); + + expect(swUpdate.checkForUpdate).toHaveBeenCalledTimes(1); + sub.unsubscribe(); + }); + + it('should emit when no update is available', async () => { + const { service } = setup(Promise.resolve(false)); + + const result = await firstValueFrom(service.afterInitialUpdateCheck$); + + expect(result).toBeUndefined(); + }); + + it('should share the initial update check result', async () => { + const { service, swUpdate } = setup(Promise.resolve(false)); + + await firstValueFrom(service.afterInitialUpdateCheck$); + await firstValueFrom(service.afterInitialUpdateCheck$); + + expect(swUpdate.checkForUpdate).toHaveBeenCalledTimes(1); + }); + + it('should emit when the update check rejects', async () => { + spyOn(Log, 'warn'); + const { service } = setup(Promise.reject(new Error('network failed'))); + + const result = await firstValueFrom(service.afterInitialUpdateCheck$); + + expect(result).toBeUndefined(); + expect(Log.warn).toHaveBeenCalled(); + }); + + it('should emit when the update check stalls', fakeAsync(() => { + spyOn(Log, 'warn'); + const { service } = setup(new Promise(() => undefined)); + let didEmit = false; + + service.afterInitialUpdateCheck$.subscribe(() => { + didEmit = true; + }); + tick(INITIAL_PWA_UPDATE_CHECK_TIMEOUT_MS); + flushMicrotasks(); + + expect(didEmit).toBeTrue(); + expect(Log.warn).toHaveBeenCalled(); + })); + + it('should skip the update check when service worker updates are disabled', async () => { + const { service, swUpdate } = setup(Promise.resolve(false), false); + + const result = await firstValueFrom(service.afterInitialUpdateCheck$); + + expect(result).toBeUndefined(); + expect(swUpdate.checkForUpdate).not.toHaveBeenCalled(); + }); +}); + +const setup = ( + checkForUpdateResult: Promise, + isEnabled: boolean = true, +): { + service: InitialPwaUpdateCheckService; + swUpdate: jasmine.SpyObj; +} => { + const swUpdate = jasmine.createSpyObj('SwUpdate', ['checkForUpdate']); + Object.defineProperty(swUpdate, 'isEnabled', { + value: isEnabled, + configurable: true, + }); + swUpdate.checkForUpdate.and.returnValue(checkForUpdateResult); + + const translateService = jasmine.createSpyObj('TranslateService', [ + 'instant', + ]); + translateService.instant.and.returnValue('Update?'); + + TestBed.configureTestingModule({ + providers: [ + InitialPwaUpdateCheckService, + { provide: SwUpdate, useValue: swUpdate }, + { provide: TranslateService, useValue: translateService }, + ], + }); + + return { + service: TestBed.inject(InitialPwaUpdateCheckService), + swUpdate, + }; +}; + +const restoreNavigatorOnLine = (descriptor: PropertyDescriptor | undefined): void => { + if (descriptor) { + Object.defineProperty(navigator, 'onLine', descriptor); + } else { + delete (navigator as { onLine?: boolean }).onLine; + } +}; diff --git a/src/app/core/initial-pwa-update-check.service.ts b/src/app/core/initial-pwa-update-check.service.ts index e01187464e..b9c14d5ce5 100644 --- a/src/app/core/initial-pwa-update-check.service.ts +++ b/src/app/core/initial-pwa-update-check.service.ts @@ -1,7 +1,7 @@ import { Injectable, inject } from '@angular/core'; import { IS_ELECTRON } from '../app.constants'; -import { EMPTY, from, Observable, of } from 'rxjs'; -import { concatMap } from 'rxjs/operators'; +import { defer, EMPTY, from, Observable, of } from 'rxjs'; +import { catchError, concatMap, shareReplay, timeout } from 'rxjs/operators'; import { T } from '../t.const'; import { SwUpdate } from '@angular/service-worker'; import { isOnline } from '../util/is-online'; @@ -9,6 +9,8 @@ import { TranslateService } from '@ngx-translate/core'; import { Log } from './log'; import { confirmDialog } from '../util/native-dialogs'; +const INITIAL_PWA_UPDATE_CHECK_TIMEOUT_MS = 8000; + @Injectable({ providedIn: 'root', }) @@ -19,7 +21,12 @@ export class InitialPwaUpdateCheckService { // NOTE: check currently triggered by sync effect afterInitialUpdateCheck$: Observable = !IS_ELECTRON && this._swUpdate.isEnabled && isOnline() - ? from(this._swUpdate.checkForUpdate()).pipe( + ? defer(() => from(this._swUpdate.checkForUpdate())).pipe( + timeout(INITIAL_PWA_UPDATE_CHECK_TIMEOUT_MS), + catchError((err: unknown) => { + Log.warn('InitialPwaUpdateCheckService: update check failed', err); + return of(false); + }), concatMap((isUpdateAvailable) => { Log.log( '___________isServiceWorkerUpdateAvailable____________', @@ -34,6 +41,7 @@ export class InitialPwaUpdateCheckService { } return of(undefined); }), + shareReplay(1), ) : of(undefined); } diff --git a/src/app/ui/material-icons-loader.service.spec.ts b/src/app/ui/material-icons-loader.service.spec.ts index 7e687003fd..0d9befa6c5 100644 --- a/src/app/ui/material-icons-loader.service.spec.ts +++ b/src/app/ui/material-icons-loader.service.spec.ts @@ -1,14 +1,26 @@ -import { TestBed } from '@angular/core/testing'; +import { fakeAsync, flushMicrotasks, TestBed, tick } from '@angular/core/testing'; import { MaterialIconsLoaderService } from './material-icons-loader.service'; +import { BodyClass } from '../app.constants'; + +const MATERIAL_ICONS_FONT = '24px "Material Symbols Outlined"'; +const MATERIAL_ICONS_FONT_READY_TIMEOUT_MS = 3000; describe('MaterialIconsLoaderService', () => { let service: MaterialIconsLoaderService; + let originalFonts: FontFaceSet | undefined; beforeEach(() => { + originalFonts = document.fonts; + document.body.classList.remove(BodyClass.isMaterialSymbolsLoaded); TestBed.configureTestingModule({}); service = TestBed.inject(MaterialIconsLoaderService); }); + afterEach(() => { + setDocumentFonts(originalFonts); + document.body.classList.remove(BodyClass.isMaterialSymbolsLoaded); + }); + it('should load icons on first call', async () => { const icons = await service.loadIcons(); expect(icons).toBeDefined(); @@ -28,4 +40,65 @@ describe('MaterialIconsLoaderService', () => { ]); expect(icons1).toBe(icons2); }); + + it('should reveal icons when FontFaceSet is unavailable', async () => { + setDocumentFonts(undefined); + + await service.ensureFontReady(); + + expect( + document.body.classList.contains(BodyClass.isMaterialSymbolsLoaded), + ).toBeTrue(); + }); + + it('should wait for the material symbols font before revealing icons', async () => { + const load = jasmine.createSpy('load').and.resolveTo([]); + setDocumentFonts({ load } as unknown as FontFaceSet); + + await service.ensureFontReady(); + + expect(load).toHaveBeenCalledWith(MATERIAL_ICONS_FONT); + expect( + document.body.classList.contains(BodyClass.isMaterialSymbolsLoaded), + ).toBeTrue(); + }); + + it('should reveal icons if the font readiness probe stalls', fakeAsync(() => { + const load = jasmine + .createSpy('load') + .and.returnValue(new Promise(() => undefined)); + setDocumentFonts({ load } as unknown as FontFaceSet); + let didResolve = false; + + service.ensureFontReady().then(() => { + didResolve = true; + }); + tick(MATERIAL_ICONS_FONT_READY_TIMEOUT_MS); + flushMicrotasks(); + + expect(didResolve).toBeTrue(); + expect( + document.body.classList.contains(BodyClass.isMaterialSymbolsLoaded), + ).toBeTrue(); + })); + + it('should reveal icons if the font readiness probe rejects', async () => { + const load = jasmine + .createSpy('load') + .and.returnValue(Promise.reject(new Error('font failed'))); + setDocumentFonts({ load } as unknown as FontFaceSet); + + await service.ensureFontReady(); + + expect( + document.body.classList.contains(BodyClass.isMaterialSymbolsLoaded), + ).toBeTrue(); + }); }); + +const setDocumentFonts = (fonts: FontFaceSet | undefined): void => { + Object.defineProperty(document, 'fonts', { + value: fonts, + configurable: true, + }); +}; diff --git a/src/app/ui/material-icons-loader.service.ts b/src/app/ui/material-icons-loader.service.ts index 918c5514e0..c11a19778e 100644 --- a/src/app/ui/material-icons-loader.service.ts +++ b/src/app/ui/material-icons-loader.service.ts @@ -2,6 +2,7 @@ import { Injectable } from '@angular/core'; import { BodyClass } from '../app.constants'; const MATERIAL_ICONS_FONT = '24px "Material Symbols Outlined"'; +const MATERIAL_ICONS_FONT_READY_TIMEOUT_MS = 3000; @Injectable({ providedIn: 'root', @@ -41,7 +42,8 @@ export class MaterialIconsLoaderService { return Promise.resolve(); } - if (!('fonts' in document)) { + const fonts = document.fonts; + if (!fonts?.load) { body.classList.add(BodyClass.isMaterialSymbolsLoaded); return Promise.resolve(); } @@ -50,7 +52,7 @@ export class MaterialIconsLoaderService { return this.fontReadyPromise; } - this.fontReadyPromise = this._loadFont(body); + this.fontReadyPromise = this._loadFont(body, fonts); return this.fontReadyPromise; } @@ -60,10 +62,22 @@ export class MaterialIconsLoaderService { return MATERIAL_ICONS; } - private async _loadFont(body: HTMLElement): Promise { + private async _loadFont(body: HTMLElement, fonts: FontFaceSet): Promise { + let timeoutId: ReturnType | undefined; + try { - await document.fonts.load(MATERIAL_ICONS_FONT); + await Promise.race([ + fonts.load(MATERIAL_ICONS_FONT), + new Promise((resolve) => { + timeoutId = setTimeout(resolve, MATERIAL_ICONS_FONT_READY_TIMEOUT_MS); + }), + ]); + } catch { + // Keep the UI usable if the browser fails the font readiness probe. } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } body.classList.add(BodyClass.isMaterialSymbolsLoaded); } }