From 9cb0818594604eca8121c708e86b2dafc8f6ac73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kan=C3=A1sz-Nagy=20Zolt=C3=A1n?= Date: Wed, 13 May 2026 09:03:05 +0200 Subject: [PATCH 1/5] QREPO-405 extending translate loader to use theme inheritence --- .../translate-browser.loader.ts | 95 ++++++++++++-- .../translate-server.loader.ts | 116 ++++++++++++++++-- 2 files changed, 192 insertions(+), 19 deletions(-) diff --git a/src/ngx-translate-loaders/translate-browser.loader.ts b/src/ngx-translate-loaders/translate-browser.loader.ts index 44e1710b148..caf8f255a69 100644 --- a/src/ngx-translate-loaders/translate-browser.loader.ts +++ b/src/ngx-translate-loaders/translate-browser.loader.ts @@ -2,11 +2,16 @@ import { HttpClient } from '@angular/common/http'; import { TransferState } from '@angular/core'; import { hasValue } from '@dspace/shared/utils/empty.util'; import { TranslateLoader } from '@ngx-translate/core'; +import * as JSON5 from 'json5'; import { + forkJoin, Observable, of, } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { + catchError, + map, +} from 'rxjs/operators'; import { environment } from '../environments/environment'; import { @@ -16,7 +21,12 @@ import { /** * A TranslateLoader for ngx-translate to retrieve i18n messages from the TransferState, or download - * them if they're not available there + * them if they're not available there. + * + * Also fetches per-theme translation overrides from `assets//i18n/.json5` for every + * configured theme and deep-merges them on top of the base translations (theme keys win). + * This removes the need for the build-time `merge-i18n` script, allowing a single Docker image + * to serve multiple customers with their own theme-specific translations. */ export class TranslateBrowserLoader implements TranslateLoader { constructor( @@ -35,18 +45,83 @@ export class TranslateBrowserLoader implements TranslateLoader { */ getTranslation(lang: string): Observable { // Get the ngx-translate messages from the transfer state, to speed up the initial page load - // client side + // client side. The server has already merged theme overrides into this state. const state = this.transferState.get(NGX_TRANSLATE_STATE, {}); const messages = state[lang]; if (hasValue(messages)) { return of(messages); - } else { - const translationHash: string = environment.production ? `.${(process.env.languageHashes as any)[lang + '.json5']}` : ''; - // If they're not available on the transfer state (e.g. when running in dev mode), retrieve - // them using HttpClient - return this.http.get(`${this.prefix}${lang}${translationHash}${this.suffix}`, { responseType: 'text' }).pipe( - map((json: any) => JSON.parse(json)), - ); } + + // Fetch base translations + every configured theme's override file (in inheritance order) + // and merge them. Theme keys override base keys; child theme keys override parent theme keys. + const base$ = this.fetchBase(lang); + const orderedThemes = this.resolveThemeLoadOrder(environment.themes as any || []); + const themeStreams = orderedThemes.map((name: string) => this.fetchThemeOverride(name, lang)); + + return forkJoin([base$, ...themeStreams]).pipe( + map((parts) => parts.reduce((acc, part) => Object.assign(acc, part), {})), + ); + } + + /** + * Resolve the load order for the configured themes, expanding `extends` chains so that + * ancestor themes are loaded before their descendants (descendant wins on conflicts). + * See {@link TranslateServerLoader.resolveThemeLoadOrder} for details. + */ + protected resolveThemeLoadOrder(configured: Array<{ name?: string; extends?: string }>): string[] { + const byName = new Map(); + configured.forEach((t) => { + if (t?.name) { + byName.set(t.name, t); + } + }); + + const result: string[] = []; + const seen = new Set(); + + for (const theme of configured) { + if (!theme?.name) { + continue; + } + const chain: string[] = []; + const visited = new Set(); + let cursor: string | undefined = theme.name; + while (cursor && !visited.has(cursor)) { + visited.add(cursor); + chain.push(cursor); + cursor = byName.get(cursor)?.extends; + } + for (const name of chain.reverse()) { + if (!seen.has(name)) { + seen.add(name); + result.push(name); + } + } + } + return result; + } + + /** + * Fetch the base i18n file (the hashed JSON produced by the build pipeline). + */ + protected fetchBase(lang: string): Observable> { + const translationHash: string = environment.production + ? `.${(process.env.languageHashes as any)[lang + '.json5']}` + : ''; + return this.http.get(`${this.prefix}${lang}${translationHash}${this.suffix}`, { responseType: 'text' }).pipe( + map((json: any) => JSON.parse(json)), + catchError(() => of({})), + ); + } + + /** + * Fetch the override JSON5 file for the given theme/language. + * Returns an empty object if the file is missing (most themes won't override every language). + */ + protected fetchThemeOverride(themeName: string, lang: string): Observable> { + return this.http.get(`assets/${themeName}/i18n/${lang}.json5`, { responseType: 'text' }).pipe( + map((text: string) => JSON5.parse(text) as Record), + catchError(() => of({})), + ); } } diff --git a/src/ngx-translate-loaders/translate-server.loader.ts b/src/ngx-translate-loaders/translate-server.loader.ts index 06312b3a2b0..6e64b618e26 100644 --- a/src/ngx-translate-loaders/translate-server.loader.ts +++ b/src/ngx-translate-loaders/translate-server.loader.ts @@ -1,12 +1,18 @@ -import { readFileSync } from 'node:fs'; +import { + existsSync, + readFileSync, +} from 'node:fs'; +import { dirname, resolve } from 'node:path'; import { TransferState } from '@angular/core'; import { TranslateLoader } from '@ngx-translate/core'; +import * as JSON5 from 'json5'; import { Observable, of, } from 'rxjs'; +import { environment } from '../environments/environment'; import { NGX_TRANSLATE_STATE, NgxTranslateState, @@ -14,7 +20,12 @@ import { /** * A TranslateLoader for ngx-translate to parse json5 files server-side, and store them in the - * TransferState + * TransferState. + * + * Also overlays per-theme translation overrides from `//i18n/.json5` + * for every configured theme, merging them on top of the base translations (theme keys win). + * The resulting merged map is stored in the TransferState so the browser does not need to + * re-fetch the theme overrides after the initial SSR response. */ export class TranslateServerLoader implements TranslateLoader { @@ -32,13 +43,100 @@ export class TranslateServerLoader implements TranslateLoader { */ public getTranslation(lang: string): Observable { const translationHash: string = (process.env.languageHashes as any)[lang + '.json5']; - // Retrieve the file for the given language, and parse it - const messages = JSON.parse(readFileSync(`${this.prefix}${lang}.${translationHash}${this.suffix}`, 'utf8')); - // Store the parsed messages in the transfer state so they'll be available immediately when the - // app loads on the client - this.storeInTransferState(lang, messages); - // Return the parsed messages to translate things server side - return of(messages); + // Retrieve the base file for the given language and parse it + const baseMessages = JSON.parse(readFileSync(`${this.prefix}${lang}.${translationHash}${this.suffix}`, 'utf8')); + + // Overlay theme override files (if any). Theme keys override base keys. + const merged = Object.assign({}, baseMessages, this.readThemeOverrides(lang)); + + // Store the parsed messages in the transfer state so they'll be available immediately when + // the app loads on the client + this.storeInTransferState(lang, merged); + // Return the merged messages to translate things server side + return of(merged); + } + + /** + * Read and merge i18n override files from every configured theme's assets folder. + * Returns an empty object if no overrides are present. + * + * @param lang the language code + * @protected + */ + protected readThemeOverrides(lang: string): Record { + const configured = (environment.themes || []) as Array<{ name?: string; extends?: string }>; + if (configured.length === 0) { + return {}; + } + + // Build the ordered list of themes to load, expanding each configured theme's inheritance + // chain (ancestor first, descendant last) so that child theme overrides win over parents. + const orderedThemes = this.resolveThemeLoadOrder(configured); + + // `this.prefix` looks like `dist/server/assets/i18n/`. Theme assets live next to `i18n/` + // at `dist/server/assets//i18n/.json5`. + const assetsRoot = dirname(this.prefix.replace(/\/$/, '')); + + const overrides: Record = {}; + for (const theme of orderedThemes) { + const filePath = resolve(`${assetsRoot}/${theme}/i18n/${lang}.json5`); + if (existsSync(filePath)) { + try { + const themeMessages = JSON5.parse(readFileSync(filePath, 'utf8')) as Record; + Object.assign(overrides, themeMessages); + } catch (e) { + // Skip malformed theme i18n files so they don't break SSR + // eslint-disable-next-line no-console + console.warn(`[TranslateServerLoader] Failed to parse theme i18n file ${filePath}:`, e); + } + } + } + return overrides; + } + + /** + * Resolve the load order for the given configured themes, expanding `extends` chains. + * + * For each configured theme we walk up its `extends` chain, then emit themes from the root + * ancestor down to the descendant. Duplicates are removed (a theme already loaded as an + * ancestor of an earlier configured theme is not loaded again). + * + * Example given `themes: [kjk (extends qulto)]`: + * load order = [qulto, kjk] → qulto keys, then kjk keys override + */ + protected resolveThemeLoadOrder(configured: Array<{ name?: string; extends?: string }>): string[] { + const byName = new Map(); + configured.forEach((t) => { + if (t?.name) { + byName.set(t.name, t); + } + }); + + const result: string[] = []; + const seen = new Set(); + + for (const theme of configured) { + if (!theme?.name) { + continue; + } + // Build chain from theme up to root ancestor (handle cycles). + const chain: string[] = []; + const visited = new Set(); + let cursor: string | undefined = theme.name; + while (cursor && !visited.has(cursor)) { + visited.add(cursor); + chain.push(cursor); + cursor = byName.get(cursor)?.extends; + } + // Reverse so we apply ancestor → descendant order. + for (const name of chain.reverse()) { + if (!seen.has(name)) { + seen.add(name); + result.push(name); + } + } + } + return result; } /** From 6add72ca3adf7f3cfd3d095aa9915c9a51324c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kan=C3=A1sz-Nagy=20Zolt=C3=A1n?= Date: Wed, 13 May 2026 09:36:57 +0200 Subject: [PATCH 2/5] QREPO-405 fixing problems reported by lint --- src/ngx-translate-loaders/translate-browser.loader.ts | 2 +- src/ngx-translate-loaders/translate-server.loader.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ngx-translate-loaders/translate-browser.loader.ts b/src/ngx-translate-loaders/translate-browser.loader.ts index caf8f255a69..bb973576441 100644 --- a/src/ngx-translate-loaders/translate-browser.loader.ts +++ b/src/ngx-translate-loaders/translate-browser.loader.ts @@ -2,7 +2,7 @@ import { HttpClient } from '@angular/common/http'; import { TransferState } from '@angular/core'; import { hasValue } from '@dspace/shared/utils/empty.util'; import { TranslateLoader } from '@ngx-translate/core'; -import * as JSON5 from 'json5'; +import JSON5 from 'json5'; import { forkJoin, Observable, diff --git a/src/ngx-translate-loaders/translate-server.loader.ts b/src/ngx-translate-loaders/translate-server.loader.ts index 6e64b618e26..8b15b0db300 100644 --- a/src/ngx-translate-loaders/translate-server.loader.ts +++ b/src/ngx-translate-loaders/translate-server.loader.ts @@ -2,11 +2,14 @@ import { existsSync, readFileSync, } from 'node:fs'; -import { dirname, resolve } from 'node:path'; +import { + dirname, + resolve, +} from 'node:path'; import { TransferState } from '@angular/core'; import { TranslateLoader } from '@ngx-translate/core'; -import * as JSON5 from 'json5'; +import JSON5 from 'json5'; import { Observable, of, From 286bb5b0c49ff104904a88a1bea10fdb106ea034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kan=C3=A1sz-Nagy=20Zolt=C3=A1n?= Date: Wed, 3 Jun 2026 15:07:36 +0200 Subject: [PATCH 3/5] QREPO-405 moving resolveThemeLoadOrder into a separate util and making tests --- .../theme-i18n.util.spec.ts | 57 +++++++++ src/ngx-translate-loaders/theme-i18n.util.ts | 54 ++++++++ .../translate-browser.loader.spec.ts | 118 ++++++++++++++++++ .../translate-browser.loader.ts | 41 +----- .../translate-server.loader.ts | 50 +------- 5 files changed, 234 insertions(+), 86 deletions(-) create mode 100644 src/ngx-translate-loaders/theme-i18n.util.spec.ts create mode 100644 src/ngx-translate-loaders/theme-i18n.util.ts create mode 100644 src/ngx-translate-loaders/translate-browser.loader.spec.ts diff --git a/src/ngx-translate-loaders/theme-i18n.util.spec.ts b/src/ngx-translate-loaders/theme-i18n.util.spec.ts new file mode 100644 index 00000000000..64034b660d0 --- /dev/null +++ b/src/ngx-translate-loaders/theme-i18n.util.spec.ts @@ -0,0 +1,57 @@ +import { NamedThemeConfig } from '@dspace/config/theme.config'; + +import { resolveThemeLoadOrder } from './theme-i18n.util'; + +describe('resolveThemeLoadOrder', () => { + const theme = (name: string, ext?: string): NamedThemeConfig => + ext ? { name, extends: ext } : { name }; + + it('returns an empty array when no themes are configured', () => { + expect(resolveThemeLoadOrder([])).toEqual([]); + }); + + it('keeps configuration order for themes without an extends chain', () => { + expect(resolveThemeLoadOrder([theme('first-theme'), theme('second-theme')])) + .toEqual(['first-theme', 'second-theme']); + }); + + it('emits ancestor before descendant for an extends chain', () => { + expect(resolveThemeLoadOrder([theme('child-theme', 'parent-theme'), theme('parent-theme')])) + .toEqual(['parent-theme', 'child-theme']); + }); + + it('emits ancestor before descendant regardless of configuration order', () => { + expect(resolveThemeLoadOrder([theme('parent-theme'), theme('child-theme', 'parent-theme')])) + .toEqual(['parent-theme', 'child-theme']); + }); + + it('expands a multi-level extends chain (root → leaf)', () => { + expect(resolveThemeLoadOrder([ + theme('child-theme', 'parent-theme'), + theme('parent-theme', 'base-theme'), + theme('base-theme'), + ])).toEqual(['base-theme', 'parent-theme', 'child-theme']); + }); + + it('includes an ancestor referenced via extends even when it is not separately configured', () => { + expect(resolveThemeLoadOrder([theme('child-theme', 'parent-theme')])) + .toEqual(['parent-theme', 'child-theme']); + }); + + it('de-duplicates a shared ancestor of multiple themes', () => { + expect(resolveThemeLoadOrder([ + theme('child-theme', 'parent-theme'), + theme('sibling-theme', 'parent-theme'), + ])).toEqual(['parent-theme', 'child-theme', 'sibling-theme']); + }); + + it('terminates and emits each theme once when extends forms a cycle', () => { + const order = resolveThemeLoadOrder([theme('theme-a', 'theme-b'), theme('theme-b', 'theme-a')]); + expect(order.length).toBe(2); + expect([...order].sort()).toEqual(['theme-a', 'theme-b']); + }); + + it('ignores entries without a (truthy) name', () => { + expect(resolveThemeLoadOrder([theme(''), theme('first-theme')])).toEqual(['first-theme']); + }); +}); diff --git a/src/ngx-translate-loaders/theme-i18n.util.ts b/src/ngx-translate-loaders/theme-i18n.util.ts new file mode 100644 index 00000000000..11707e2cb5a --- /dev/null +++ b/src/ngx-translate-loaders/theme-i18n.util.ts @@ -0,0 +1,54 @@ +import { ThemeConfig } from '@dspace/config/theme.config'; + +/** + * Resolve the load order for the given configured themes, expanding their `extends` chains. + * + * For each configured theme we walk up its `extends` chain and then emit the themes from the root + * ancestor down to the descendant. Duplicates are removed (a theme already emitted as an ancestor + * of an earlier configured theme is not emitted again), and `extends` cycles are handled gracefully + * (each theme is visited at most once while building a single chain). + * + * Applying i18n override files in the resulting order makes descendant theme keys win over their + * ancestors, and later-configured themes win over earlier ones. + * + * Example given `themes: [child-theme (extends base-theme)]`: + * load order = [base-theme, child-theme] → base-theme keys first, then child-theme keys override + * + * @param configured the configured themes (typically `environment.themes`) + * @returns the ordered, de-duplicated list of theme names to load (ancestor → descendant) + */ +export function resolveThemeLoadOrder(configured: ThemeConfig[]): string[] { + const byName = new Map(); + configured.forEach((theme) => { + if (theme?.name) { + byName.set(theme.name, theme); + } + }); + + const result: string[] = []; + const seen = new Set(); + + for (const theme of configured) { + if (!theme?.name) { + continue; + } + // Build the chain from this theme up to its root ancestor (guarding against `extends` cycles). + const chain: string[] = []; + const visited = new Set(); + let cursor: string | undefined = theme.name; + while (cursor && !visited.has(cursor)) { + visited.add(cursor); + chain.push(cursor); + cursor = byName.get(cursor)?.extends; + } + // Reverse so we emit ancestor → descendant order. + for (const name of chain.reverse()) { + if (!seen.has(name)) { + seen.add(name); + result.push(name); + } + } + } + + return result; +} diff --git a/src/ngx-translate-loaders/translate-browser.loader.spec.ts b/src/ngx-translate-loaders/translate-browser.loader.spec.ts new file mode 100644 index 00000000000..68ebe7a2e0a --- /dev/null +++ b/src/ngx-translate-loaders/translate-browser.loader.spec.ts @@ -0,0 +1,118 @@ +import { + of, + throwError, +} from 'rxjs'; + +import { environment } from '../environments/environment'; +import { TranslateBrowserLoader } from './translate-browser.loader'; + +describe('TranslateBrowserLoader', () => { + const PREFIX = 'assets/i18n/'; + const SUFFIX = '.json'; + + let transferState: { get: jasmine.Spy; set: jasmine.Spy }; + let http: { get: jasmine.Spy }; + let loader: TranslateBrowserLoader; + let originalThemes: typeof environment.themes; + + /** Match the base i18n request regardless of the production content hash. */ + const isBaseRequest = (url: string): boolean => url.startsWith(`${PREFIX}en`) && url.endsWith(SUFFIX); + + beforeEach(() => { + transferState = { + get: jasmine.createSpy('get'), + set: jasmine.createSpy('set'), + }; + http = { get: jasmine.createSpy('get') }; + loader = new TranslateBrowserLoader(transferState as any, http as any, PREFIX, SUFFIX); + originalThemes = environment.themes; + }); + + afterEach(() => { + environment.themes = originalThemes; + }); + + it('returns the cached translations from the TransferState without any HTTP call', (done) => { + const cached = { 'home.title': 'Cached' }; + transferState.get.and.returnValue({ en: cached }); + + loader.getTranslation('en').subscribe((messages) => { + expect(messages).toEqual(cached); + expect(http.get).not.toHaveBeenCalled(); + done(); + }); + }); + + it('fetches the base file and merges theme overrides on top (theme keys win)', (done) => { + environment.themes = [{ name: 'custom' }]; + transferState.get.and.returnValue({}); + http.get.and.callFake((url: string) => { + if (isBaseRequest(url)) { + return of(JSON.stringify({ 'home.title': 'Base', 'home.subtitle': 'Base sub' })); + } + if (url === 'assets/custom/i18n/en.json5') { + return of('{ "home.title": "Custom" }'); + } + return throwError(() => new Error('404')); + }); + + loader.getTranslation('en').subscribe((messages) => { + expect(messages).toEqual({ 'home.title': 'Custom', 'home.subtitle': 'Base sub' }); + done(); + }); + }); + + it('falls back to the base translations when a theme override file is missing', (done) => { + environment.themes = [{ name: 'custom' }]; + transferState.get.and.returnValue({}); + http.get.and.callFake((url: string) => { + if (isBaseRequest(url)) { + return of(JSON.stringify({ 'home.title': 'Base' })); + } + return throwError(() => new Error('404')); + }); + + loader.getTranslation('en').subscribe((messages) => { + expect(messages).toEqual({ 'home.title': 'Base' }); + done(); + }); + }); + + it('applies overrides in inheritance order so descendant themes win over ancestors', (done) => { + environment.themes = [{ name: 'child-theme', extends: 'parent-theme' }, { name: 'parent-theme' }]; + transferState.get.and.returnValue({}); + http.get.and.callFake((url: string) => { + if (isBaseRequest(url)) { + return of(JSON.stringify({ key: 'base' })); + } + if (url === 'assets/parent-theme/i18n/en.json5') { + return of('{ key: "parent" }'); + } + if (url === 'assets/child-theme/i18n/en.json5') { + return of('{ key: "child" }'); + } + return throwError(() => new Error('404')); + }); + + loader.getTranslation('en').subscribe((messages) => { + expect(messages.key).toBe('child'); + done(); + }); + }); + + it('returns the base translations unchanged when no themes are configured', (done) => { + environment.themes = []; + transferState.get.and.returnValue({}); + http.get.and.callFake((url: string) => { + if (isBaseRequest(url)) { + return of(JSON.stringify({ 'home.title': 'Base' })); + } + return throwError(() => new Error('404')); + }); + + loader.getTranslation('en').subscribe((messages) => { + expect(messages).toEqual({ 'home.title': 'Base' }); + done(); + }); + }); +}); diff --git a/src/ngx-translate-loaders/translate-browser.loader.ts b/src/ngx-translate-loaders/translate-browser.loader.ts index bb973576441..93d20426591 100644 --- a/src/ngx-translate-loaders/translate-browser.loader.ts +++ b/src/ngx-translate-loaders/translate-browser.loader.ts @@ -18,6 +18,7 @@ import { NGX_TRANSLATE_STATE, NgxTranslateState, } from './ngx-translate-state'; +import { resolveThemeLoadOrder } from './theme-i18n.util'; /** * A TranslateLoader for ngx-translate to retrieve i18n messages from the TransferState, or download @@ -55,7 +56,7 @@ export class TranslateBrowserLoader implements TranslateLoader { // Fetch base translations + every configured theme's override file (in inheritance order) // and merge them. Theme keys override base keys; child theme keys override parent theme keys. const base$ = this.fetchBase(lang); - const orderedThemes = this.resolveThemeLoadOrder(environment.themes as any || []); + const orderedThemes = resolveThemeLoadOrder(environment.themes ?? []); const themeStreams = orderedThemes.map((name: string) => this.fetchThemeOverride(name, lang)); return forkJoin([base$, ...themeStreams]).pipe( @@ -63,44 +64,6 @@ export class TranslateBrowserLoader implements TranslateLoader { ); } - /** - * Resolve the load order for the configured themes, expanding `extends` chains so that - * ancestor themes are loaded before their descendants (descendant wins on conflicts). - * See {@link TranslateServerLoader.resolveThemeLoadOrder} for details. - */ - protected resolveThemeLoadOrder(configured: Array<{ name?: string; extends?: string }>): string[] { - const byName = new Map(); - configured.forEach((t) => { - if (t?.name) { - byName.set(t.name, t); - } - }); - - const result: string[] = []; - const seen = new Set(); - - for (const theme of configured) { - if (!theme?.name) { - continue; - } - const chain: string[] = []; - const visited = new Set(); - let cursor: string | undefined = theme.name; - while (cursor && !visited.has(cursor)) { - visited.add(cursor); - chain.push(cursor); - cursor = byName.get(cursor)?.extends; - } - for (const name of chain.reverse()) { - if (!seen.has(name)) { - seen.add(name); - result.push(name); - } - } - } - return result; - } - /** * Fetch the base i18n file (the hashed JSON produced by the build pipeline). */ diff --git a/src/ngx-translate-loaders/translate-server.loader.ts b/src/ngx-translate-loaders/translate-server.loader.ts index 8b15b0db300..2c16a18dbad 100644 --- a/src/ngx-translate-loaders/translate-server.loader.ts +++ b/src/ngx-translate-loaders/translate-server.loader.ts @@ -20,6 +20,7 @@ import { NGX_TRANSLATE_STATE, NgxTranslateState, } from './ngx-translate-state'; +import { resolveThemeLoadOrder } from './theme-i18n.util'; /** * A TranslateLoader for ngx-translate to parse json5 files server-side, and store them in the @@ -67,14 +68,14 @@ export class TranslateServerLoader implements TranslateLoader { * @protected */ protected readThemeOverrides(lang: string): Record { - const configured = (environment.themes || []) as Array<{ name?: string; extends?: string }>; + const configured = environment.themes ?? []; if (configured.length === 0) { return {}; } // Build the ordered list of themes to load, expanding each configured theme's inheritance // chain (ancestor first, descendant last) so that child theme overrides win over parents. - const orderedThemes = this.resolveThemeLoadOrder(configured); + const orderedThemes = resolveThemeLoadOrder(configured); // `this.prefix` looks like `dist/server/assets/i18n/`. Theme assets live next to `i18n/` // at `dist/server/assets//i18n/.json5`. @@ -97,51 +98,6 @@ export class TranslateServerLoader implements TranslateLoader { return overrides; } - /** - * Resolve the load order for the given configured themes, expanding `extends` chains. - * - * For each configured theme we walk up its `extends` chain, then emit themes from the root - * ancestor down to the descendant. Duplicates are removed (a theme already loaded as an - * ancestor of an earlier configured theme is not loaded again). - * - * Example given `themes: [kjk (extends qulto)]`: - * load order = [qulto, kjk] → qulto keys, then kjk keys override - */ - protected resolveThemeLoadOrder(configured: Array<{ name?: string; extends?: string }>): string[] { - const byName = new Map(); - configured.forEach((t) => { - if (t?.name) { - byName.set(t.name, t); - } - }); - - const result: string[] = []; - const seen = new Set(); - - for (const theme of configured) { - if (!theme?.name) { - continue; - } - // Build chain from theme up to root ancestor (handle cycles). - const chain: string[] = []; - const visited = new Set(); - let cursor: string | undefined = theme.name; - while (cursor && !visited.has(cursor)) { - visited.add(cursor); - chain.push(cursor); - cursor = byName.get(cursor)?.extends; - } - // Reverse so we apply ancestor → descendant order. - for (const name of chain.reverse()) { - if (!seen.has(name)) { - seen.add(name); - result.push(name); - } - } - } - return result; - } - /** * Store the i18n messages for the given language code in the transfer state, so they can be * retrieved client side From 8143bf211b062dd9e36e0c46c752fd4c62f0b831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kan=C3=A1sz-Nagy=20Zolt=C3=A1n?= Date: Thu, 4 Jun 2026 21:25:07 +0200 Subject: [PATCH 4/5] QREPO-405 using active theme chain for determining theme order --- .../theme-i18n.util.spec.ts | 73 +++++++++++-------- src/ngx-translate-loaders/theme-i18n.util.ts | 68 ++++++++--------- .../translate-browser.loader.spec.ts | 56 ++++++++++++++ .../translate-browser.loader.ts | 17 +++-- .../translate-server.loader.ts | 16 ++-- 5 files changed, 148 insertions(+), 82 deletions(-) diff --git a/src/ngx-translate-loaders/theme-i18n.util.spec.ts b/src/ngx-translate-loaders/theme-i18n.util.spec.ts index 64034b660d0..5d17d0c9adc 100644 --- a/src/ngx-translate-loaders/theme-i18n.util.spec.ts +++ b/src/ngx-translate-loaders/theme-i18n.util.spec.ts @@ -1,57 +1,68 @@ import { NamedThemeConfig } from '@dspace/config/theme.config'; -import { resolveThemeLoadOrder } from './theme-i18n.util'; +import { resolveActiveThemeChain } from './theme-i18n.util'; -describe('resolveThemeLoadOrder', () => { +describe('resolveActiveThemeChain', () => { const theme = (name: string, ext?: string): NamedThemeConfig => ext ? { name, extends: ext } : { name }; - it('returns an empty array when no themes are configured', () => { - expect(resolveThemeLoadOrder([])).toEqual([]); + it('returns an empty array when activeName is empty', () => { + expect(resolveActiveThemeChain([theme('custom')], '')).toEqual([]); }); - it('keeps configuration order for themes without an extends chain', () => { - expect(resolveThemeLoadOrder([theme('first-theme'), theme('second-theme')])) - .toEqual(['first-theme', 'second-theme']); + it('returns [active] when the active theme has no extends', () => { + expect(resolveActiveThemeChain([theme('custom')], 'custom')).toEqual(['custom']); }); - it('emits ancestor before descendant for an extends chain', () => { - expect(resolveThemeLoadOrder([theme('child-theme', 'parent-theme'), theme('parent-theme')])) - .toEqual(['parent-theme', 'child-theme']); + it('returns [parent, active] when active extends a configured parent', () => { + expect(resolveActiveThemeChain( + [theme('custom'), theme('child-theme', 'custom')], + 'child-theme', + )).toEqual(['custom', 'child-theme']); }); - it('emits ancestor before descendant regardless of configuration order', () => { - expect(resolveThemeLoadOrder([theme('parent-theme'), theme('child-theme', 'parent-theme')])) - .toEqual(['parent-theme', 'child-theme']); + it('resolves the chain even when the active theme is listed before its parent', () => { + expect(resolveActiveThemeChain( + [theme('child-theme', 'parent-theme'), theme('parent-theme')], + 'child-theme', + )).toEqual(['parent-theme', 'child-theme']); }); - it('expands a multi-level extends chain (root → leaf)', () => { - expect(resolveThemeLoadOrder([ - theme('child-theme', 'parent-theme'), - theme('parent-theme', 'base-theme'), - theme('base-theme'), - ])).toEqual(['base-theme', 'parent-theme', 'child-theme']); + it('expands a multi-level chain (root → mid → active)', () => { + expect(resolveActiveThemeChain( + [theme('child-theme', 'parent-theme'), theme('parent-theme', 'base-theme'), theme('base-theme')], + 'child-theme', + )).toEqual(['base-theme', 'parent-theme', 'child-theme']); }); - it('includes an ancestor referenced via extends even when it is not separately configured', () => { - expect(resolveThemeLoadOrder([theme('child-theme', 'parent-theme')])) - .toEqual(['parent-theme', 'child-theme']); + it('includes an ancestor even when it is not separately listed in configured themes', () => { + expect(resolveActiveThemeChain( + [theme('child-theme', 'parent-theme')], + 'child-theme', + )).toEqual(['parent-theme', 'child-theme']); }); - it('de-duplicates a shared ancestor of multiple themes', () => { - expect(resolveThemeLoadOrder([ - theme('child-theme', 'parent-theme'), - theme('sibling-theme', 'parent-theme'), - ])).toEqual(['parent-theme', 'child-theme', 'sibling-theme']); + it('does NOT include sibling themes — only the active chain', () => { + // sibling-theme and child-theme both extend parent-theme; when child-theme is active, + // sibling-theme must not appear in the chain. + const chain = resolveActiveThemeChain( + [theme('parent-theme'), theme('child-theme', 'parent-theme'), theme('sibling-theme', 'parent-theme')], + 'child-theme', + ); + expect(chain).toEqual(['parent-theme', 'child-theme']); + expect(chain).not.toContain('sibling-theme'); }); - it('terminates and emits each theme once when extends forms a cycle', () => { - const order = resolveThemeLoadOrder([theme('theme-a', 'theme-b'), theme('theme-b', 'theme-a')]); + it('terminates and includes each theme once when extends forms a cycle', () => { + const order = resolveActiveThemeChain( + [theme('theme-a', 'theme-b'), theme('theme-b', 'theme-a')], + 'theme-a', + ); expect(order.length).toBe(2); expect([...order].sort()).toEqual(['theme-a', 'theme-b']); }); - it('ignores entries without a (truthy) name', () => { - expect(resolveThemeLoadOrder([theme(''), theme('first-theme')])).toEqual(['first-theme']); + it('returns [active] when activeName does not match any configured theme', () => { + expect(resolveActiveThemeChain([theme('custom')], 'unknown-theme')).toEqual(['unknown-theme']); }); }); diff --git a/src/ngx-translate-loaders/theme-i18n.util.ts b/src/ngx-translate-loaders/theme-i18n.util.ts index 11707e2cb5a..6f860025660 100644 --- a/src/ngx-translate-loaders/theme-i18n.util.ts +++ b/src/ngx-translate-loaders/theme-i18n.util.ts @@ -1,54 +1,44 @@ import { ThemeConfig } from '@dspace/config/theme.config'; /** - * Resolve the load order for the given configured themes, expanding their `extends` chains. + * Resolve the i18n load order for a single **active** theme, walking up its + * `extends` chain so ancestors are loaded before descendants (ancestor keys + * are overridden by descendant keys). * - * For each configured theme we walk up its `extends` chain and then emit the themes from the root - * ancestor down to the descendant. Duplicates are removed (a theme already emitted as an ancestor - * of an earlier configured theme is not emitted again), and `extends` cycles are handled gracefully - * (each theme is visited at most once while building a single chain). + * Only the active theme's inheritance chain is returned. Sibling themes that + * are configured but not active are intentionally excluded — loading all + * configured themes caused cross-theme key pollution (a sibling theme's + * translations silently overriding the active theme's keys). * - * Applying i18n override files in the resulting order makes descendant theme keys win over their - * ancestors, and later-configured themes win over earlier ones. + * This mirrors the official `merge-i18n` CLI approach, which merges exactly + * one theme at a time: `npm run merge-i18n -- -s src/themes//assets/i18n`. * - * Example given `themes: [child-theme (extends base-theme)]`: - * load order = [base-theme, child-theme] → base-theme keys first, then child-theme keys override + * `extends` cycles are handled gracefully (each theme name appears at most once). + * + * Examples (config `[base-theme, child-theme (extends base-theme), sibling-theme (extends base-theme)]`): + * active = 'base-theme' (no extends) → ['base-theme'] + * active = 'child-theme' (extends: 'base-theme') → ['base-theme', 'child-theme'] + * active = 'sibling-theme' (extends: 'base-theme') → ['base-theme', 'sibling-theme'] * * @param configured the configured themes (typically `environment.themes`) - * @returns the ordered, de-duplicated list of theme names to load (ancestor → descendant) + * @param activeName the name of the active/default theme + * @returns ordered list of theme names to load (root ancestor → active theme) */ -export function resolveThemeLoadOrder(configured: ThemeConfig[]): string[] { +export function resolveActiveThemeChain(configured: ThemeConfig[], activeName: string): string[] { const byName = new Map(); - configured.forEach((theme) => { - if (theme?.name) { - byName.set(theme.name, theme); + configured.forEach((t) => { + if (t?.name) { + byName.set(t.name, t); } }); - const result: string[] = []; - const seen = new Set(); - - for (const theme of configured) { - if (!theme?.name) { - continue; - } - // Build the chain from this theme up to its root ancestor (guarding against `extends` cycles). - const chain: string[] = []; - const visited = new Set(); - let cursor: string | undefined = theme.name; - while (cursor && !visited.has(cursor)) { - visited.add(cursor); - chain.push(cursor); - cursor = byName.get(cursor)?.extends; - } - // Reverse so we emit ancestor → descendant order. - for (const name of chain.reverse()) { - if (!seen.has(name)) { - seen.add(name); - result.push(name); - } - } + const chain: string[] = []; + const visited = new Set(); + let cursor: string | undefined = activeName; + while (cursor && !visited.has(cursor)) { + visited.add(cursor); + chain.push(cursor); + cursor = byName.get(cursor)?.extends; } - - return result; + return chain.reverse(); // ancestor → descendant } diff --git a/src/ngx-translate-loaders/translate-browser.loader.spec.ts b/src/ngx-translate-loaders/translate-browser.loader.spec.ts index 68ebe7a2e0a..02e5e42d625 100644 --- a/src/ngx-translate-loaders/translate-browser.loader.spec.ts +++ b/src/ngx-translate-loaders/translate-browser.loader.spec.ts @@ -115,4 +115,60 @@ describe('TranslateBrowserLoader', () => { done(); }); }); + + it('does NOT apply sibling theme overrides when the active theme is first in the list', (done) => { + // Regression: when parent-theme is active, a sibling theme that also extends parent-theme + // must NOT override parent-theme's translations. + environment.themes = [ + { name: 'parent-theme' }, + { name: 'sibling-theme', extends: 'parent-theme' }, + ]; + transferState.get.and.returnValue({}); + http.get.and.callFake((url: string) => { + if (isBaseRequest(url)) { + return of(JSON.stringify({ key: 'base' })); + } + if (url === 'assets/parent-theme/i18n/en.json5') { + return of('{ key: "parent" }'); + } + if (url === 'assets/sibling-theme/i18n/en.json5') { + // sibling has its own value for the same key — must NOT win when parent-theme is active + return of('{ key: "sibling" }'); + } + return throwError(() => new Error('404')); + }); + + loader.getTranslation('en').subscribe((messages) => { + expect(messages.key).toBe('parent'); + done(); + }); + }); + + it('applies the full ancestor chain when a child theme is active', (done) => { + // When child-theme (extends parent-theme) is active, parent-theme keys provide the base + // and child-theme keys override only what they define. + environment.themes = [ + { name: 'child-theme', extends: 'parent-theme' }, + { name: 'parent-theme' }, + ]; + transferState.get.and.returnValue({}); + http.get.and.callFake((url: string) => { + if (isBaseRequest(url)) { + return of(JSON.stringify({ 'key.a': 'base-a', 'key.b': 'base-b' })); + } + if (url === 'assets/parent-theme/i18n/en.json5') { + return of('{ "key.a": "parent-a" }'); + } + if (url === 'assets/child-theme/i18n/en.json5') { + return of('{ "key.b": "child-b" }'); + } + return throwError(() => new Error('404')); + }); + + loader.getTranslation('en').subscribe((messages) => { + expect(messages['key.a']).toBe('parent-a'); // inherited from parent-theme + expect(messages['key.b']).toBe('child-b'); // overridden by child-theme + done(); + }); + }); }); diff --git a/src/ngx-translate-loaders/translate-browser.loader.ts b/src/ngx-translate-loaders/translate-browser.loader.ts index 93d20426591..efbd109c895 100644 --- a/src/ngx-translate-loaders/translate-browser.loader.ts +++ b/src/ngx-translate-loaders/translate-browser.loader.ts @@ -1,5 +1,7 @@ import { HttpClient } from '@angular/common/http'; import { TransferState } from '@angular/core'; +import { AppConfig } from '@dspace/config/app-config.interface'; +import { getDefaultThemeConfig } from '@dspace/config/config.util'; import { hasValue } from '@dspace/shared/utils/empty.util'; import { TranslateLoader } from '@ngx-translate/core'; import JSON5 from 'json5'; @@ -18,14 +20,16 @@ import { NGX_TRANSLATE_STATE, NgxTranslateState, } from './ngx-translate-state'; -import { resolveThemeLoadOrder } from './theme-i18n.util'; +import { resolveActiveThemeChain } from './theme-i18n.util'; /** * A TranslateLoader for ngx-translate to retrieve i18n messages from the TransferState, or download * them if they're not available there. * - * Also fetches per-theme translation overrides from `assets//i18n/.json5` for every - * configured theme and deep-merges them on top of the base translations (theme keys win). + * Also fetches per-theme translation overrides from `assets//i18n/.json5` for the + * active theme and its ancestors (root → active), merging them on top of the base translations + * so the active theme's keys win. Only the active theme's inheritance chain is loaded — sibling + * themes are excluded to prevent cross-theme key pollution. * This removes the need for the build-time `merge-i18n` script, allowing a single Docker image * to serve multiple customers with their own theme-specific translations. */ @@ -53,10 +57,11 @@ export class TranslateBrowserLoader implements TranslateLoader { return of(messages); } - // Fetch base translations + every configured theme's override file (in inheritance order) - // and merge them. Theme keys override base keys; child theme keys override parent theme keys. + // Fetch base translations + the active theme's override files (root ancestor → active theme). + // Only the active theme's inheritance chain is loaded; sibling themes are excluded. const base$ = this.fetchBase(lang); - const orderedThemes = resolveThemeLoadOrder(environment.themes ?? []); + const activeName = getDefaultThemeConfig(environment as unknown as AppConfig).name; + const orderedThemes = resolveActiveThemeChain(environment.themes ?? [], activeName); const themeStreams = orderedThemes.map((name: string) => this.fetchThemeOverride(name, lang)); return forkJoin([base$, ...themeStreams]).pipe( diff --git a/src/ngx-translate-loaders/translate-server.loader.ts b/src/ngx-translate-loaders/translate-server.loader.ts index 2c16a18dbad..d05912e03ba 100644 --- a/src/ngx-translate-loaders/translate-server.loader.ts +++ b/src/ngx-translate-loaders/translate-server.loader.ts @@ -8,6 +8,7 @@ import { } from 'node:path'; import { TransferState } from '@angular/core'; +import { getDefaultThemeConfig } from '@dspace/config/config.util'; import { TranslateLoader } from '@ngx-translate/core'; import JSON5 from 'json5'; import { @@ -20,14 +21,16 @@ import { NGX_TRANSLATE_STATE, NgxTranslateState, } from './ngx-translate-state'; -import { resolveThemeLoadOrder } from './theme-i18n.util'; +import { resolveActiveThemeChain } from './theme-i18n.util'; /** * A TranslateLoader for ngx-translate to parse json5 files server-side, and store them in the * TransferState. * * Also overlays per-theme translation overrides from `//i18n/.json5` - * for every configured theme, merging them on top of the base translations (theme keys win). + * for the active theme and its ancestors (in root → active order), merging them on top of the + * base translations so the active theme's keys win. Only the active theme's inheritance chain is + * loaded — sibling themes are excluded to prevent cross-theme key pollution. * The resulting merged map is stored in the TransferState so the browser does not need to * re-fetch the theme overrides after the initial SSR response. */ @@ -61,7 +64,8 @@ export class TranslateServerLoader implements TranslateLoader { } /** - * Read and merge i18n override files from every configured theme's assets folder. + * Read and merge i18n override files for the active theme's inheritance chain. + * Only the active theme (and its ancestors) are loaded; sibling themes are excluded. * Returns an empty object if no overrides are present. * * @param lang the language code @@ -73,9 +77,9 @@ export class TranslateServerLoader implements TranslateLoader { return {}; } - // Build the ordered list of themes to load, expanding each configured theme's inheritance - // chain (ancestor first, descendant last) so that child theme overrides win over parents. - const orderedThemes = resolveThemeLoadOrder(configured); + // Resolve only the active theme's chain (root ancestor → active theme). + const activeName = getDefaultThemeConfig(environment).name; + const orderedThemes = resolveActiveThemeChain(configured, activeName); // `this.prefix` looks like `dist/server/assets/i18n/`. Theme assets live next to `i18n/` // at `dist/server/assets//i18n/.json5`. From c594b07a73a0d42e4c8d038b00351a3fb4e545f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kan=C3=A1sz-Nagy=20Zolt=C3=A1n?= Date: Thu, 4 Jun 2026 21:55:02 +0200 Subject: [PATCH 5/5] QREPO-405 fix type cast for getDefaultThemeConfig in server loader --- src/ngx-translate-loaders/translate-server.loader.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ngx-translate-loaders/translate-server.loader.ts b/src/ngx-translate-loaders/translate-server.loader.ts index d05912e03ba..82b331bd4c0 100644 --- a/src/ngx-translate-loaders/translate-server.loader.ts +++ b/src/ngx-translate-loaders/translate-server.loader.ts @@ -8,6 +8,7 @@ import { } from 'node:path'; import { TransferState } from '@angular/core'; +import { AppConfig } from '@dspace/config/app-config.interface'; import { getDefaultThemeConfig } from '@dspace/config/config.util'; import { TranslateLoader } from '@ngx-translate/core'; import JSON5 from 'json5'; @@ -78,7 +79,7 @@ export class TranslateServerLoader implements TranslateLoader { } // Resolve only the active theme's chain (root ancestor → active theme). - const activeName = getDefaultThemeConfig(environment).name; + const activeName = getDefaultThemeConfig(environment as unknown as AppConfig).name; const orderedThemes = resolveActiveThemeChain(configured, activeName); // `this.prefix` looks like `dist/server/assets/i18n/`. Theme assets live next to `i18n/`