Skip to content

Fix #5652 - Runtime i18n Loading for Theme Translations (no build-time merge for i18n files) - #5653

Open
kanasznagyzoltan wants to merge 7 commits into
DSpace:mainfrom
qultoltd:QREPO-405-Runtime-i18n-Loading-for-Theme-Translations
Open

Fix #5652 - Runtime i18n Loading for Theme Translations (no build-time merge for i18n files)#5653
kanasznagyzoltan wants to merge 7 commits into
DSpace:mainfrom
qultoltd:QREPO-405-Runtime-i18n-Loading-for-Theme-Translations

Conversation

@kanasznagyzoltan

@kanasznagyzoltan kanasznagyzoltan commented May 13, 2026

Copy link
Copy Markdown
Contributor

References

Description

Make the i18n TranslateLoader theme-aware: instead of relying on the build-time merge-i18n script, theme-specific translation overrides are loaded and merged at runtime from assets/<theme>/i18n/<lang>.json5, following the active theme's extends chain. This lets a single Docker image serve multiple customers/themes with their own translations.

Instructions for Reviewers

Changes in this PR:

  • src/ngx-translate-loaders/translate-server.loader.ts — After reading the base i18n file (dist/.../assets/i18n/<lang>.<hash>.json), the loader overlays the active theme's override files (dist/.../assets/<theme>/i18n/<lang>.json5) on top, in inheritance order (root ancestor first, active theme last, so the active theme's keys win). A malformed theme file is skipped with a warning, so it cannot break SSR. The merged map is stored in NGX_TRANSLATE_STATE, so the browser does not re-fetch theme overrides after SSR.
  • src/ngx-translate-loaders/translate-browser.loader.ts — When the TransferState is empty (CSR / dev mode), the loader fetches the base i18n file plus the active theme chain's override .json5 files over HTTP and merges them client-side. Missing override files are tolerated via catchError.
  • src/ngx-translate-loaders/theme-i18n.util.ts (new) — Shared resolveActiveThemeChain() helper used by both loaders. It walks the active theme's extends chain up to its root ancestor and returns the theme names in ancestor → descendant order, with cycle protection. Sibling themes that are configured but not active are deliberately excluded: loading every configured theme allowed unrelated themes' keys to silently override the active theme's. This mirrors the merge-i18n CLI, which also merges exactly one theme at a time. The active theme is the default theme config — the entry with no regex, handle or uuid rule.
  • docs/ThemeTranslations.md (new) — Documentation covering where theme translation files go, the merge order, which theme's overrides apply, SSR/CSR behaviour, and the relationship to merge-i18n. Linked from the README file listing.
  • Teststheme-i18n.util.spec.ts covers the load-order logic (extends chains, active-chain-only behaviour, cycles); translate-browser.loader.spec.ts covers the browser merge (TransferState short-circuit, base + theme merge precedence, missing-file fallback). The server loader imports Node's fs/path, which cannot run in the browser-based Karma suite, so its ordering is covered by the shared-util spec and its merge mirrors the browser path.
  • No API or configuration changes. Themes without assets/<theme>/i18n/*.json5 files are ignored (missing file → empty object → no effect on the merged result). Existing setups that still run merge-i18n keep working unchanged.

How it works

Given a config like:

themes:
  - name: custom-child
    extends: custom-parent

…and i18n files at:

src/assets/i18n/hu.json5                          (base)
src/themes/custom-parent/assets/i18n/hu.json5     (parent overrides)
src/themes/custom-child/assets/i18n/hu.json5      (child overrides)

…then with custom-child as the active theme, the loader merges them at runtime in the order base → custom-parent → custom-child, so custom-child keys override custom-parent keys, which override base keys. If custom-child does not define a key, it falls back to custom-parent's value, then to base.

How to test

  1. Ensure src/assets/i18n/<lang>.json5 contains only base content (no merged theme content), and do not run npm run merge-i18n.
  2. Place theme-specific overrides in src/themes/<theme>/assets/i18n/<lang>.json5.
  3. Build (npm run build:prod) and serve the SSR output. On the home page, verify that theme-specific keys render the override value, that switching language keeps using the theme override, and that non-overridden keys fall back to the base translation.
  4. SSR hydration: in DevTools → Network, the initial HTML already contains the translated text, and no request to /assets/<theme>/i18n/<lang>.json5 fires on first navigation (served from NGX_TRANSLATE_STATE).
  5. CSR fallback: run npm run start:dev (no SSR) and confirm the loader fetches base + theme files over HTTP and merges them in the browser.
  6. Inheritance (optional): with a child theme that extends another, override a key only in the child and another only in the parent — both should resolve correctly when the child theme is active.
  7. No regression: a project with no theme i18n files behaves identically to before this PR.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using npm run lint
  • My PR doesn't introduce circular dependencies (verified via npm run check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR aligns with Accessibility guidelines if it makes changes to the user interface.
  • My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations.
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

@lgeggleston lgeggleston added improvement i18n / l10n Internationalisation and localisation, related to message catalogs code task themes labels May 13, 2026
@lgeggleston lgeggleston moved this to 🙋 Needs Reviewers Assigned in DSpace 10.0 Release May 13, 2026
@lgeggleston

Copy link
Copy Markdown
Contributor

Hi @kanasznagyzoltan, thank you for this PR! Since this would probably qualify as an improvement to build process rather than a new user feature, I added it to the 10.0 project to start. However since we're close to the May 22 code freeze, this will likely go towards a later release unless reviews are very quick for it.

@lgeggleston lgeggleston added the needs documentation PR is missing documentation. All new features and config changes require documentation. label May 13, 2026
@lgeggleston lgeggleston moved this to 🙋 Needs Reviewers Assigned in DSpace 11.0 Release Jun 2, 2026
@kanasznagyzoltan

Copy link
Copy Markdown
Contributor Author

Hi @kanasznagyzoltan, thank you for this PR! Since this would probably qualify as an improvement to build process rather than a new user feature, I added it to the 10.0 project to start. However since we're close to the May 22 code freeze, this will likely go towards a later release unless reviews are very quick for it.

Thank you @lgeggleston
I have updated my PR following the fresh release of the DSpace 10.0 release.
I’d appreciate it if you and others could take a look and share their's thoughts.

@kanasznagyzoltan
kanasznagyzoltan force-pushed the QREPO-405-Runtime-i18n-Loading-for-Theme-Translations branch from d049149 to 8143bf2 Compare June 4, 2026 19:35
@kanasznagyzoltan
kanasznagyzoltan force-pushed the QREPO-405-Runtime-i18n-Loading-for-Theme-Translations branch from f40f41d to c594b07 Compare June 4, 2026 19:58
@kanasznagyzoltan

kanasznagyzoltan commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Hi @lgeggleston (and @saschaszott who welcomed this PR on Slack) — a gentle nudge on this one. It's now rebased on the current main, CI is fully green, there are no conflicts, and it's an isolated change (+410/−19 across 5 files, only the i18n loader), so it should be low-risk to review.

5-minute test recipe:

  • Pick two themes with an extends chain (a child theme extending a parent).
  • In the child theme, add src/assets//i18n/en.json5 with a single overridden key, e.g. "nav.home": "Home (theme override)".
  • npm run build once, then run the server build or npm run start:dev — no merge-i18n step needed.
  • Load the UI with that theme active: the theme's "Home (theme override)" value wins over the base key, following the extends order (ancestor first, child theme last), and it's applied at runtime — no per-theme rebuild.

Regarding the needs documentation label — happy to write this up; would a short page on the DSpace wiki (under the theming / i18n section) be the right place, or a section in the repo docs? I can have it ready quickly so it doesn't hold up the merge.

Even a quick review before Jun 30 would be hugely appreciated. Thanks!

@lgeggleston

Copy link
Copy Markdown
Contributor

@kanasznagyzoltan: I can definitely put this on my list to take a look, but to be honest I can't guarantee that will be possible by the 30th - and for this change I'd only be equipped to test rather than code review. I only do limited amounts of PR testing at the moment, and I usually try to focus first on either time-sensitive fixes or PRs that already have at least 1 review and need a secondary tester.

Thank you for keeping this up to date, and I'm aware it's frustrating to not have reviews yet - the large backlog of PRs without reviewers is unfortunately one of the biggest struggles DSpace has been dealing with (and on that note, all the testing and reviewing you and your team have been doing is deeply appreciated!). I can only give the usual recommendations for this - you are welcome to post in the DSpace Slack, or ask another developer to Trade reviews.

Second - thanks for being ready to write up a little documentation. The DSpace wiki would be the right place; one place it could work is in this section, but wherever within the wiki you feel it makes sense is likely good!

@kanasznagyzoltan

Copy link
Copy Markdown
Contributor Author

@lgeggleston
Thanks, that's helpful. I haven't written any DSpace documentation yet — I'm still looking into how the documentation process works.

In the meantime, do you happen to know whether anyone has been able to take a look at the PR since? Even high-level feedback on the approach itself would be very welcome — I'm happy to adjust based on it.

@kanasznagyzoltan

Copy link
Copy Markdown
Contributor Author

Hi @lgeggleston,

Update — this is now merged with current main (no conflicts, CI green), and I've added documentation in docs/ThemeTranslations.md, linked from the README file listing. That should cover the needs documentation label.

I've also corrected the PR description: an earlier revision loaded every configured theme, which let sibling themes' keys overwrite one another, so it now loads only the active theme's extends chain. The description was still describing the old behaviour.

Short recap for anyone picking this up: it makes the i18n TranslateLoader theme-aware, so theme translation overrides are applied at runtime instead of being merged into the base language files at build time. The practical payoff is that a single build — and a single Docker image — can serve several sites, each with its own theme translations, rather than needing a separate build per theme. It's self-contained: three source files under src/ngx-translate-loaders/ plus their specs, no API or configuration changes, and setups that still run merge-i18n are unaffected.

There's a quick (5-10 minute) testing recipe in my comment above, and I'm happy to walk anyone through it on Slack or to adjust the approach if reviewers would prefer something different.

@alanorth

Copy link
Copy Markdown
Contributor

Thank you @kanasznagyzoltan! I tried this today with the following configuration:

  • Overriding several strings in src/themes/dspace/assets/i18n/en.json5
  • Overriding several strings in src/themes/dspace/assets/i18n/es.json5

Note that I used the dspace theme in the default configuration in production mode without any extends and it worked perfectly. Some sites customize the default dspace theme directly and I was glad to see that this works. I think this patch will make maintenance of custom strings vastly easier for site operators.

So this is +1 from me by testing. I have not reviewed the code.

@alanorth
alanorth self-requested a review August 29, 2026 06:55
@github-project-automation github-project-automation Bot moved this from 🙋 Needs Reviewers Assigned to 👍 Reviewer Approved in DSpace 11.0 Release Aug 29, 2026
@kanasznagyzoltan

Copy link
Copy Markdown
Contributor Author

Thank you @kanasznagyzoltan! I tried this today with the following configuration:

  • Overriding several strings in src/themes/dspace/assets/i18n/en.json5
  • Overriding several strings in src/themes/dspace/assets/i18n/es.json5

Note that I used the dspace theme in the default configuration in production mode without any extends and it worked perfectly. Some sites customize the default dspace theme directly and I was glad to see that this works. I think this patch will make maintenance of custom strings vastly easier for site operators.

So this is +1 from me by testing. I have not reviewed the code.

Thank you, @alanorth — much appreciated, and good to have the default dspace theme with no extends confirmed, since that's probably the most common setup.

If you still have it running, would you mind also trying a theme that extends another? That's the one path your test didn't cover: a key defined only in the parent should fall through to the child, and a key defined in both should resolve to the child's value.

@alanorth

Copy link
Copy Markdown
Contributor

Sorry @kanasznagyzoltan, the only site with parent and child themes I have access to is running DSpace 8 so I can't test the extends scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code task i18n / l10n Internationalisation and localisation, related to message catalogs improvement needs documentation PR is missing documentation. All new features and config changes require documentation. themes

Projects

Status: 👍 Reviewer Approved

Development

Successfully merging this pull request may close these issues.

Runtime i18n Loading for Theme Translations (no build-time merge for i18n files)

3 participants