ZCU-PUB/Backport: Fix home-page SSR->CSR flicker

.gitignore

@@ -6,6 +6,9 @@
 /tsd_typings/
 npm-debug.log
 
+# build/install/spec logs emitted by local deploy tooling
+/_*.log
+
 /build/
 
 /coverage

angular.json

@@ -105,8 +105,8 @@
               "budgets": [
                 {
                   "type": "initial",
-                  "maximumWarning": "3mb",
-                  "maximumError": "5mb"
+                  "maximumWarning": "5.5mb",
+                  "maximumError": "6mb"
                 },
                 {
                   "type": "anyComponentStyle",

src/app/app.component.spec.ts

@@ -1,9 +1,10 @@
 import { Store, StoreModule } from '@ngrx/store';
-import { ComponentFixture, inject, TestBed, waitForAsync } from '@angular/core/testing';
+import { ComponentFixture, discardPeriodicTasks, fakeAsync, inject, TestBed, tick, waitForAsync } from '@angular/core/testing';
 import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
 import { CommonModule } from '@angular/common';
 import { ActivatedRoute, Router } from '@angular/router';
 import { TranslateLoader, TranslateModule } from '@ngx-translate/core';
+import { BehaviorSubject } from 'rxjs';
 
 // Load the implementations that should be tested
 import { AppComponent } from './app.component';
@@ -30,7 +31,7 @@ import { Angulartics2DSpace } from './statistics/angulartics/dspace-provider';
 import { storeModuleConfig } from './app.reducer';
 import { LocaleService } from './core/locale/locale.service';
 import { authReducer } from './core/auth/auth.reducer';
-import { provideMockStore } from '@ngrx/store/testing';
+import { MockStore, provideMockStore } from '@ngrx/store/testing';
 import { ThemeService } from './shared/theme-support/theme.service';
 import { getMockThemeService } from './shared/mocks/theme-service.mock';
 import { BreadcrumbsService } from './breadcrumbs/breadcrumbs.service';
@@ -41,7 +42,7 @@ let comp: AppComponent;
 let fixture: ComponentFixture<AppComponent>;
 const menuService = new MenuServiceStub();
 const initialState = {
-  core: { auth: { loading: false } }
+  core: { auth: { loading: false, blocking: false } }
 };
 
 export function getMockLocaleService(): LocaleService {
@@ -127,4 +128,78 @@ describe('App component', () => {
     });
 
   });
+
+  describe('removeSsrOverlayWhenContentVisible', () => {
+    // The inline bootstrap script in src/index.html injects window.__dspaceRemoveSsrOverlay.
+    // Once auth blocking and theme loading are both false, AppComponent waits for the <ds-app> DOM
+    // to settle (no element added/removed for the quiet window) and only then removes the overlay.
+    let mockStore: MockStore;
+    let themeLoading$: BehaviorSubject<boolean>;
+    let themeService: ThemeService;
+    let originalRaF: typeof window.requestAnimationFrame;
+    let dsAppEl: HTMLElement;
+
+    beforeEach(() => {
+      mockStore = TestBed.inject(MockStore);
+      themeService = TestBed.inject(ThemeService);
+      themeLoading$ = new BehaviorSubject<boolean>(true);
+      (themeService as any).isThemeLoading$ = themeLoading$.asObservable();
+      mockStore.setState({ core: { auth: { loading: false, blocking: true } } });
+
+      // A settled <ds-app> with real content present, so the DOM-settle watcher can resolve.
+      dsAppEl = document.createElement('ds-app');
+      dsAppEl.setAttribute('style', 'display:block;height:800px');
+      dsAppEl.innerHTML = '<main id="main-content" style="display:block;height:800px">home content</main>';
+      document.body.appendChild(dsAppEl);
+
+      // Force rAF to a synchronous shim so assertions are deterministic.
+      originalRaF = window.requestAnimationFrame;
+      (window as any).requestAnimationFrame = (cb: FrameRequestCallback) => {
+        cb(0);
+        return 0 as any;
+      };
+    });
+
+    afterEach(() => {
+      (window as any).requestAnimationFrame = originalRaF;
+      delete (window as any).__dspaceRemoveSsrOverlay;
+      if (dsAppEl && dsAppEl.parentNode) { dsAppEl.parentNode.removeChild(dsAppEl); }
+    });
+
+    it('removes the overlay once auth/theme are ready AND the DOM has settled', fakeAsync(() => {
+      const spy = jasmine.createSpy('__dspaceRemoveSsrOverlay');
+      window.__dspaceRemoveSsrOverlay = spy;
+
+      // Re-construct so constructor-time subscription picks up our patched streams + global.
+      const f = TestBed.createComponent(AppComponent);
+      f.detectChanges();
+
+      expect(spy).not.toHaveBeenCalled();
+
+      mockStore.setState({ core: { auth: { loading: false, blocking: false } } });
+      themeLoading$.next(false);
+
+      // Not removed at the gate: the DOM-settle quiet window must elapse first.
+      expect(spy).not.toHaveBeenCalled();
+      tick(700); // > SETTLE_QUIET_MS
+      expect(spy).toHaveBeenCalledTimes(1);
+
+      discardPeriodicTasks();
+    }));
+
+    it('is a no-op when the global is not injected (e.g. CSR-only route, SSR skipped)', fakeAsync(() => {
+      // Global intentionally absent; constructor should not throw and should not break later.
+      delete (window as any).__dspaceRemoveSsrOverlay;
+
+      const f = TestBed.createComponent(AppComponent);
+      expect(() => f.detectChanges()).not.toThrow();
+
+      mockStore.setState({ core: { auth: { loading: false, blocking: false } } });
+      themeLoading$.next(false);
+      tick(700);
+
+      expect(window.__dspaceRemoveSsrOverlay).toBeUndefined();
+      discardPeriodicTasks();
+    }));
+  });
 });

