rusackas commented on code in PR #39357:
URL: https://github.com/apache/superset/pull/39357#discussion_r3440493444
##########
superset/templates/superset/spa.html:
##########
@@ -145,6 +145,29 @@
{% endblock %}
{% block tail_js %}
+ {#
+ Expose the language pack on window BEFORE the entry bundle loads,
+ so module-level `const X = t('...')` calls across webpack-split
+ chunks all find a configured translator on first access. The
+ bootstrap div is the single source of truth; we parse it once and
+ stash the pack on a global that the translator reads lazily.
+ See upstream issue #35330.
+ #}
+ <script nonce="{{ macros.get_nonce() }}">
Review Comment:
One trade-off to call out: the full Jed pack now ships inline in every
full-page HTML response (and HTML-attribute-escaped inside `data-bootstrap`, so
a touch bigger), instead of the separately-cacheable `/language_pack/` fetch.
Probably fine for an SPA where hard reloads are rare, but worth being
deliberate about it.
##########
superset-frontend/src/preamble.ts:
##########
@@ -64,34 +64,53 @@ export default function initPreamble(): Promise<void> {
// Setup SupersetClient early so we can fetch language pack
setupClient({ appRoot: applicationRoot() });
- // Load language pack before rendering
- // Use native fetch to avoid race condition with SupersetClient
initialization
+ // Load language pack before rendering.
+ // Prefer the bootstrap-injected pack (stashed on window by the inline
+ // script in spa.html, sourced from common.language_pack) so
+ // module-level `const X = t('...')` calls in code-split chunks all
+ // see a configured translator. Fall back to the async fetch only
+ // when the bootstrap payload didn't carry the pack (e.g. embedded
+ // or a legacy entry that doesn't extend spa.html). See issue #35330.
if (lang !== 'en') {
- const abortController = new AbortController();
- const timeoutId = window.setTimeout(() => {
- abortController.abort();
- }, LANGUAGE_PACK_REQUEST_TIMEOUT_MS);
-
- try {
- const languagePackUrl = makeUrl(`/superset/language_pack/${lang}/`);
- const resp = await fetch(languagePackUrl, {
- signal: abortController.signal,
- });
- if (!resp.ok) {
- throw new Error(`Failed to fetch language pack: ${resp.status}`);
- }
- const json = await resp.json();
- configure({ languagePack: json as LanguagePack });
+ const bootstrapPack =
+ (bootstrapData.common as { language_pack?: LanguagePack })
+ .language_pack ??
+ (typeof window !== 'undefined'
+ ? window.__SUPERSET_LANGUAGE_PACK__
+ : undefined);
+ if (bootstrapPack) {
+ configure({ languagePack: bootstrapPack });
dayjs.locale(lang);
- } catch (err) {
- logging.warn(
- 'Failed to fetch language pack, falling back to default.',
- err,
- );
- configure();
- dayjs.locale('en');
- } finally {
- window.clearTimeout(timeoutId);
+ } else {
Review 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.
##########
superset/views/base.py:
##########
@@ -550,7 +551,22 @@ def common_bootstrap_payload() -> dict[str, Any]:
locale = get_locale()
# Convert locale to string for proper cache key hashing
locale_str = str(locale) if locale else None
- return cached_common_bootstrap_data(utils.get_user_id(), locale_str)
+ payload = dict(cached_common_bootstrap_data(utils.get_user_id(),
locale_str))
+ # Inject the Jed language pack outside the per-user memoize so the cached
+ # payload stays small and the pack is shared across users for the same
+ # locale. The frontend uses it to configure the translator synchronously,
+ # before any code-split chunk evaluates a module-level `const X = t('...')`
+ # (upstream issue #35330).
+ language = payload.get("locale")
+ if language and language != "en":
+ # `get_language_pack` falls back to an empty English pack on miss;
+ # treat empty as "no pack" so the frontend can fall through to the
+ # async fetch instead of configuring with an empty translator.
+ pack = get_language_pack(language) or None
+ else:
+ pack = None
+ payload["language_pack"] = pack
Review Comment:
Two small things here. `get_language_pack` returns the empty English pack on
a miss (never falsy), so the `or None` never actually fires and the comment's
"fall through to the async fetch" doesn't happen. And since
`COMMON_BOOTSTRAP_OVERRIDES_FUNC` can set `language_pack` itself (the
workaround in #35330 does exactly that), this clobbers it. Both fixable by
preferring an existing pack and dropping the dead `or None`:
```suggestion
language = payload.get("locale")
if language and language != "en":
# Respect a pack already provided via
COMMON_BOOTSTRAP_OVERRIDES_FUNC,
# otherwise load the shared one. get_language_pack returns the empty
# English pack on a miss, which is the right result (English) when no
# translation file exists.
pack = payload.get("language_pack") or get_language_pack(language)
else:
pack = None
payload["language_pack"] = pack
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
