Skip to content

Commit 5c9c6bc

Browse files
vidiecanjmjr-rkclaude
authored
Fix home-page SSR->CSR flicker (#1287)
* Fix home-page SSR->CSR flicker Angular 15 has no provideClientHydration; on every browser load Angular tears down the entire SSR DOM and rebuilds the component tree from scratch. Measured CLS = 0.89 at t=1.76s on /home (PerformanceObserver on dev-5). The visible flicker is that ~600ms rebuild window between SSR view and populated CSR view. Two compounding causes, addressed in this PR: 1. CustomEagerThemeModule was commented out in src/themes/eager-themes.module.ts, so every custom-themed wrapper (footer, header, root, ...) was lazy-loaded via webpack code-splitting on the browser, stretching the gap. Re-enable it (the existing custom/eager-theme.module.ts already declares the right set). Bumps initial bundle by ~256KB; angular.json budget raised from 5MB to 8MB to accommodate. 2. The bigger cause - no hydration - is masked by an inline pre-bootstrap script in src/index.html that: - Captures all <style ng-transition="dspace-angular"> blocks into <style data-dspace-ssr-keep> tags Angular won't strip (Angular removes the originals on bootstrap, which is why a naive overlay renders unstyled). - Moves (not clones) the SSR-rendered <ds-app> children into an absolute-positioned overlay so they keep every live DOM/style detail. - Hides the now-empty <ds-app> via a data-attribute and CSS rule. - Exposes window.__dspaceRemoveSsrOverlay() for AppComponent to call once ApplicationRef.isStable fires (with one rAF + 50ms pad). - 15s safety fallback in case isStable never fires. Bots and no-JS users still get the original SSR <ds-app> (the overlay is JS-added). Real users see continuous SSR-rendered content while CSR rebuilds invisibly underneath, then a 150ms fade reveals the CSR DOM in its final data-loaded state. Verified locally via Service Worker that suppresses the removal: overlay's header height is 80px (proper styling preserved) versus 698px (the unstyled fallback before this fix's style-preservation step). Includes a small Windows cmd deploy helper at scripts/dspace-deploy.bat and matching skill doc at .claude/skills/dspace-deploy/SKILL.md - multi-instance safe local dev stack via the existing docker compose files. * Lint: curly braces around early-return if * Address Copilot review - angular.json: tighten budget back to 5.5MB warn / 6MB error (was 8MB) - index.html: re-entrancy guard on __dspaceRemoveSsrOverlay (null the pointer up-front so the isStable + 15s safety fallback can't double-fade) - index.html: drop aria-hidden from overlay so screen-reader users get the SSR snapshot during boot (ds-app underneath has visibility:hidden which already excludes it from a11y tree) - index.html: console.warn on the overlay-script catch so a silently broken flicker fix is at least diagnosable in DevTools - typings.d.ts: typed Window.__dspaceRemoveSsrOverlay augmentation; drop the `as any` cast in AppComponent.removeSsrOverlayWhenStable - app.component.spec.ts: cover removeSsrOverlayWhenStable (calls the global once on isStable=true; no-op when global absent) - Drop scripts/dspace-deploy.bat + .claude/skills/dspace-deploy/SKILL.md from this PR per request (local dev tooling, will live elsewhere) * Disable SSR overlay when Cypress is driving The overlay holds the SSR-rendered children alongside <ds-app>'s CSR-rendered children during the masking window. Cypress's cy.get(selector) sees both copies, so unique-id selectors return 2 elements and cy.click() fails. The overlay is purely a UX smoothing layer (no behaviour to E2E-validate), so short-circuit when window.Cypress is present. Browser users are unaffected. * Advance root flicker fix to the final DOM-settle overlay mechanism (#1318, #1321) The original overlay removal waited for ApplicationRef.isStable, which is held hostage by post-login/admin zone activity (page looks rendered but is not interactive - dspace-customers#725). Replace it with the final mechanism used across the customer backports: keep the SSR snapshot until the routed <ds-app> DOM has settled (MutationObserver + quiet window, content-height / #main-content check, 10s cap), and make the overlay a purely visual mask so the live app stays interactive underneath. index.html / app.component.ts / spec / typings are synced to the final version; the existing ngAfterViewInit delay(0) is preserved. Keeping the trunk on the final mechanism means new customer branches cut from dtq-dev start correct instead of re-inheriting the old isStable approach. Ref: #1318, #1321 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: jm <jm@maz> Co-authored-by: Juraj Roka <95219754+jr-rk@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a9fc7f5 commit 5c9c6bc

7 files changed

Lines changed: 378 additions & 10 deletions

File tree

.gitignore

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,16 @@ python_data_import/debug.log.txt
4747
python_data_import/logs.txt
4848
python_data_import/date.txt
4949
*/__pycache__/
50+
51+
# Local-only artifacts generated by scripts/dspace-deploy.bat (one-per-instance, never committed)
52+
/docker/.env.dspace-*
53+
/docker/.override.dspace-*.yml
54+
55+
# Playwright MCP captures + investigation artifacts
56+
/.playwright-mcp/
57+
/PROGRESS.md
58+
/flicker-snapshots-*.json
59+
/perf-*.json
60+
/home-initial.png
61+
/overlay-state.png
62+
/verify-*.png

angular.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@
100100
"budgets": [
101101
{
102102
"type": "initial",
103-
"maximumWarning": "3mb",
104-
"maximumError": "5mb"
103+
"maximumWarning": "5.5mb",
104+
"maximumError": "6mb"
105105
},
106106
{
107107
"type": "anyComponentStyle",

src/app/app.component.spec.ts

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { Store, StoreModule } from '@ngrx/store';
2-
import { ComponentFixture, inject, TestBed, waitForAsync } from '@angular/core/testing';
2+
import { ComponentFixture, discardPeriodicTasks, fakeAsync, inject, TestBed, tick, waitForAsync } from '@angular/core/testing';
33
import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
44
import { CommonModule } from '@angular/common';
55
import { ActivatedRoute, Router } from '@angular/router';
66
import { TranslateLoader, TranslateModule } from '@ngx-translate/core';
7+
import { BehaviorSubject } from 'rxjs';
78

89
// Load the implementations that should be tested
910
import { AppComponent } from './app.component';
@@ -30,7 +31,7 @@ import { Angulartics2DSpace } from './statistics/angulartics/dspace-provider';
3031
import { storeModuleConfig } from './app.reducer';
3132
import { LocaleService } from './core/locale/locale.service';
3233
import { authReducer } from './core/auth/auth.reducer';
33-
import { provideMockStore } from '@ngrx/store/testing';
34+
import { MockStore, provideMockStore } from '@ngrx/store/testing';
3435
import { ThemeService } from './shared/theme-support/theme.service';
3536
import { getMockThemeService } from './shared/mocks/theme-service.mock';
3637
import { BreadcrumbsService } from './breadcrumbs/breadcrumbs.service';
@@ -41,7 +42,7 @@ let comp: AppComponent;
4142
let fixture: ComponentFixture<AppComponent>;
4243
const menuService = new MenuServiceStub();
4344
const initialState = {
44-
core: { auth: { loading: false } }
45+
core: { auth: { loading: false, blocking: false } }
4546
};
4647

4748
export function getMockLocaleService(): LocaleService {
@@ -127,4 +128,78 @@ describe('App component', () => {
127128
});
128129

129130
});
131+
132+
describe('removeSsrOverlayWhenContentVisible', () => {
133+
// The inline bootstrap script in src/index.html injects window.__dspaceRemoveSsrOverlay.
134+
// Once auth blocking and theme loading are both false, AppComponent waits for the <ds-app> DOM
135+
// to settle (no element added/removed for the quiet window) and only then removes the overlay.
136+
let mockStore: MockStore;
137+
let themeLoading$: BehaviorSubject<boolean>;
138+
let themeService: ThemeService;
139+
let originalRaF: typeof window.requestAnimationFrame;
140+
let dsAppEl: HTMLElement;
141+
142+
beforeEach(() => {
143+
mockStore = TestBed.inject(MockStore);
144+
themeService = TestBed.inject(ThemeService);
145+
themeLoading$ = new BehaviorSubject<boolean>(true);
146+
(themeService as any).isThemeLoading$ = themeLoading$.asObservable();
147+
mockStore.setState({ core: { auth: { loading: false, blocking: true } } });
148+
149+
// A settled <ds-app> with real content present, so the DOM-settle watcher can resolve.
150+
dsAppEl = document.createElement('ds-app');
151+
dsAppEl.setAttribute('style', 'display:block;height:800px');
152+
dsAppEl.innerHTML = '<main id="main-content" style="display:block;height:800px">home content</main>';
153+
document.body.appendChild(dsAppEl);
154+
155+
// Force rAF to a synchronous shim so assertions are deterministic.
156+
originalRaF = window.requestAnimationFrame;
157+
(window as any).requestAnimationFrame = (cb: FrameRequestCallback) => {
158+
cb(0);
159+
return 0 as any;
160+
};
161+
});
162+
163+
afterEach(() => {
164+
(window as any).requestAnimationFrame = originalRaF;
165+
delete (window as any).__dspaceRemoveSsrOverlay;
166+
if (dsAppEl && dsAppEl.parentNode) { dsAppEl.parentNode.removeChild(dsAppEl); }
167+
});
168+
169+
it('removes the overlay once auth/theme are ready AND the DOM has settled', fakeAsync(() => {
170+
const spy = jasmine.createSpy('__dspaceRemoveSsrOverlay');
171+
window.__dspaceRemoveSsrOverlay = spy;
172+
173+
// Re-construct so constructor-time subscription picks up our patched streams + global.
174+
const f = TestBed.createComponent(AppComponent);
175+
f.detectChanges();
176+
177+
expect(spy).not.toHaveBeenCalled();
178+
179+
mockStore.setState({ core: { auth: { loading: false, blocking: false } } });
180+
themeLoading$.next(false);
181+
182+
// Not removed at the gate: the DOM-settle quiet window must elapse first.
183+
expect(spy).not.toHaveBeenCalled();
184+
tick(700); // > SETTLE_QUIET_MS
185+
expect(spy).toHaveBeenCalledTimes(1);
186+
187+
discardPeriodicTasks();
188+
}));
189+
190+
it('is a no-op when the global is not injected (e.g. CSR-only route, SSR skipped)', fakeAsync(() => {
191+
// Global intentionally absent; constructor should not throw and should not break later.
192+
delete (window as any).__dspaceRemoveSsrOverlay;
193+
194+
const f = TestBed.createComponent(AppComponent);
195+
expect(() => f.detectChanges()).not.toThrow();
196+
197+
mockStore.setState({ core: { auth: { loading: false, blocking: false } } });
198+
themeLoading$.next(false);
199+
tick(700);
200+
201+
expect(window.__dspaceRemoveSsrOverlay).toBeUndefined();
202+
discardPeriodicTasks();
203+
}));
204+
});
130205
});

src/app/app.component.ts

Lines changed: 134 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
import { distinctUntilChanged, take, withLatestFrom, delay } from 'rxjs/operators';
1+
import { debounceTime, distinctUntilChanged, filter, startWith, switchMap, take, takeUntil, withLatestFrom, delay } from 'rxjs/operators';
22
import { DOCUMENT, isPlatformBrowser } from '@angular/common';
33
import {
44
AfterViewInit,
55
ChangeDetectionStrategy,
66
Component,
77
HostListener,
88
Inject,
9+
NgZone,
10+
OnDestroy,
911
OnInit,
1012
PLATFORM_ID,
1113
} from '@angular/core';
@@ -16,7 +18,7 @@ import {
1618
Router,
1719
} from '@angular/router';
1820

19-
import { BehaviorSubject, Observable } from 'rxjs';
21+
import { BehaviorSubject, combineLatest, Observable, race, Subject, timer } from 'rxjs';
2022
import { select, Store } from '@ngrx/store';
2123
import { NgbModal, NgbModalConfig } from '@ng-bootstrap/ng-bootstrap';
2224
import { TranslateService } from '@ngx-translate/core';
@@ -38,10 +40,22 @@ import { distinctNext } from './core/shared/distinct-next';
3840
styleUrls: ['./app.component.scss'],
3941
changeDetection: ChangeDetectionStrategy.OnPush,
4042
})
41-
export class AppComponent implements OnInit, AfterViewInit {
43+
export class AppComponent implements OnInit, AfterViewInit, OnDestroy {
4244
notificationOptions;
4345
models;
4446

47+
/**
48+
* Emits on destroy to tear down the SSR-overlay-removal pipeline (gate subscription, the
49+
* MutationObserver and its debounce/cap timers all unsubscribe via `takeUntil(destroyed$)`).
50+
* AppComponent is the root component so this is mostly defensive + test hygiene.
51+
*/
52+
private destroyed$ = new Subject<void>();
53+
54+
/** SSR anti-flicker overlay (see src/index.html) removal tuning. */
55+
private readonly ssrOverlaySettleQuietMs = 600; // routed page is "done" after this long with no DOM change
56+
private readonly ssrOverlaySettleMaxMs = 10000; // backstop reveal (below index.html's 15s catastrophic net)
57+
private readonly ssrOverlayMinContentHeightPx = 200; // proves <ds-app> is no longer the empty shell
58+
4559
/**
4660
* Whether or not the authentication is currently blocking the UI
4761
*/
@@ -74,6 +88,7 @@ export class AppComponent implements OnInit, AfterViewInit {
7488
private cssService: CSSVariableService,
7589
private modalService: NgbModal,
7690
private modalConfig: NgbModalConfig,
91+
private ngZone: NgZone,
7792
) {
7893
this.notificationOptions = environment.notifications;
7994

@@ -82,13 +97,129 @@ export class AppComponent implements OnInit, AfterViewInit {
8297

8398
if (isPlatformBrowser(this.platformId)) {
8499
this.trackIdleModal();
100+
this.removeSsrOverlayWhenContentVisible();
85101
}
86102

87103
this.isThemeLoading$ = this.themeService.isThemeLoading$;
88104

89105
this.storeCSSVariables();
90106
}
91107

108+
/**
109+
* Drops the SSR mask overlay installed by the inline bootstrap script in src/index.html once the
110+
* routed CSR page has actually finished rendering. Picking the right moment is the whole problem,
111+
* and the two earlier attempts each fixed one symptom and reintroduced the other:
112+
*
113+
* - PR #1288 waited for `ApplicationRef.isStable`. No flicker, but isStable is held hostage by ANY
114+
* ongoing zone async — after an admin login the app keeps the zone busy (authz/widgets, periodic
115+
* polling, AAI/discojuice scripts) so isStable fires many seconds late (or hits the 15s
116+
* fallback). The inert snapshot then masks the live page -> "looks rendered but not interactive"
117+
* (issue #725).
118+
* - PR #1317 switched to the loader-swap gate `!isAuthenticationBlocking && !isThemeLoading` plus a
119+
* single requestAnimationFrame. Prompt, but that gate only un-hides `<router-outlet>`; the home
120+
* page then renders piecewise (navbar, search box, community list, recent submissions) as each
121+
* section's data arrives. Dropping the snapshot at the gate — or, as an earlier revision of this
122+
* method did, as soon as *some* content exists — exposes a half-built page that visibly pops
123+
* into place on a hard reload -> the flicker.
124+
*
125+
* The signal we actually need is "the routed page has stopped changing". So, after the gate opens,
126+
* we keep the snapshot until the live <ds-app> DOM has SETTLED (no element added/removed for a
127+
* short quiet window, with real content present). This stays decoupled from isStable (DOM-settle
128+
* ignores non-rendering background async, so admin reveals in a few seconds rather than ~15s).
129+
* See {@link routedPageReadyToReveal$}.
130+
*/
131+
private removeSsrOverlayWhenContentVisible(): void {
132+
const win: Window | undefined = this._window?.nativeWindow;
133+
if (!win || typeof win.__dspaceRemoveSsrOverlay !== 'function') {
134+
return; // SSR was skipped for this route, so no overlay was installed — nothing to remove
135+
}
136+
// Run outside Angular: a MutationObserver watching the whole app must not trigger change
137+
// detection (it would also keep ApplicationRef.isStable permanently false).
138+
this.ngZone.runOutsideAngular(() => {
139+
this.routedPageReadyToReveal$().pipe(
140+
takeUntil(this.destroyed$),
141+
).subscribe(() => {
142+
// one frame so the freshly rendered content is painted before the snapshot fades out
143+
this.runAfterNextFrame(win, () => win.__dspaceRemoveSsrOverlay?.());
144+
});
145+
});
146+
}
147+
148+
/**
149+
* Emits once when it is safe to drop the SSR snapshot: the auth/theme loader gate has opened
150+
* (same condition root.component.html uses to swap its fullscreen loader for the routed content)
151+
* AND the routed page's DOM has settled. See {@link dsAppDomSettled$}.
152+
*/
153+
private routedPageReadyToReveal$(): Observable<unknown> {
154+
const loaderGateOpen$ = combineLatest([
155+
this.store.pipe(select(isAuthenticationBlocking), distinctUntilChanged()),
156+
this.themeService.isThemeLoading$,
157+
]).pipe(
158+
filter(([authBlocking, themeLoading]: [boolean, boolean]) => !authBlocking && !themeLoading),
159+
take(1),
160+
);
161+
return loaderGateOpen$.pipe(
162+
switchMap(() => this.dsAppDomSettled$()),
163+
);
164+
}
165+
166+
/**
167+
* Emits once when the live <ds-app> subtree stops being mutated (elements added/removed) for
168+
* `ssrOverlaySettleQuietMs` AND it holds real content — or after `ssrOverlaySettleMaxMs`, whichever
169+
* comes first. The cap guarantees a page that never goes quiet (constant background DOM updates)
170+
* still reveals; the 15s fallback in index.html remains the ultimate net.
171+
*/
172+
private dsAppDomSettled$(): Observable<unknown> {
173+
const dsApp: Element | null = this.document.querySelector('ds-app');
174+
if (!dsApp) {
175+
return timer(this.ssrOverlaySettleMaxMs);
176+
}
177+
const elementMutations$ = new Observable<void>((subscriber) => {
178+
const observer = new MutationObserver((records) => {
179+
if (records.some((record) => this.isElementChildListChange(record))) {
180+
subscriber.next();
181+
}
182+
});
183+
observer.observe(dsApp, { childList: true, subtree: true });
184+
return () => observer.disconnect();
185+
});
186+
const settled$ = elementMutations$.pipe(
187+
startWith(undefined), // start the quiet window immediately
188+
debounceTime(this.ssrOverlaySettleQuietMs), // ... reset by each render, fires once quiet
189+
filter(() => this.dsAppHasRenderedContent(dsApp)), // ... but never on the empty shell
190+
);
191+
return race(settled$, timer(this.ssrOverlaySettleMaxMs)).pipe(take(1));
192+
}
193+
194+
/** True once the live <ds-app> is no longer the empty shell the overlay script left behind. */
195+
private dsAppHasRenderedContent(dsApp: Element): boolean {
196+
const height = dsApp.getBoundingClientRect?.().height ?? 0;
197+
return height >= this.ssrOverlayMinContentHeightPx && dsApp.querySelector('#main-content') !== null;
198+
}
199+
200+
/** A childList mutation that adds or removes at least one element node (ignores text/attr noise). */
201+
private isElementChildListChange(record: MutationRecord): boolean {
202+
if (record.type !== 'childList') {
203+
return false;
204+
}
205+
const changedNodes = [...Array.from(record.addedNodes), ...Array.from(record.removedNodes)];
206+
return changedNodes.some((node) => node.nodeType === Node.ELEMENT_NODE);
207+
}
208+
209+
/** Runs `callback` after the next paint (or synchronously if requestAnimationFrame is unavailable). */
210+
private runAfterNextFrame(win: Window, callback: () => void): void {
211+
if (typeof win.requestAnimationFrame === 'function') {
212+
win.requestAnimationFrame(() => callback());
213+
} else {
214+
callback();
215+
}
216+
}
217+
218+
ngOnDestroy(): void {
219+
this.destroyed$.next();
220+
this.destroyed$.complete();
221+
}
222+
92223
ngOnInit() {
93224
/** Implement behavior for interface {@link ModalBeforeDismiss} */
94225
this.modalConfig.beforeDismiss = async function () {

0 commit comments

Comments
 (0)