fix(i18n): inject language pack into bootstrap to translate code-split chunks#39357
fix(i18n): inject language pack into bootstrap to translate code-split chunks#39357alex-poor wants to merge 5 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #39357 +/- ##
==========================================
+ Coverage 64.36% 64.37% +0.01%
==========================================
Files 2651 2651
Lines 144812 144838 +26
Branches 33417 33428 +11
==========================================
+ Hits 93208 93245 +37
+ Misses 49935 49922 -13
- Partials 1669 1671 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
alex-poor
force-pushed
the
fix/i18n-bootstrap-injection
branch
4 times, most recently
from
d3a4c45 to
254ec95
Compare
dosubot
Bot
added
change:frontend
Code Review Agent Run #e94e97Actionable Suggestions - 0Additional Suggestions - 1
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
sadpandajoe
added
🎪 ⚡ showtime-trigger
github-actions
Bot
added
🎪 254ec95 🚦 building
github-actions
Bot
added
🎪 254ec95 🚦 deploying
|
🎪 Showtime deployed environment on GHA for 254ec95 • Environment: http://34.215.177.143:8080 (admin/admin) |
github-actions
Bot
removed
🎪 ⚡ showtime-trigger-stop
🎪 254ec95 🚦 running
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR addresses an i18n race where module-scope t('...') calls in lazily loaded/code-split chunks can evaluate before the async language-pack fetch completes, causing some strings to remain in English.
Changes:
- Adds server-side loading of Jed
messages.jsoninto the bootstrap payload ascommon.language_pack. - Exposes the pack early via an inline script in
spa.html(window.__SUPERSET_LANGUAGE_PACK__) before the entry bundle loads. - Updates the frontend translator singleton to self-configure from the global pack and updates
preamble.tsto prefer the injected pack with async fetch as fallback.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit_tests/views/test_bootstrap_auth.py | Adds unit coverage for _load_language_pack and asserts language pack presence in bootstrap payload |
| superset/views/base.py | Loads messages.json from disk and injects it into the common bootstrap data |
| superset/templates/superset/spa.html | Adds inline script to stash common.language_pack onto window.__SUPERSET_LANGUAGE_PACK__ before bundle load |
| superset-frontend/src/preamble.ts | Prefers bootstrap/global-injected language pack; keeps async fetch as fallback |
| superset-frontend/packages/superset-core/src/translation/TranslatorSingleton.ts | Auto-configures translator from window.__SUPERSET_LANGUAGE_PACK__ on first access to survive chunk duplication |
| superset-frontend/packages/superset-core/src/translation/TranslatorSingleton.test.ts | Adds tests covering the new window-based auto-configuration behavior |
Code Review Agent Run #0125ecActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
hi @michael-s-molina @mistercrunch ! I have resolved the copilot review issues and will be grateful for any thoughts or advice on this PR. Thank you. |
…t chunks
Module-level `const X = t('...')` calls in code-split chunks evaluate
before the async `/superset/language_pack/<lang>/` fetch in preamble.ts
can resolve, capturing the English msgid forever. apache#36893 fixed the
initial-render race by awaiting initPreamble before ReactDOM.render,
but chunks like DashboardContainer (ChartContextMenu ->
useDrillDetailMenuItems) still evaluate later and hit an unconfigured
translator. Result: strings inside component render bodies translate,
but strings captured at module scope stay English, even though
bootstrapData.common.locale is set and the language pack endpoint
returns a valid pack. Reported for pt_BR, ru, zh and others in apache#35330.
Fix: inject the full Jed language pack into the HTML bootstrap payload
under `common.language_pack`. An inline <script> in spa.html — which
runs before the entry bundle loads — stashes the pack on
`window.__SUPERSET_LANGUAGE_PACK__`. TranslatorSingleton lazily
self-configures from that global on first `getInstance()`, so every
chunk-local copy webpack splitChunks may produce ends up configured
identically. preamble.ts now prefers the bootstrap-injected pack and
keeps the async fetch as a fallback for entries that don't extend
spa.html (e.g. embedded).
Net effect: `t()` returns the translated msgstr on first evaluation
everywhere, including module-level constants in lazy chunks.
Fixes apache#35330
…emoize Address review feedback on the bootstrap language-pack injection: * Drop the duplicate `_load_language_pack` reader in views/base.py and reuse `superset.translations.utils.get_language_pack`, which already has a process-level dict cache keyed by locale and is the existing loader used by the async `/superset/language_pack/<lang>/` endpoint. * Move the pack injection out of `cached_common_bootstrap_data` (which is memoized per `(user_id, locale)`) and into `common_bootstrap_payload` so the cached entry stays small and the pack itself is shared across users for the same locale instead of being duplicated in cache. * Treat an empty/falsy pack as "no pack" so locales that miss the JSON file (the shared utility falls back to an empty English pack) hand the frontend `null` and the existing async-fetch fallback fires, matching prior behavior. * Replace the `_load_language_pack` unit tests (function removed) with tests against `common_bootstrap_payload`. The new tests don't depend on the per-user memoize, removing a flakiness path where a prior test populating `(user_id=1, locale=None)` would skip the under-test code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…payload `common_bootstrap_payload` now copies the cached dict and adds a `language_pack` key so the pack isn't duplicated inside the per-user memoize. Update the existing locale-stringification test to assert on the relevant fields rather than the full dict, since the wrapper intentionally returns a superset of the cached payload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rusackas
force-pushed
the
fix/i18n-bootstrap-injection
branch
from
23581c0 to
e553319
Compare
rusackas
left a comment
rusackas
left a comment
There was a problem hiding this comment.
Rebased this onto current master and pushed for you, there were real conflicts to work through: master moved the pre-configure warning into warnPreConfigure (so I had to wire autoConfigureFromWindow() into t()/tn() ahead of the warn check, otherwise it warns even when the window pack is present), and a _get_frontend_config_value helper landed right next to your new code. Translation + bootstrap tests pass locally.
The approach itself looks good to me, synchronous availability before any chunk evaluates is the right fix. A few small inline notes below, none of them blockers.
| dayjs.locale('en'); | ||
| } finally { | ||
| window.clearTimeout(timeoutId); | ||
| } else { |
There was a problem hiding this comment.
Worth flagging as a known limitation: embedded doesn't extend spa.html, so it never gets the injected pack and lands here on the async fetch, which still has the same code-split chunk race this PR is fixing. So embedded dashboards would stay partially translated. Fine to leave for a follow-up, just might be worth a note so it's not assumed covered.
There was a problem hiding this comment.
As far as I can tell embedded is actually covered (I think!).
EmbeddedView.embeddedrenderssuperset/spa.htmlwithentry="embedded"and callscommon_bootstrap_payload()(embedded/view.py#L97-L109), so the injectedcommon.language_packand the inline<script>that setswindow.__SUPERSET_LANGUAGE_PACK__(beforejs_bundle(entry)) both apply.- The embedded webpack entry is
addPreamble('src/embedded/index.tsx'), which prependspreamble.ts— so the preferred injected-pack path here runs for embedded too, andTranslatorSingleton.autoConfigureFromWindow()self-configures from the global on the firstt()/tn()regardless. spa.htmlis the only template carryingdata-bootstrap/js_bundle(entry), so every entry goes through this injection.
whether an embedded load is actually translated still depends on the resolved locale (?lang=/session/Accept-Language) being non-English — if it resolves to en, the pack is null and there's nothing to inject, same as the main app.
Let me know if you're seeing it hit the async fetch path in practice, but as far as I can trace it embedded shouldn't.
get_language_pack returns the empty English Jed pack on a miss (never falsy), so the trailing 'or None' was dead code. It also clobbered a language_pack already set via COMMON_BOOTSTRAP_OVERRIDES_FUNC (the documented apache#35330 workaround). Prefer an existing pack, then fall back to the shared loader. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review Agent Run #75addeActionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
SUMMARY
Fixes #35330. Module-level
const X = t('...')calls in code-split chunks evaluate before the async/superset/language_pack/<lang>/fetch inpreamble.tscan resolve, capturing the English msgid forever. #36893 fixed the initial-render race by awaitinginitPreamble()beforeReactDOM.render, but lazily-loaded chunks (e.g.DashboardContainer→ChartContextMenu→useDrillDetailMenuItems) still evaluate later and hit an unconfigured translator. Result: strings inside component render bodies translate, but strings captured in module-scope constants stay English, even thoughbootstrapData.common.localeis set and the language pack endpoint returns a valid pack. Same symptom reported on 5.0.0 / 6.0.0 for pt_BR, ru, zh in #35330 and linked issues.Fix: inject the full Jed language pack into the HTML bootstrap payload under
common.language_pack. An inline<script>inspa.html— which runs before the entry bundle loads — stashes the pack onwindow.__SUPERSET_LANGUAGE_PACK__.TranslatorSingletonlazily self-configures from that global on firstgetInstance(), so every chunk-local copy that webpacksplitChunksmay produce ends up configured identically.preamble.tsnow prefers the bootstrap-injected pack and keeps the async fetch as fallback for entries that don't extendspa.html(embedded, legacy).Net effect:
t()returns the translated msgstr on first evaluation everywhere, including module-level constants in lazy chunks.Files changed:
superset/views/base.py— readtranslations/<lang>/LC_MESSAGES/messages.jsonintocommon.language_pack(skipped foren, silently absent if the file isn't present, respects the existingcached_common_bootstrap_datamemoization).superset/templates/superset/spa.html— inline script that parses the bootstrap div and setswindow.__SUPERSET_LANGUAGE_PACK__beforejs_bundle(entry).superset-frontend/packages/superset-core/src/translation/TranslatorSingleton.ts—autoConfigureFromWindow()called fromgetInstance()to self-configure from the global.superset-frontend/src/preamble.ts— prefer the injected pack; keep async fetch as fallback.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Tested on a live deployment with
mi(Māori) locale. Before: navbar translated, chart context menu items ("Drill to detail", etc.) and many other module-scope strings stayed English. After: every string with a msgstr renders translated on first load.TESTING INSTRUCTIONS
BUILD_TRANSLATIONS=truesomessages.jsonfiles exist on disk.pt_BR,ru,mi).JSON.parse(document.getElementById('app').dataset.bootstrap).common.language_packshould return the Jed dict, andwindow.__SUPERSET_LANGUAGE_PACK__should be the same object.language_packshould benulland the async fetch path should not run.ADDITIONAL INFORMATION