alex-poor commented on code in PR #39357:
URL: https://github.com/apache/superset/pull/39357#discussion_r3441050595


##########
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:
   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](https://github.com/apache/superset/blob/master/superset/embedded/view.py#L97)),
 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.



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

Reply via email to