From 2acb90a6c2fd18e600a16ab44c6666bb8ec6e7f7 Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 19 May 2026 02:24:24 +0200 Subject: [PATCH 1/5] 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 + diff --git a/src/themes/eager-themes.module.ts b/src/themes/eager-themes.module.ts index bbd77d2a40f..93518ee01a8 100644 --- a/src/themes/eager-themes.module.ts +++ b/src/themes/eager-themes.module.ts @@ -1,7 +1,7 @@ import { NgModule } from '@angular/core'; import { EagerThemeModule as DSpaceEagerThemeModule } from './dspace/eager-theme.module'; import { EagerThemeModule } from './dspace/eager-theme.module'; -// import { EagerThemeModule as CustomEagerThemeModule } from './custom/eager-theme.module'; +import { EagerThemeModule as CustomEagerThemeModule } from './custom/eager-theme.module'; /** * This module bundles the eager theme modules for all available themes. @@ -9,6 +9,11 @@ import { EagerThemeModule } from './dspace/eager-theme.module'; * and entry components (to ensure their decorators get picked up). * * Themes that aren't in use should not be imported here so they don't take up unnecessary space in the main bundle. + * + * NOTE: CustomEagerThemeModule is included to prevent the home-page flicker that occurs when + * the active theme is `custom`. Without it, every themed wrapper (footer, header, root, ...) is + * lazy-loaded via webpack code-splitting on the browser, leaving visible gaps after the SSR DOM + * is torn down and before the CSR DOM is materialised. */ @NgModule({ imports: [ @@ -17,6 +22,7 @@ import { EagerThemeModule } from './dspace/eager-theme.module'; // Issue: https://github.com/DSpace/dspace-angular/issues/1897 // Useful info in PR: https://github.com/DSpace/dspace-angular/pull/2262#issuecomment-1557146081 EagerThemeModule, + CustomEagerThemeModule, ], }) export class EagerThemesModule { From 15d05f732205a4e03aa07c44e36dd6a1532f6b4b Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 19 May 2026 02:31:16 +0200 Subject: [PATCH 2/5] Lint: curly braces around early-return if --- src/app/app.component.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 2318c0374ed..68a7e82a261 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -103,7 +103,9 @@ export class AppComponent implements OnInit, AfterViewInit { */ private removeSsrOverlayWhenStable(): void { const w = this._window?.nativeWindow as any; - if (!w || typeof w.__dspaceRemoveSsrOverlay !== 'function') return; + if (!w || typeof w.__dspaceRemoveSsrOverlay !== 'function') { + return; + } // run outside Angular so we don't keep changeDetection ticking on the overlay timer this.ngZone.runOutsideAngular(() => { this.appRef.isStable.pipe( From 79c1d5340cc23a6eeea2969cb0574dc9ca3c3d54 Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 19 May 2026 02:41:49 +0200 Subject: [PATCH 3/5] 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) --- .claude/skills/dspace-deploy/SKILL.md | 75 ---------- angular.json | 4 +- scripts/dspace-deploy.bat | 191 -------------------------- src/app/app.component.spec.ts | 63 ++++++++- src/app/app.component.ts | 2 +- src/index.html | 29 +++- src/typings.d.ts | 9 ++ 7 files changed, 95 insertions(+), 278 deletions(-) delete mode 100644 .claude/skills/dspace-deploy/SKILL.md delete mode 100644 scripts/dspace-deploy.bat diff --git a/.claude/skills/dspace-deploy/SKILL.md b/.claude/skills/dspace-deploy/SKILL.md deleted file mode 100644 index 2f3a33412c1..00000000000 --- a/.claude/skills/dspace-deploy/SKILL.md +++ /dev/null @@ -1,75 +0,0 @@ ---- -name: dspace-deploy -description: Deploy the current dspace-angular sources (FE) together with the BE (dspace, dspacedb, dspacesolr) locally via Docker compose. Multi-instance safe — every instance uses a distinct INSTANCE digit (1-9) so ports / container names / subnets do not clash. Use this when the user asks to "start the stack", "fire up dspace locally", "spin up FE+BE", "deploy current sources locally", or "run multiple instances side by side". ---- - -# /dspace-deploy — Spin up DSpace FE + BE locally - -## What this skill does - -Brings up a local DSpace 7 stack (Angular FE + REST BE + Postgres + Solr) for the **current source branch** using Docker compose, in a way that is **multi-instance safe** — multiple instances can run on the same host because every port, container name, network subnet and volume namespace is parameterised by a single digit `INSTANCE` (1-9). - -Heavy lifting is delegated to the existing `docker/docker-compose.yml` + `docker/docker-compose-rest.yml`, which already use `${INSTANCE}` everywhere. A generated compose override file: -- relocates the network subnet to `10.10${INSTANCE}.0.0/16` (the default `172.2${INSTANCE}.0.0/16` clashes with most users' other compose projects); -- adds `host.docker.internal:host-gateway` to the FE container so SSR can reach the BE via the host gateway — and Docker Desktop already wires `host.docker.internal` into the Windows hosts file, so the browser on the host resolves the same hostname. - -## Resulting URLs - -For a given `INSTANCE=N`: - -| Service | Host URL | Container | -|-------------|-----------------------------------------|-------------------| -| Angular UI | `http://localhost:400N/` | `dspace-angularN` | -| REST API | `http://localhost:808N/server/api` | `dspaceN` | -| Postgres | `localhost:543N` | `dspacedbN` | -| Solr admin | `http://localhost:898N` | `dspacesolrN` | - -## Helper script - -`scripts\dspace-deploy.bat` — a Windows cmd batch script (NO PowerShell — the user has explicitly forbidden it). - -Invocation: - -``` -scripts\dspace-deploy.bat [INSTANCE] [ACTION] [TAG] -``` - -- `INSTANCE` — single digit 1-9, default `7` (5 and 8 are reserved for the prod deploy workflow). -- `ACTION` — `up` (default), `rebuild` (force-recreate with `--build`), `down`, `status`, `logs`. -- `TAG` — image tag for all four images (default `dspace-7_x`). - -Examples: - -``` -scripts\dspace-deploy.bat :: up instance 7 using published images -scripts\dspace-deploy.bat 6 :: up instance 6 -scripts\dspace-deploy.bat 7 rebuild :: rebuild FE from current sources + force-recreate -scripts\dspace-deploy.bat 7 down :: tear down instance 7 (with -v) -scripts\dspace-deploy.bat 7 status :: show compose ps -scripts\dspace-deploy.bat 7 logs :: follow logs -``` - -## How to invoke from the agent - -1. Confirm Docker is reachable (`docker info`). -2. Pick `INSTANCE` (arg or `7`). -3. Refuse to overwrite a running stack with the same `INSTANCE` unless `rebuild` is explicitly requested. -4. Run the batch script via `Bash` (it works under MINGW/Git Bash too — paths are absolute via `%~dp0`). -5. After `up`, the script itself polls FE + BE up to ~7.5 min and prints the URL table. - -## Multi-instance contract - -`docker/docker-compose.yml` and `docker/docker-compose-rest.yml` already use `${INSTANCE}` in: - -- Ports: `400${INSTANCE}`, `808${INSTANCE}`, `543${INSTANCE}`, `898${INSTANCE}`, `264${INSTANCE}`, `801${INSTANCE}`, `987${INSTANCE}`. -- Container names: `dspace${INSTANCE}`, `dspacedb${INSTANCE}`, `dspacesolr${INSTANCE}`, `dspace-angular${INSTANCE}`. -- Subnet (overridden by this skill to `10.10${INSTANCE}.0.0/16`). - -So a 2nd instance just needs `scripts\dspace-deploy.bat 6` while `7` is up. - -## Notes - -- **Local dev / verification only**. The production-style deploy on `dev-5` uses `.github/workflows/deploy.yml`; do not reuse this skill there. -- First start downloads ~4 GB of images and the BE may take 2-5 minutes to be ready (Solr cores + DB migration). -- Admin user / data ingestion is **not** performed automatically. If needed, run `docker compose -p dspace-N -f docker/docker-compose.yml -f docker/cli.yml -f docker/docker-compose-rest.yml run --rm dspace-cli create-administrator ...`. -- **NEVER use PowerShell here.** Per user instruction, only cmd / bash. The helper script is a `.bat` deliberately. diff --git a/angular.json b/angular.json index ae0d57b9d4b..cf0b2f24038 100644 --- a/angular.json +++ b/angular.json @@ -99,8 +99,8 @@ "budgets": [ { "type": "initial", - "maximumWarning": "6mb", - "maximumError": "8mb" + "maximumWarning": "5.5mb", + "maximumError": "6mb" }, { "type": "anyComponentStyle", diff --git a/scripts/dspace-deploy.bat b/scripts/dspace-deploy.bat deleted file mode 100644 index c41ad0678ba..00000000000 --- a/scripts/dspace-deploy.bat +++ /dev/null @@ -1,191 +0,0 @@ -@echo off -setlocal EnableDelayedExpansion -REM scripts/dspace-deploy.bat -REM Helper for the dspace-deploy skill. See .claude/skills/dspace-deploy/SKILL.md. -REM -REM Brings up / down a multi-instance-safe DSpace FE+BE stack via docker compose. -REM Every instance uses a single digit (1-9) to namespace ports, container names, subnets. -REM -REM Usage: -REM scripts\dspace-deploy.bat [instance=7] [up|down|status|logs|rebuild] [tag=dspace-7_x] -REM -REM Examples: -REM scripts\dspace-deploy.bat - up instance 7 -REM scripts\dspace-deploy.bat 6 - up instance 6 -REM scripts\dspace-deploy.bat 7 rebuild - force-recreate instance 7 with --build -REM scripts\dspace-deploy.bat 7 down - tear down instance 7 (volumes too) -REM scripts\dspace-deploy.bat 7 status - show container status -REM scripts\dspace-deploy.bat 7 logs - follow logs - -set "INSTANCE=%~1" -set "ACTION=%~2" -set "TAG=%~3" -set "REMOTE_BE=%~4" - -if "%INSTANCE%"=="" set "INSTANCE=7" -if "%ACTION%"=="" set "ACTION=up" -if "%TAG%"=="" set "TAG=dspace-7_x" -REM REMOTE_BE: if set (e.g. "dev-5.pc:84"), the FE bypasses the local BE and proxies to that host. -REM Useful for testing UI changes without seeding the local DB. - -REM Validate instance is a single digit 1-9 -echo %INSTANCE%| findstr /r "^[1-9]$" >nul -if errorlevel 1 ( - echo ERROR: INSTANCE must be a single digit 1-9, got: %INSTANCE% - exit /b 1 -) - -REM Bind to 0.0.0.0 so both `localhost` (browser) and `host.docker.internal` (which on -REM Docker Desktop resolves to the host's LAN IP via the Windows hosts file) reach the same port. -REM If we bound to 127.0.0.1, the FE-container SSR call would work via the docker-gateway resolution, -REM but the browser's call would hit 192.168.x.x:port -> connection refused. -set "HOST_IP=0.0.0.0" -REM Subnet prefix: 10.10X.0.0/16 (X = INSTANCE digit). Stays out of the 172.2X.0.0/16 -REM range used by other DSpace projects on the same host. -set "SUBNET_PREFIX=10.10%INSTANCE%" -set "PROJECT=dspace-%INSTANCE%" - -REM Resolve repo root (scripts is one level below repo root) -pushd "%~dp0\.." -set "REPO_ROOT=%CD%" -popd - -set "ENVFILE=%REPO_ROOT%\docker\.env.dspace-%INSTANCE%" -set "OVERRIDE=%REPO_ROOT%\docker\.override.dspace-%INSTANCE%.yml" - -echo === dspace-deploy: instance=%INSTANCE% action=%ACTION% tag=%TAG% === - -REM Write env file (cmd writes without BOM) -( - echo # Auto-generated by scripts/dspace-deploy.bat - echo INSTANCE=%INSTANCE% - echo HOST_IP=%HOST_IP% - echo TIMEZONE=Europe/Bratislava - echo. - echo DSPACE_UI_IMAGE=dataquest/dspace-angular:%TAG% - echo DSPACE_REST_IMAGE=dataquest/dspace:%TAG% - echo DSPACE_DB_IMAGE=dataquest/dspace-postgres-pgcrypto:%TAG% - echo DSPACE_SOLR_IMAGE=dataquest/dspace-solr:%TAG% - echo. - echo DSPACE_UI_NAMESPACE=/ - echo DSPACE_REST_NAMESPACE=/server - echo. - if defined REMOTE_BE ( - for /f "tokens=1,2 delims=:" %%a in ("%REMOTE_BE%") do ( - echo # Pointing FE at remote BE %REMOTE_BE% ^(useful for testing UI without seeding the local DB^) - echo DSPACE_HOST=%%a - echo DSPACE_REST_PORT=%%b - echo REST_URL=http://%REMOTE_BE%/server - echo UI_URL=http://host.docker.internal:400%INSTANCE% - ) - ) else ( - echo # host.docker.internal so SSR ^(inside container^) and browser ^(on host^) both resolve - echo DSPACE_HOST=host.docker.internal - echo DSPACE_REST_PORT=808%INSTANCE% - echo REST_URL=http://host.docker.internal:808%INSTANCE%/server - echo UI_URL=http://host.docker.internal:400%INSTANCE% - ) -) > "%ENVFILE%" -echo Wrote %ENVFILE% - -REM Write compose override (subnet + extra_hosts + correct pm2 config path) -REM -REM The base docker-compose.yml hardcodes the FE entrypoint to: -REM /bin/sh -c "pm2-runtime start dspace-ui.json ..." -REM But locally-built images put that file at /app/docker/dspace-ui.json (see Dockerfile), -REM so pm2 exits ENOENT on first start and the container loops. Override the entrypoint -REM to point at the right path AND surface its output so failures are diagnosable. -( - echo # Auto-generated by scripts/dspace-deploy.bat - echo networks: - echo dspacenet: - echo ipam: - echo config: - echo - subnet: %SUBNET_PREFIX%.0.0/16 - echo services: - echo dspace: - echo environment: - echo proxies__P__trusted__P__ipranges: '%SUBNET_PREFIX%.0' - echo REST_URL: http://host.docker.internal:808%INSTANCE%/server - echo UI_URL: http://host.docker.internal:400%INSTANCE% - echo rest__P__cors__P__allowed__D__origins: 'http://host.docker.internal:400%INSTANCE%, http://localhost:400%INSTANCE%' - echo dspace-angular: - echo extra_hosts: - echo - host.docker.internal:host-gateway - echo entrypoint: - echo - /bin/sh - echo - -c - echo - if [ -f /app/dspace-ui.json ]; then pm2-runtime start dspace-ui.json; else pm2-runtime start docker/dspace-ui.json; fi -) > "%OVERRIDE%" -echo Wrote %OVERRIDE% (subnet %SUBNET_PREFIX%.0.0/16) - -set COMPOSE_BASE=docker compose --env-file "%ENVFILE%" -p %PROJECT% -f "%REPO_ROOT%\docker\docker-compose.yml" -f "%REPO_ROOT%\docker\docker-compose-rest.yml" -f "%OVERRIDE%" - -if /i "%ACTION%"=="status" ( - %COMPOSE_BASE% ps - goto :eof -) -if /i "%ACTION%"=="logs" ( - %COMPOSE_BASE% logs --tail=200 -f - goto :eof -) -if /i "%ACTION%"=="down" ( - echo === Tearing down [%PROJECT%] === - %COMPOSE_BASE% down -v --remove-orphans - goto :eof -) - -REM Default: up (or rebuild) -docker info --format "{{.ServerVersion}}" >nul 2>&1 -if errorlevel 1 ( - echo ERROR: docker info failed - is Docker Desktop running? - exit /b 1 -) - -if /i "%ACTION%"=="rebuild" ( - echo === Pulling ^& building [%PROJECT%] === - %COMPOSE_BASE% pull - %COMPOSE_BASE% up -d --build --force-recreate --remove-orphans -) else ( - echo === Pulling images === - %COMPOSE_BASE% pull - echo === Up [%PROJECT%] === - %COMPOSE_BASE% up -d --no-build --remove-orphans -) -if errorlevel 1 ( - echo ERROR: docker compose up failed - exit /b 1 -) - -echo === Waiting for services to become ready === -set "BE_URL=http://%HOST_IP%:808%INSTANCE%/server/api" -set "FE_URL=http://%HOST_IP%:400%INSTANCE%/" -set /a TRIES=0 -:wait_loop -set /a TRIES+=1 -if %TRIES% GTR 90 goto :ready_timeout - -REM Use curl which is bundled with Windows 10+ -for /f "tokens=*" %%i in ('curl -s -o nul -w "%%{http_code}" --max-time 4 "%BE_URL%" 2^>nul') do set "BE_CODE=%%i" -for /f "tokens=*" %%i in ('curl -s -o nul -w "%%{http_code}" --max-time 4 "%FE_URL%" 2^>nul') do set "FE_CODE=%%i" -echo try %TRIES%: BE=%BE_CODE% FE=%FE_CODE% -if "%BE_CODE%"=="200" if "%FE_CODE%"=="200" goto :ready_ok - -timeout /t 5 /nobreak >nul -goto :wait_loop - -:ready_timeout -echo WARNING: services not fully ready (BE=%BE_CODE% FE=%FE_CODE%). Inspect: scripts\dspace-deploy.bat %INSTANCE% logs -goto :print_urls - -:ready_ok -echo === Ready === - -:print_urls -echo. -echo === URLs === -echo UI: %FE_URL% -echo REST: %BE_URL% -echo Solr: http://%HOST_IP%:898%INSTANCE% -echo DB: postgres://dspace:dspace@%HOST_IP%:543%INSTANCE%/dspace -endlocal diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index e921c67acea..9294f1ff1dd 100644 --- a/src/app/app.component.spec.ts +++ b/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 { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; +import { ComponentFixture, fakeAsync, flush, inject, TestBed, tick, waitForAsync } from '@angular/core/testing'; +import { ApplicationRef, 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'; @@ -127,4 +128,62 @@ describe('App component', () => { }); }); + + describe('removeSsrOverlayWhenStable', () => { + // The inline bootstrap script in src/index.html injects window.__dspaceRemoveSsrOverlay + // and AppComponent must call it exactly once when ApplicationRef.isStable first emits true. + let appRef: ApplicationRef; + let isStable$: BehaviorSubject; + let originalRaF: typeof window.requestAnimationFrame; + + beforeEach(() => { + appRef = TestBed.inject(ApplicationRef); + isStable$ = new BehaviorSubject(false); + // Patch isStable to our controllable subject for this test only + Object.defineProperty(appRef, 'isStable', { value: isStable$.asObservable() }); + + // Force rAF to a synchronous shim so we can flush() through the chain deterministically. + 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; + }); + + it('removes the overlay once isStable emits true', fakeAsync(() => { + const spy = jasmine.createSpy('__dspaceRemoveSsrOverlay'); + window.__dspaceRemoveSsrOverlay = spy; + + // Re-construct so the constructor-time subscription picks up our patched isStable + global. + const f = TestBed.createComponent(AppComponent); + f.detectChanges(); + + expect(spy).not.toHaveBeenCalled(); + + isStable$.next(true); + tick(50); // matches the 50ms pad after rAF in removeSsrOverlayWhenStable + flush(); + + expect(spy).toHaveBeenCalledTimes(1); + })); + + 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(); + + isStable$.next(true); + tick(50); + flush(); + + expect(window.__dspaceRemoveSsrOverlay).toBeUndefined(); + })); + }); }); diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 68a7e82a261..8200033fc20 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -102,7 +102,7 @@ export class AppComponent implements OnInit, AfterViewInit { * is also a 15s hard fallback inside the script itself in case isStable never fires. */ private removeSsrOverlayWhenStable(): void { - const w = this._window?.nativeWindow as any; + const w: Window | undefined = this._window?.nativeWindow; if (!w || typeof w.__dspaceRemoveSsrOverlay !== 'function') { return; } diff --git a/src/index.html b/src/index.html index df101361e8b..cf17c9b44ad 100644 --- a/src/index.html +++ b/src/index.html @@ -53,10 +53,6 @@ if (!app || !app.firstElementChild) return; if (document.getElementById('__dspace_ssr_overlay')) return; - // Build overlay and MOVE (not clone) the SSR children into it. Moving keeps every live DOM - // detail (Angular's view-encapsulation attributes, computed inline styles, image-load state, - // etc.) so the overlay is pixel-identical to what the user already saw before Angular booted. - // Cloning via innerHTML loses parent-context-dependent rendering. // Critical: Angular's BrowserModule.withServerTransition removes ALL