src/app/app.component.ts

@@ -1,11 +1,13 @@
-import { distinctUntilChanged, take, withLatestFrom } from 'rxjs/operators';
+import { debounceTime, distinctUntilChanged, filter, startWith, switchMap, take, takeUntil, withLatestFrom } from 'rxjs/operators';
 import { DOCUMENT, isPlatformBrowser } from '@angular/common';
 import {
   AfterViewInit,
   ChangeDetectionStrategy,
   Component,
   HostListener,
   Inject,
+  NgZone,
+  OnDestroy,
   OnInit,
   PLATFORM_ID,
 } from '@angular/core';
@@ -16,7 +18,7 @@ import {
   Router,
 } from '@angular/router';
 
-import { BehaviorSubject, Observable } from 'rxjs';
+import { BehaviorSubject, combineLatest, Observable, race, Subject, timer } from 'rxjs';
 import { select, Store } from '@ngrx/store';
 import { NgbModal, NgbModalConfig } from '@ng-bootstrap/ng-bootstrap';
 import { TranslateService } from '@ngx-translate/core';
@@ -38,10 +40,22 @@ import { distinctNext } from './core/shared/distinct-next';
   styleUrls: ['./app.component.scss'],
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class AppComponent implements OnInit, AfterViewInit {
+export class AppComponent implements OnInit, AfterViewInit, OnDestroy {
   notificationOptions;
   models;
 
+  /**
+   * Emits on destroy to tear down the SSR-overlay-removal pipeline (gate subscription, the
+   * MutationObserver and its debounce/cap timers all unsubscribe via `takeUntil(destroyed$)`).
+   * AppComponent is the root component so this is mostly defensive + test hygiene.
+   */
+  private destroyed$ = new Subject<void>();
+
+  /** SSR anti-flicker overlay (see src/index.html) removal tuning. */
+  private readonly ssrOverlaySettleQuietMs = 600;      // routed page is "done" after this long with no DOM change
+  private readonly ssrOverlaySettleMaxMs = 10000;      // backstop reveal (below index.html's 15s catastrophic net)
+  private readonly ssrOverlayMinContentHeightPx = 200; // proves <ds-app> is no longer the empty shell
+
   /**
    * Whether or not the authentication is currently blocking the UI
    */
@@ -74,6 +88,7 @@ export class AppComponent implements OnInit, AfterViewInit {
     private cssService: CSSVariableService,
     private modalService: NgbModal,
     private modalConfig: NgbModalConfig,
+    private ngZone: NgZone,
   ) {
     this.notificationOptions = environment.notifications;
 
@@ -82,13 +97,129 @@ export class AppComponent implements OnInit, AfterViewInit {
 
     if (isPlatformBrowser(this.platformId)) {
       this.trackIdleModal();
+      this.removeSsrOverlayWhenContentVisible();
     }
 
     this.isThemeLoading$ = this.themeService.isThemeLoading$;
 
     this.storeCSSVariables();
   }
 
+  /**
+   * Drops the SSR mask overlay installed by the inline bootstrap script in src/index.html once the
+   * routed CSR page has actually finished rendering. Picking the right moment is the whole problem,
+   * and the two earlier attempts each fixed one symptom and reintroduced the other:
+   *
+   *  - PR #1288 waited for `ApplicationRef.isStable`. No flicker, but isStable is held hostage by ANY
+   *    ongoing zone async — after an admin login the app keeps the zone busy (authz/widgets, periodic
+   *    polling, AAI/discojuice scripts) so isStable fires many seconds late (or hits the 15s
+   *    fallback). The inert snapshot then masks the live page -> "looks rendered but not interactive"
+   *    (issue #725).
+   *  - PR #1317 switched to the loader-swap gate `!isAuthenticationBlocking && !isThemeLoading` plus a
+   *    single requestAnimationFrame. Prompt, but that gate only un-hides `<router-outlet>`; the home
+   *    page then renders piecewise (navbar, search box, community list, recent submissions) as each
+   *    section's data arrives. Dropping the snapshot at the gate — or, as an earlier revision of this
+   *    method did, as soon as *some* content exists — exposes a half-built page that visibly pops
+   *    into place on a hard reload -> the flicker.
+   *
+   * The signal we actually need is "the routed page has stopped changing". So, after the gate opens,
+   * we keep the snapshot until the live <ds-app> DOM has SETTLED (no element added/removed for a
+   * short quiet window, with real content present). This stays decoupled from isStable (DOM-settle
+   * ignores non-rendering background async, so admin reveals in a few seconds rather than ~15s).
+   * See {@link routedPageReadyToReveal$}.
+   */
+  private removeSsrOverlayWhenContentVisible(): void {
+    const win: Window | undefined = this._window?.nativeWindow;
+    if (!win || typeof win.__dspaceRemoveSsrOverlay !== 'function') {
+      return; // SSR was skipped for this route, so no overlay was installed — nothing to remove
+    }
+    // Run outside Angular: a MutationObserver watching the whole app must not trigger change
+    // detection (it would also keep ApplicationRef.isStable permanently false).
+    this.ngZone.runOutsideAngular(() => {
+      this.routedPageReadyToReveal$().pipe(
+        takeUntil(this.destroyed$),
+      ).subscribe(() => {
+        // one frame so the freshly rendered content is painted before the snapshot fades out
+        this.runAfterNextFrame(win, () => win.__dspaceRemoveSsrOverlay?.());
+      });
+    });
+  }
+
+  /**
+   * Emits once when it is safe to drop the SSR snapshot: the auth/theme loader gate has opened
+   * (same condition root.component.html uses to swap its fullscreen loader for the routed content)
+   * AND the routed page's DOM has settled. See {@link dsAppDomSettled$}.
+   */
+  private routedPageReadyToReveal$(): Observable<unknown> {
+    const loaderGateOpen$ = combineLatest([
+      this.store.pipe(select(isAuthenticationBlocking), distinctUntilChanged()),
+      this.themeService.isThemeLoading$,
+    ]).pipe(
+      filter(([authBlocking, themeLoading]: [boolean, boolean]) => !authBlocking && !themeLoading),
+      take(1),
+    );
+    return loaderGateOpen$.pipe(
+      switchMap(() => this.dsAppDomSettled$()),
+    );
+  }
+
+  /**
+   * Emits once when the live <ds-app> subtree stops being mutated (elements added/removed) for
+   * `ssrOverlaySettleQuietMs` AND it holds real content — or after `ssrOverlaySettleMaxMs`, whichever
+   * comes first. The cap guarantees a page that never goes quiet (constant background DOM updates)
+   * still reveals; the 15s fallback in index.html remains the ultimate net.
+   */
+  private dsAppDomSettled$(): Observable<unknown> {
+    const dsApp: Element | null = this.document.querySelector('ds-app');
+    if (!dsApp) {
+      return timer(this.ssrOverlaySettleMaxMs);
+    }
+    const elementMutations$ = new Observable<void>((subscriber) => {
+      const observer = new MutationObserver((records) => {
+        if (records.some((record) => this.isElementChildListChange(record))) {
+          subscriber.next();
+        }
+      });
+      observer.observe(dsApp, { childList: true, subtree: true });
+      return () => observer.disconnect();
+    });
+    const settled$ = elementMutations$.pipe(
+      startWith(undefined),                              // start the quiet window immediately
+      debounceTime(this.ssrOverlaySettleQuietMs),        // ... reset by each render, fires once quiet
+      filter(() => this.dsAppHasRenderedContent(dsApp)), // ... but never on the empty shell
+    );
+    return race(settled$, timer(this.ssrOverlaySettleMaxMs)).pipe(take(1));
+  }
+
+  /** True once the live <ds-app> is no longer the empty shell the overlay script left behind. */
+  private dsAppHasRenderedContent(dsApp: Element): boolean {
+    const height = dsApp.getBoundingClientRect?.().height ?? 0;
+    return height >= this.ssrOverlayMinContentHeightPx && dsApp.querySelector('#main-content') !== null;
+  }
+
+  /** A childList mutation that adds or removes at least one element node (ignores text/attr noise). */
+  private isElementChildListChange(record: MutationRecord): boolean {
+    if (record.type !== 'childList') {
+      return false;
+    }
+    const changedNodes = [...Array.from(record.addedNodes), ...Array.from(record.removedNodes)];
+    return changedNodes.some((node) => node.nodeType === Node.ELEMENT_NODE);
+  }
+
+  /** Runs `callback` after the next paint (or synchronously if requestAnimationFrame is unavailable). */
+  private runAfterNextFrame(win: Window, callback: () => void): void {
+    if (typeof win.requestAnimationFrame === 'function') {
+      win.requestAnimationFrame(() => callback());
+    } else {
+      callback();
+    }
+  }
+
+  ngOnDestroy(): void {
+    this.destroyed$.next();
+    this.destroyed$.complete();
+  }
+
   ngOnInit() {
     /** Implement behavior for interface {@link ModalBeforeDismiss} */
     this.modalConfig.beforeDismiss = async function () {

src/app/shared/mocks/theme-service.mock.ts

@@ -4,11 +4,18 @@ import { ThemeConfig } from '../../../config/theme.config';
 import { isNotEmpty } from '../empty.util';
 
 export function getMockThemeService(themeName = 'base', themes?: ThemeConfig[]): ThemeService {
+  // getThemeName$ is a real method on ThemeService (called as getThemeName$()),
+  // so it must stay a spy method that returns an Observable.
+  // isThemeLoading$ is a real property getter on ThemeService, so it must be a
+  // property on the mock - not a spy method - or AsyncPipe / combineLatest will
+  // receive a function instead of a stream.
   const spy = jasmine.createSpyObj('themeService', {
     getThemeName: themeName,
     getThemeName$: observableOf(themeName),
     getThemeConfigFor: undefined,
     listenForRouteChanges: undefined,
+  }, {
+    isThemeLoading$: observableOf(false),
   });
 
   if (isNotEmpty(themes)) {