Copilot commented on code in PR #40379:
URL: https://github.com/apache/superset/pull/40379#discussion_r3290518094
##########
superset-frontend/eslint-rules/eslint-plugin-i18n-strings/index.ts:
##########
@@ -65,6 +65,86 @@ const plugin: { rules: Record<string, Rule.RuleModule> } = {
};
},
},
+ 'no-eager-t-in-config': {
+ meta: {
+ type: 'problem',
+ fixable: 'code',
+ docs: {
+ description:
+ 'Disallow eager t()/tn() calls for `label` and `description` in
config objects evaluated at module load (e.g., controlPanel files). The
translation is captured at module-evaluation time, before i18n has loaded, and
never updates when the user switches language. Wrap the call in an arrow
function so it is evaluated at render time.',
+ },
+ schema: [
+ {
+ type: 'object',
+ properties: {
+ properties: {
+ type: 'array',
+ items: { type: 'string' },
+ },
+ },
+ additionalProperties: false,
+ },
+ ],
+ messages: {
+ eager:
+ 'Eager `{{property}}: t(...)` is evaluated at module load, before
i18n is initialized. Wrap in an arrow function: `{{property}}: () => t(...)`.',
+ },
Review Comment:
The rule reports on both t() and tn() calls (TRANSLATE_FNS includes both),
but the `eager` message hard-codes `t(...)` in the example/fix text. This makes
warnings confusing for `tn()` cases; consider wording it as `t()/tn()` and/or
interpolating the actual callee name in the message.
##########
superset-frontend/packages/superset-core/src/translation/TranslatorSingleton.ts:
##########
@@ -33,22 +37,39 @@ function configure(config?: TranslatorConfig) {
}
function getInstance() {
- if (!isConfigured) {
- console.warn('You should call configure(...) before calling other
methods');
- }
-
if (typeof singleton === 'undefined') {
singleton = new Translator();
}
return singleton;
}
+function warnPreConfigure(key: string) {
+ // Only warn in non-production builds — production callers may legitimately
+ // tolerate the fallback, and the noise isn't useful at runtime.
+ if (
+ typeof process !== 'undefined' &&
+ process.env?.NODE_ENV === 'production'
+ ) {
+ return;
+ }
+ if (warnedPreConfigureKeys.has(key)) return;
+ warnedPreConfigureKeys.add(key);
+ console.warn(
+ `[i18n] t(${JSON.stringify(key)}) was called before configure() — ` +
+ `the result is the fallback language and will not update when the ` +
+ `user switches language. If this call is at module load (e.g., a ` +
+ `controlPanel \`label\`/\`description\`), wrap it in an arrow ` +
+ `function: \`() => t(${JSON.stringify(key)})\`.`,
+ );
Review Comment:
warnPreConfigure() is reused by both t() and tn(), but the warning message
always formats the call as `t(<key>)` and suggests `() => t(<key>)`. When tn()
triggers this path, the log becomes misleading. Consider passing the function
name into warnPreConfigure (e.g., `warnPreConfigure('tn', key)`) and tailoring
the message/suggested fix accordingly.
--
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]
