You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/10/22 03:30:59 UTC
[PR] fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings [superset]
rusackas opened a new pull request, #45605:
URL: https://github.com/apache/superset/pull/45605
## Description
Adopts #39667 (fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings), by @bestlong.
Flask-Babel locale codes use underscores and mixed case (`zh_TW`, `pt_BR`), but dayjs expects lowercase hyphenated names (`zh-tw`, `pt-br`). `dayjs.locale()` was called with the raw Flask-Babel code, so `zh_TW` silently fell back to English. This adds a `DAYJS_LOCALE_MAPPING` and applies it both in `useLocale` and in the bootstrap `preamble.ts` call.
Rebased onto current `master` and fixed the one thing actually failing CI: the new test imported `renderHook` from `@testing-library/react-hooks`, which isn't a dependency anymore (every other hook test on `master` imports it from `@testing-library/react` directly). Swapped that import; the rest of the original diff is unchanged.
One of CodeAnt's two review comments on the original PR doesn't hold up: it flagged the `preamble.ts` import of `DAYJS_LOCALE_MAPPING` from `./hooks/useLocale` as newly pulling React/Redux/dayjs-locale weight into the bootstrap bundle, but `preamble.ts` already did a bare `import './hooks/useLocale';` before this PR, so that module was already fully loaded on every entrypoint. No change in bundling behavior either way.
Pushing this as a fresh PR rather than to @bestlong's branch directly: the fork has drifted far enough from current `master` to touch `.github/workflows/asf-allowlist-check.yml`, which a GitHub App token without `workflow` scope can't push.
## BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — i18n fix, no visual change beyond correct zh_TW rendering.
## TESTING INSTRUCTIONS
1. Set `DEFAULT_LOCALE = 'zh_TW'` in `superset_config.py` (or switch profile language to 繁體中文).
2. Reload — Ant Design components (date pickers, pagination) render in Traditional Chinese, and dayjs date formatting follows zh-TW conventions.
3. `cd superset-frontend && npx jest src/hooks/useLocale.test.tsx` — 16/16 passing.
## ADDITIONAL INFORMATION
- [x] Has associated issue: related to i18n locale mapping
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #45605:
URL: https://github.com/apache/superset/pull/45605#discussion_r4354691083
##########
superset-frontend/src/preamble.test.ts:
##########
@@ -72,7 +72,9 @@ jest.mock('./utils/getBootstrapData', () => ({
jest.mock('./utils/pathUtils', () => ({
makeUrl: mockMakeUrl,
}));
-jest.mock('./hooks/useLocale', () => ({}));
+jest.mock('./hooks/useLocale', () => ({
Review Comment:
The `zh_TW` → `zh-tw` mapping is stubbed here but nothing in this suite bootstraps `zh_TW`: the window-pack test only asserts `'fr'` and the fetch-success test has no dayjs assertion. If either `dayjs.locale(DAYJS_LOCALE_MAPPING[lang] ?? lang)` call in `preamble.ts` were reverted to `dayjs.locale(lang)`, every test would still pass and `zh_TW` would silently fall back to English again on the bootstrap path. Could you add a case for each path (preloaded pack and successful fetch) with a `zh_TW` locale that asserts `mockDayjsLocale` was called with `'zh-tw'`?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #45605:
URL: https://github.com/apache/superset/pull/45605#issuecomment-6332101849
## CodeAnt PR Risk: Medium Risk
- The PR needs attention before merging.
- The language-pack fetch no longer uses an operator-supplied or preloaded pack, so custom translations can be replaced or lost when the request fails.
**Assessed commit:** `aea85b103fdc`
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #45605:
URL: https://github.com/apache/superset/pull/45605#discussion_r4353771521
##########
superset-frontend/src/preamble.ts:
##########
@@ -64,65 +64,40 @@ export default function initPreamble(): Promise<void> {
// Setup SupersetClient early so we can fetch language pack
setupClient({ appRoot: applicationRoot() });
- // Initialize feature flags before the first await below. Not every
- // entry point awaits initPreamble() before importing plugins (see
- // webpack.config.js's addPreamble()), and plugin modules can call
- // isFeatureEnabled() during import, which would see stale/missing
- // flags if this ran after the language-pack fetch instead. See #37310.
- initFeatureFlags(bootstrapData.common.feature_flags);
-
- // Load language pack before rendering.
- // Prefer the pack already on hand: either an operator-supplied
- // common.language_pack (COMMON_BOOTSTRAP_OVERRIDES_FUNC) or the
- // window global set by the versioned script tag spa.html loads
- // before this bundle. Either way, module-level `const X = t('...')`
- // calls in code-split chunks all see a configured translator. Fall
- // back to the async fetch only when neither is present (e.g. the
- // script tag failed to load, or a legacy entry that doesn't extend
- // spa.html). See issue #35330.
+ // Load language pack before rendering
+ // Use native fetch to avoid race condition with SupersetClient initialization
if (lang !== 'en') {
- 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);
- } else {
- const abortController = new AbortController();
- const timeoutId = window.setTimeout(() => {
- abortController.abort();
- }, LANGUAGE_PACK_REQUEST_TIMEOUT_MS);
-
- try {
- const languagePackUrl = makeUrl(`/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);
+ 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 });
Review Comment:
Good catch. The branch had reverted the bootstrap/window pack path in `preamble.ts`; I restored it and only the `dayjs` locale mapping differs now.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #45605:
URL: https://github.com/apache/superset/pull/45605#issuecomment-6352072591
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45605?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 84.12%. Comparing base ([`87d1abb`](https://app.codecov.io/gh/apache/superset/commit/87d1abb21e6e1610b4aa42b69d50a960a427fe69?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`27c565e`](https://app.codecov.io/gh/apache/superset/commit/27c565e01fa866fba865506527a5255b3e6cf768?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #45605 +/- ##
=======================================
Coverage 84.12% 84.12%
=======================================
Files 3056 3056
Lines 201418 201421 +3
Branches 47095 47098 +3
=======================================
+ Hits 169434 169447 +13
+ Misses 28736 28726 -10
Partials 3248 3248
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/45605/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/45605/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `80.19% <100.00%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45605?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #45605:
URL: https://github.com/apache/superset/pull/45605#issuecomment-6354768483
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 16fb77b7b97051ab202fb7ef45d090c8c193bc55 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6adab026beafeef6808c0745 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-45605--superset-docs-preview.netlify.app](https://deploy-preview-45605--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#q1YqKMrPLShRslJS0lFKKkrMS84AstMyK_QzDS3ydKsydEvKdXPykxNzUoEKUlILcvIrPVOASswSUxKTDIzMklIT01JT08wsDCySDcxNTJVqAQ) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-45605--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45605:
URL: https://github.com/apache/superset/pull/45605#discussion_r4344313984
##########
superset-frontend/src/preamble.ts:
##########
@@ -64,65 +64,40 @@ export default function initPreamble(): Promise<void> {
// Setup SupersetClient early so we can fetch language pack
setupClient({ appRoot: applicationRoot() });
- // Initialize feature flags before the first await below. Not every
- // entry point awaits initPreamble() before importing plugins (see
- // webpack.config.js's addPreamble()), and plugin modules can call
- // isFeatureEnabled() during import, which would see stale/missing
- // flags if this ran after the language-pack fetch instead. See #37310.
- initFeatureFlags(bootstrapData.common.feature_flags);
-
- // Load language pack before rendering.
- // Prefer the pack already on hand: either an operator-supplied
- // common.language_pack (COMMON_BOOTSTRAP_OVERRIDES_FUNC) or the
- // window global set by the versioned script tag spa.html loads
- // before this bundle. Either way, module-level `const X = t('...')`
- // calls in code-split chunks all see a configured translator. Fall
- // back to the async fetch only when neither is present (e.g. the
- // script tag failed to load, or a legacy entry that doesn't extend
- // spa.html). See issue #35330.
+ // Load language pack before rendering
+ // Use native fetch to avoid race condition with SupersetClient initialization
if (lang !== 'en') {
- 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);
- } else {
- const abortController = new AbortController();
- const timeoutId = window.setTimeout(() => {
- abortController.abort();
- }, LANGUAGE_PACK_REQUEST_TIMEOUT_MS);
-
- try {
- const languagePackUrl = makeUrl(`/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);
+ 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 });
Review Comment:
**Suggestion:** This replaces a server-supplied operator language pack with the fetched catalog; if the request fails, the catch also resets translations to English.
**Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely` · 🏷️ `Logic error`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a7274ca9224b40249271079e445d639a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a7274ca9224b40249271079e445d639a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/preamble.ts
**Line:** 84:84
**Comment:**
*Logic Error: This replaces a server-supplied operator language pack with the fetched catalog; if the request fails, the catch also resets translations to English.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45605&comment_hash=9e7cfd376b7588ad030f72d7228f96db500bbc59bddc7242a15dcda70b88b28a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45605&comment_hash=9e7cfd376b7588ad030f72d7228f96db500bbc59bddc7242a15dcda70b88b28a&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #45605:
URL: https://github.com/apache/superset/pull/45605
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(i18n): Add support for Traditional Chinese locale zh_TW in dayjs and update locale mappings [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #45605:
URL: https://github.com/apache/superset/pull/45605#discussion_r4355066012
##########
superset-frontend/src/preamble.test.ts:
##########
@@ -72,7 +72,9 @@ jest.mock('./utils/getBootstrapData', () => ({
jest.mock('./utils/pathUtils', () => ({
makeUrl: mockMakeUrl,
}));
-jest.mock('./hooks/useLocale', () => ({}));
+jest.mock('./hooks/useLocale', () => ({
Review Comment:
Added both cases in 16fb77b: preloaded pack and successful fetch, each with `zh_TW` asserting `mockDayjsLocale` gets `zh-tw`.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org