Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,82 @@ test('resetTranslation does nothing when not yet configured', () => {
consoleSpy.mockRestore();
});
});

// --- autoConfigureFromWindow ----------------------------------------------
// These cover the bootstrap-injection path used to dodge the
// module-level `const X = t(...)` race across code-split chunks
// (upstream issue #35330).

test('t() self-configures from window.__SUPERSET_LANGUAGE_PACK__ on first call', () => {
jest.isolateModules(() => {
(window as any).__SUPERSET_LANGUAGE_PACK__ = {
domain: 'superset',
locale_data: {
superset: {
'': {
domain: 'superset',
lang: 'fr',
plural_forms: 'nplurals=2; plural=(n > 1);',
},
hello: ['bonjour'],
},
},
};
const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
const { t } = require('./TranslatorSingleton');
expect(t('hello')).toBe('bonjour');
// No "should call configure" warning because we self-configured first.
expect(consoleSpy).not.toHaveBeenCalled();
consoleSpy.mockRestore();
delete (window as any).__SUPERSET_LANGUAGE_PACK__;
});
});

test('t() falls back to msgid when window has no language pack', () => {
jest.isolateModules(() => {
delete (window as any).__SUPERSET_LANGUAGE_PACK__;
const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
const { t } = require('./TranslatorSingleton');
expect(t('hello')).toBe('hello');
expect(consoleSpy).toHaveBeenCalledWith(
expect.stringMatching(/was called before configure\(\)/),
);
consoleSpy.mockRestore();
});
});

test('explicit configure() takes precedence over window pack', () => {
jest.isolateModules(() => {
(window as any).__SUPERSET_LANGUAGE_PACK__ = {
domain: 'superset',
locale_data: {
superset: {
'': {
domain: 'superset',
lang: 'fr',
plural_forms: 'nplurals=2; plural=(n > 1);',
},
hello: ['bonjour'],
},
},
};
const { configure, t } = require('./TranslatorSingleton');
configure({
languagePack: {
domain: 'superset',
locale_data: {
superset: {
'': {
domain: 'superset',
lang: 'es',
plural_forms: 'nplurals=2; plural=(n != 1);',
},
hello: ['hola'],
},
},
},
});
expect(t('hello')).toBe('hola');
delete (window as any).__SUPERSET_LANGUAGE_PACK__;
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,31 @@ function configure(config?: TranslatorConfig) {
return singleton;
}

// When webpack splits @apache-superset/core across chunks, each
// chunk-local copy of this module has its own `singleton` and
// `isConfigured` state. The first `t()` call in a late chunk hits a
// fresh, unconfigured Translator and returns the English msgid. To
// make translations survive chunk duplication, the HTML template
// stashes the language pack on window and we self-configure from it
// on first access. See upstream issue #35330.
declare global {
interface Window {
__SUPERSET_LANGUAGE_PACK__?: TranslatorConfig['languagePack'];
}
}

function autoConfigureFromWindow() {
if (isConfigured) return;
if (typeof window === 'undefined') return;
const pack = window.__SUPERSET_LANGUAGE_PACK__;
if (pack) {
configure({ languagePack: pack });
}
}

function getInstance() {
autoConfigureFromWindow();

if (typeof singleton === 'undefined') {
singleton = new Translator();
}
Expand Down Expand Up @@ -85,11 +109,16 @@ function addLocaleData(data: LocaleData) {
}

function t(input: string, ...args: unknown[]) {
// Self-configure from the bootstrap-injected window pack before deciding
// whether to warn, so a chunk-local copy that hasn't seen configure() yet
// doesn't warn (or fall back to English) when the pack is available.
autoConfigureFromWindow();
if (!isConfigured) warnPreConfigure('t', input);
return getInstance().translate(input, ...args);
}

function tn(key: string, ...args: unknown[]) {
autoConfigureFromWindow();
if (!isConfigured) warnPreConfigure('tn', key);
return getInstance().translateWithNumber(key, ...args);
}
Expand Down
71 changes: 45 additions & 26 deletions superset-frontend/src/preamble.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell embedded is actually covered (I think!).

  • EmbeddedView.embedded renders superset/spa.html with entry="embedded" and calls common_bootstrap_payload() (embedded/view.py#L97-L109), so the injected common.language_pack and the inline <script> that sets window.__SUPERSET_LANGUAGE_PACK__ (before js_bundle(entry)) both apply.
  • The embedded webpack entry is addPreamble('src/embedded/index.tsx'), which prepends preamble.ts — so the preferred injected-pack path here runs for embedded too, and TranslatorSingleton.autoConfigureFromWindow() self-configures from the global on the first t()/tn() regardless.
  • spa.html is the only template carrying data-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.

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 });
if (typeof window !== 'undefined') {
window.__SUPERSET_LANGUAGE_PACK__ = json as LanguagePack;
}
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);
}
}
}

