fix(i18n): inject language pack into bootstrap to translate code-split chunks#39357
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #39357 +/- ##
==========================================
+ Coverage 64.70% 64.71% +0.01%
==========================================
Files 2686 2686
Lines 148625 148651 +26
Branches 34298 34309 +11
==========================================
+ Hits 96164 96206 +42
+ Misses 50696 50680 -16
Partials 1765 1765
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
🎪 ⌛ 48h
|
🎪 Showtime deployed environment on GHA for 254ec95 • Environment: http://34.215.177.143:8080 (admin/admin) |
![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.
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 |
|
Everything from my pass is addressed, so LGTM. It's gone conflicting with master since, though. After another rebase, hopefully we can merge. |
rusackas
added
the
merge-if-green
Resolves the preamble.ts conflict: master renamed the language pack endpoint from /superset/language_pack/<lang>/ to /language_pack/<lang>/ (route move); keep the bootstrap-pack-first structure from this branch and adopt the new URL in the async-fetch fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Rebased (merged current master) to clear the conflict... master moved the language pack endpoint to I also dug into the payload question since inlining the pack isn't free. Measured with the French catalog: ~237KB of Jed JSON, which is ~67KB gzipped inside the HTML. That cost only applies to non-English locales (en injects null, so default deployments pay nothing), and only on full page loads, which are rare in an SPA. On a cold cache this is actually faster than master: the fetch approach serializes HTML → JS bundle → language pack round trip before first render, and the inline pack removes that whole hop. On warm loads it's ~67KB of extra HTML where the fetch would have been free from browser cache. One more point in its favor: LGTM once CI comes back green. 🤞 |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
TranslatorSingleton.ts declares the typed Window.__SUPERSET_LANGUAGE_PACK__ global, so the test file can use the property directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Code Review Agent Run #17330cActionable 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 |
…ripts Follow-up to #39357, which inlined the Jed language pack into the bootstrap HTML to fix the code-split translation race (#35330). That made non-English full-page loads carry ~67KB (gzipped) of pack on every request since HTML is uncacheable. This keeps the synchronous-configuration guarantee but moves the pack to a content-addressed script: spa.html emits <script src="/language_pack/<lang>/<version>/script.js"> before the entry bundle, where <version> is a short content hash of the pack file. The endpoint serves window.__SUPERSET_LANGUAGE_PACK__ = {...} with Cache-Control: public, max-age=31536000, immutable, so browsers fetch the pack once per translation change instead of once per page load, and cache-bust automatically on upgrade (fixing the stale-pack window the unversioned /language_pack/<lang>/ fetch had under the 1-year SEND_FILE_MAX_AGE_DEFAULT). Details: - English emits no tag and pays nothing. - A stale version in a cached HTML page still gets current content, served no-cache so it can't pin under the wrong address. - Packs provided via COMMON_BOOTSTRAP_OVERRIDES_FUNC still ride the bootstrap payload and suppress the script tag (spa.html stashes them on window instead), preserving the historical workaround. - The endpoint is deliberately unauthenticated: catalogs are static public repo content with no user data, and must load for anonymous principals (login page, embedded). - preamble.ts fallback chain unchanged: bootstrap pack, window global, then async fetch if the script failed to load. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bito Automatic Review Skipped – PR Already Merged |
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