rusackas opened a new pull request, #40379:
URL: https://github.com/apache/superset/pull/40379
### SUMMARY
Follow-up to #40229. That PR fixed a class of bugs where chart-control
labels stayed in the English fallback even after switching language — root
cause was `label: t('Foo')` at module-load time, evaluated before i18n
initialization. The fix in that PR converted call sites to the arrow form
`label: () => t('Foo')` so the lookup happens at render time.
This PR doesn't change runtime behavior — instead it adds **three guard
rails** against future regressions of the same bug class:
1. **ESLint rule `i18n-strings/no-eager-t-in-config`** (autofixable). Flags
`label: t(...)` / `description: t(...)` and rewrites to the arrow form. Wired
as `'warn'` for `**/controlPanel.{ts,tsx,js,jsx}` files so authors can sweep
files with `eslint --fix` as they touch them, without breaking CI on the ~1055
pre-existing call sites. Promote to `'error'` once the codebase is clean.
2. **`check-custom-rules.js` parallel check**. ESLint isn't run in
CI/pre-commit (that path uses `oxlint` + the babel-traverse
`check-custom-rules.js`), so this mirrors the rule there as a warning. Same
shape as the existing `checkI18nTemplates` check.
3. **`t()` / `tn()` dev-mode warning**. When called before `configure()`,
emits a once-per-key console warning that names the key and suggests the
arrow-function fix. Replaces the previous noisy per-call generic warning.
Suppressed when `NODE_ENV === 'production'`.
Also adds a brief note to `BaseControlConfig`'s docstring
(`packages/superset-ui-chart-controls/src/types.ts`) so the lazy-form
convention is documented where authors actually look.
### Why these specific guards (and not others)
I considered a few alternatives in [this
thread](https://github.com/apache/superset/pull/40229) — Proxy returns,
blocking i18n init at bootstrap, tag-template syntax. All have wider blast
radius (break \`typeof === 'string'\`, JSON serialization, Redux DevTools,
etc.) or only fix the initial-load case while leaving runtime language switches
broken. The lazy-function pattern is already what the framework supports
throughout (`ControlPanelsContainer`, `ExploreViewContainer`,
`getControlItemsMap`, etc. all handle `typeof label === 'function'`), so the
cheapest durable fix is to *enforce the pattern* via lint and surface its
absence at dev runtime.
### Why warn-only initially
There are ~1055 pre-existing call sites across ~65 controlPanel files. Most
are safe to mass-autofix (the framework already handles the function form), but
a few custom renderers (e.g., `plugin-chart-handlebars`'s own ControlHeader)
currently fall through to `null` on function-valued labels. So the conservative
path is: ship the rule, sweep files gradually as people touch them, fix
renderer edge cases in parallel, then promote to `'error'`.
### TESTING INSTRUCTIONS
**ESLint plugin rule** (verified locally):
- Drop a fresh `controlPanel.ts` with `const c = { label: t('Foo'),
description: t('Bar') };` into `src/`.
- \`npx eslint <path>\` → 2 warnings: `Eager \`label: t(...)\` is evaluated
at module load... Wrap in an arrow function: \`label: () => t(...)\``.
- \`npx eslint --fix <path>\` → rewrites to `() => t('Foo')` / `() =>
t('Bar')`.
**`check-custom-rules.js`** (verified locally):
- `node scripts/check-custom-rules.js
src/chartCustomizations/components/DynamicGroupBy/controlPanel.ts` → 4 warnings
(the section-level labels + a description still eager in that file).
- `node scripts/check-custom-rules.js
src/explore/components/ControlHeader.tsx` → 0 warnings (non-controlPanel files
are untouched).
**`t()` dev warning** (unit-tested):
- Call `t('apple')` three times before `configure()` → exactly 1 warning
containing `"apple"` and `() => t("apple")`.
- Call with `NODE_ENV=production` → no warning.
- `resetTranslation()` clears the dedupe set.
- See `TranslatorSingleton.test.ts` — 12 tests pass.
### ADDITIONAL INFORMATION
- [ ] Has associated issue: follow-up to #40229
- [ ] Required feature flags:
- [ ] 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: [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]
