[kanasznagyzoltan]

References

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

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.
• Tests — theme-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]

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.

[kanasznagyzoltan]

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]

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]

@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]

@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]

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]

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.

[kanasznagyzoltan]

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]

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.

[kanasznagyzoltan]

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.

No problem — the extends path is covered by unit tests, so that's not blocking. Could you suggest someone who might be able to review the code, or give it a second look?

[kanasznagyzoltan]

@tdonohue sorry to tag you directly. @alanorth kindly tested this and gave a +1, and @mwood offered on Slack to take a look as well. Documentation was added in docs/ThemeTranslations.md, so the needs documentation label is now stale and can probably come off.

Would you be able to point me to someone who could do the code review? It's on current main with green CI, and it's self-contained — three source files under src/ngx-translate-loaders/ plus their specs.

[alanorth]

Thanks again @kanasznagyzoltan. I have removed the needs documentation tag and completed the checklist in the PR description. In the mean time you could rebase it on top of current main if you have time.

I was going to propose that this be added to the list of topics in the next DSpace developer meeting because it is a significant and welcome improvement for managing custom strings.

[kanasznagyzoltan]

Thanks @alanorth — for cleaning up the label, filling in the checklist, and especially for offering to raise this at a developer meeting.
I have just merged the current main, so the branch is up to date and conflict-free.

[kanasznagyzoltan]

The Node 20.x job failed on an AuthService test, which isn't related to the changes in this PR — it only touches src/ngx-translate-loaders/ plus docs. Could someone with the necessary permissions re-run the failed job?

[alanorth]

Thanks, @kanasznagyzoltan. The CI failures were transient and are passing now. Now we need to have one more reviewer test this and comment on implementation since I only did a functional review.

[kanasznagyzoltan]

Thanks, @kanasznagyzoltan. The CI failures were transient and are passing now. Now we need to have one more reviewer test this and comment on implementation since I only did a functional review.

Thank you @alanorth :)

[milanmajchrak]

Hi @kanasznagyzoltan ,
Thanks for this PR, I like the idea. Customers often want just a few strings worded differently. With this they can override only those keys in their theme instead of copying the whole file.

I tested it locally in Docker (main vs. this PR, same backend) using the Claude Code and recorded a video. I checked the results myself in the browser.

One concern: the browser now always requests assets/<theme>/i18n/<lang>.json5, even when the theme has no overrides (translate-browser.loader.ts#L89-L94). The default dspace theme has no such file, so instances that don't use this feature get:

• a 404 error in the browser console (GET /assets/dspace/i18n/en.json5 404)
• a slower first load. The server doesn't find the file and takes longer to answer with a 404. The translated text waits for that answer (forkJoin).

On a first visit to a client-rendered page like /search, this added about 0.5 s on an idle server. On a busy server it added 3.5 s (median).

Video: left is main, right is this PR. The server is handling 8 other page loads at the same time.

Could the theme file be requested only when the theme actually has overrides?

pr-5653-demo.mp4

[kanasznagyzoltan]

Hi @milanmajchrak

I really thank you for the testing.

The build already enumerates the base i18n files and injects the result into the bundle:

new EnvironmentPlugin({
  languageHashes: getFileHashes(join(__dirname, '..', 'src', 'assets', 'i18n'), /.*\.json5/g),
}),

My idea: I'd add the theme-level counterpart next to it — a small map of which themes ship an override file for which language, built from the same kind of filesystem scan:

{ custom: ['en', 'hu'] }

The browser loader would then filter the active theme chain against that map before issuing any request. If nothing in the chain has a file for the current language, it returns the base translations directly, so the behaviour is identical to what it was before this PR: no request, no 404, no added wait. Where a theme does have an override, the request happens exactly as it does now.

If it sounds good, I will have some time over the next few days to implement it, and then it would be ready for another round of testing.

[milanmajchrak]

Hi @milanmajchrak

I really thank you for the testing.

The build already enumerates the base i18n files and injects the result into the bundle:

new EnvironmentPlugin({
  languageHashes: getFileHashes(join(__dirname, '..', 'src', 'assets', 'i18n'), /.*\.json5/g),
}),

My idea: I'd add the theme-level counterpart next to it — a small map of which themes ship an override file for which language, built from the same kind of filesystem scan:

{ custom: ['en', 'hu'] }

The browser loader would then filter the active theme chain against that map before issuing any request. If nothing in the chain has a file for the current language, it returns the base translations directly, so the behaviour is identical to what it was before this PR: no request, no 404, no added wait. Where a theme does have an override, the request happens exactly as it does now.

If it sounds good, I will have some time over the next few days to implement it, and then it would be ready for another round of testing.

Hi @kanasznagyzoltan Thanks, that sounds good. I'm happy to test it again when it's ready.

[kanasznagyzoltan]

@milanmajchrak this is implemented now.

The build collects which themes provide an i18n override file for which language, and the browser loader only requests files that exist. When nothing in the active theme chain overrides the current language, it returns the base translations directly — so there's no request and no 404 at all, and instances that don't use theme translations behave exactly as they did before this PR.

Thanks in advance if you have time to test it again.