Expand Down
30 changes: 30 additions & 0 deletions superset/templates/superset/spa.html
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,36 @@
{% 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.

Trade-off: this ships the full Jed pack inline in every full-page
HTML response (only for non-English locales; English injects null)
rather than via the separately-cacheable `/language_pack/` fetch.
That synchronous availability is the whole point — the async fetch
can't guarantee the pack is configured before a code-split chunk
evaluates. Acceptable for an SPA where hard reloads are rare.
#}
<script nonce="{{ macros.get_nonce() }}">
Comment thread
alex-poor marked this conversation as resolved.
(function () {
try {
var el = document.getElementById('app');
if (!el) return;
var data = JSON.parse(el.getAttribute('data-bootstrap') || '{}');
var pack = data && data.common && data.common.language_pack;
if (pack) {
window.__SUPERSET_LANGUAGE_PACK__ = pack;
}
} catch (e) {
/* swallow: fall back to async fetch in preamble.ts */
}
})();
</script>
{% if entry %}
{{ js_bundle(assets_prefix, entry) }}
{% endif %}
Expand Down
20 changes: 19 additions & 1 deletion superset/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
from superset.themes.utils import (
is_valid_theme,
)
from superset.translations.utils import get_language_pack
from superset.utils import core as utils, json
from superset.utils.filters import get_dataset_access_filters
from superset.utils.version import get_version_metadata
Expand Down Expand Up @@ -550,7 +551,24 @@ 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":
# Respect a pack already provided via COMMON_BOOTSTRAP_OVERRIDES_FUNC
# (the workaround in #35330 does exactly that), 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
Comment thread
alex-poor marked this conversation as resolved.
return payload


def get_spa_payload(extra_data: dict[str, Any] | None = None) -> dict[str, Any]:
Expand Down
5 changes: 4 additions & 1 deletion tests/unit_tests/views/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ def __str__(self) -> str:

# Verify cached_common_bootstrap_data was called with string locale
mock_cached.assert_called_once_with(1, "de_DE")
assert result == {"test": "data"}
# The wrapper copies the cached dict and injects `language_pack` after
# the memoized call so the per-locale pack isn't duplicated per user.
assert result["test"] == "data"
assert "language_pack" in result


@patch("superset.views.base.utils.get_user_id", return_value=1)
Expand Down
72 changes: 71 additions & 1 deletion tests/unit_tests/views/test_bootstrap_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
AUTH_SAML,
)

from superset.views.base import cached_common_bootstrap_data
from superset.views.base import cached_common_bootstrap_data, common_bootstrap_payload


@pytest.fixture(autouse=True)
Expand Down Expand Up @@ -133,3 +133,73 @@ def test_recaptcha_shown_for_non_federated_auth(
payload = _get_bootstrap()

assert payload["conf"]["RECAPTCHA_PUBLIC_KEY"] == "test-key"


# --- language_pack injection --------------------------------------------
#
# The Jed pack is injected by `common_bootstrap_payload` (outside the
# memoized `cached_common_bootstrap_data`) using the shared
# `superset.translations.utils.get_language_pack`. Tests here cover the
# wrapper to confirm the pack lands on the payload for non-English
# locales and is None for English.


def test_common_bootstrap_payload_includes_language_pack_for_non_english(
app_context: None,
) -> None:
"""common.language_pack carries the shared utility's pack for non-en."""
fake_pack = {"domain": "superset", "locale_data": {"superset": {}}}
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value={"locale": "fr"},
),
patch(
"superset.views.base.get_language_pack",
return_value=fake_pack,
) as mock_get,
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="fr"),
):
payload = common_bootstrap_payload()

assert payload["language_pack"] == fake_pack
mock_get.assert_called_once_with("fr")


def test_common_bootstrap_payload_skips_pack_for_english(
app_context: None,
) -> None:
"""English short-circuits: pack is None and the utility is not called."""
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value={"locale": "en"},
),
patch("superset.views.base.get_language_pack") as mock_get,
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="en"),
):
payload = common_bootstrap_payload()

assert payload["language_pack"] is None
mock_get.assert_not_called()


def test_common_bootstrap_payload_does_not_mutate_memoized_dict(
app_context: None,
) -> None:
"""Injecting language_pack must not write back into the memoize cache."""
cached: dict[str, Any] = {"locale": "fr"}
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value=cached,
),
patch("superset.views.base.get_language_pack", return_value={"x": 1}),
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="fr"),
):
common_bootstrap_payload()

assert "language_pack" not in cached
Loading