From 3c6da3f24143545990e47fe8e829bbb7d3aea1be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Thu, 12 Jun 2025 13:30:54 +0200 Subject: [PATCH] i18n: removes useTranslate from ESLint rules (#106597) * i18n: removes useTranslate from eslint rules * chore: remove some more useTranslate --- eslint.config.js | 2 +- .../no-untranslated-strings.test.js | 100 ++----------- .../translation-utils.cjs | 140 ++---------------- .../ExtensionToolbarItemButton.test.tsx | 2 +- 4 files changed, 30 insertions(+), 214 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 2b8c6260969..055bb6e4158 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -128,7 +128,7 @@ module.exports = [ { group: ['react-i18next', 'i18next'], importNames: ['t'], - message: 'Please import useTranslate from @grafana/i18n and use the t function instead', + message: 'Please import from @grafana/i18n instead', }, { group: ['react-i18next'], diff --git a/packages/grafana-i18n/src/eslint/no-untranslated-strings/no-untranslated-strings.test.js b/packages/grafana-i18n/src/eslint/no-untranslated-strings/no-untranslated-strings.test.js index 8c5afef89bc..9913ab10f73 100644 --- a/packages/grafana-i18n/src/eslint/no-untranslated-strings/no-untranslated-strings.test.js +++ b/packages/grafana-i18n/src/eslint/no-untranslated-strings/no-untranslated-strings.test.js @@ -7,9 +7,8 @@ const filename = 'public/app/features/some-feature/nested/SomeFile.tsx'; const packageName = '@grafana/i18n'; const TRANS_IMPORT = `import { Trans } from '${packageName}';`; -const T_IMPORT = `import { t } from '${packageName}/internal';`; -const USE_TRANSLATE_IMPORT = `import { useTranslate } from '${packageName}';`; -const TRANS_AND_USE_TRANSLATE_IMPORT = `import { Trans, useTranslate } from '${packageName}';`; +const T_IMPORT = `import { t } from '${packageName}';`; +const TRANS_AND_T_IMPORT = `import { Trans, t } from '${packageName}';`; const ruleTester = new RuleTester({ languageOptions: { @@ -330,7 +329,7 @@ const Foo = () =>
) @@ -429,10 +427,9 @@ const Foo = () => { { messageId: 'wrapWithT', output: ` -${TRANS_AND_USE_TRANSLATE_IMPORT} +${TRANS_AND_T_IMPORT} const Foo = () => { - const { t } = useTranslate(); -return ( + return (
) }`, @@ -469,36 +466,6 @@ const Foo = () => { }, ], }, - { - name: 'Fixes correctly when useTranslate already exists', - code: ` -${USE_TRANSLATE_IMPORT} -const Foo = () => { - const { t } = useTranslate(); - return ( -
- ) -}`, - filename, - errors: [ - { - messageId: 'noUntranslatedStringsProp', - suggestions: [ - { - messageId: 'wrapWithT', - output: ` -${USE_TRANSLATE_IMPORT} -const Foo = () => { - const { t } = useTranslate(); - return ( -
- ) -}`, - }, - ], - }, - ], - }, { name: 'Fixes and uses ID from attribute if exists', @@ -563,41 +530,6 @@ const Foo = () =>
`, ], }, - { - name: 'Fixes correctly when useTranslate import already exists', - code: ` -${USE_TRANSLATE_IMPORT} -const Foo = () => { - const { t } = useTranslate(); - return (<> -
-
- ) -} -`, - filename, - errors: [ - { - messageId: 'noUntranslatedStringsProp', - suggestions: [ - { - messageId: 'wrapWithT', - output: ` -${USE_TRANSLATE_IMPORT} -const Foo = () => { - const { t } = useTranslate(); - return (<> -
-
- ) -} -`, - }, - ], - }, - ], - }, - { name: 'Fixes correctly when no return statement', code: ` @@ -637,8 +569,7 @@ const Foo = () =>
`, { messageId: 'wrapWithT', output: ` -${T_IMPORT} -${TRANS_IMPORT} +${TRANS_AND_T_IMPORT} const Foo = () =>
`, }, ], @@ -787,10 +718,9 @@ const Foo = () => { { messageId: 'wrapWithT', output: ` -${USE_TRANSLATE_IMPORT} +${T_IMPORT} const Foo = () => { - const { t } = useTranslate(); -const thing = { + const thing = { label: t(\"some-feature.foo.thing.label.test\", \"test\"), } @@ -893,10 +823,9 @@ const Foo = () => { filename, options: [{ forceFix: ['public/app/features/some-feature'] }], output: ` -${USE_TRANSLATE_IMPORT} +${T_IMPORT} const Foo = () => { - const { t } = useTranslate(); -return
+ return
}`, errors: [ { @@ -905,10 +834,9 @@ return
{ messageId: 'wrapWithT', output: ` -${USE_TRANSLATE_IMPORT} +${T_IMPORT} const Foo = () => { - const { t } = useTranslate(); -return
+ return
}`, }, ], diff --git a/packages/grafana-i18n/src/eslint/no-untranslated-strings/translation-utils.cjs b/packages/grafana-i18n/src/eslint/no-untranslated-strings/translation-utils.cjs index 77de33dbed0..070e0c5ee53 100644 --- a/packages/grafana-i18n/src/eslint/no-untranslated-strings/translation-utils.cjs +++ b/packages/grafana-i18n/src/eslint/no-untranslated-strings/translation-utils.cjs @@ -98,11 +98,10 @@ function canBeFixed(node, context) { } // If we're going to try and fix using `t`, and it already exists in the scope, - // but not from `useTranslate`, then we can't fix/provide a suggestion + // then we can't fix/provide a suggestion if (isPropertyOrAttribute && parentMethod) { const hasTDeclaration = getTDeclaration(parentMethod, context); - const hasUseTranslateDeclaration = methodHasUseTranslate(parentMethod, context); - if (hasTDeclaration && !hasUseTranslateDeclaration) { + if (hasTDeclaration) { return false; } } @@ -135,6 +134,15 @@ function canBeFixed(node, context) { return true; } +/** + * For a given node, check the scope and find a variable declaration of `t` + * @param {Node} node + * @param {RuleContextWithOptions} context + */ +function getTDeclaration(node, context) { + return context.sourceCode.getScope(node).variables.find((v) => v.name === 't'); +} + /** * Gets the translation prefix from the filename * @param {RuleContextWithOptions} context @@ -233,44 +241,11 @@ function getComponentNames(node, context) { return names; } -/** - * For a given node, check the scope and find a variable declaration of `t` - * @param {Node} node - * @param {RuleContextWithOptions} context - */ -function getTDeclaration(node, context) { - return context.sourceCode.getScope(node).variables.find((v) => v.name === 't'); -} - -/** - * Checks if a node has a variable declaration of `t` - * that came from a `useTranslate` call - * @param {Node} node The node - * @param {RuleContextWithOptions} context - */ -function methodHasUseTranslate(node, context) { - const tDeclaration = getTDeclaration(node, context); - return ( - tDeclaration && - tDeclaration.defs.find((definition) => { - const isVariableDeclaration = definition.node.type === AST_NODE_TYPES.VariableDeclarator; - const declarationInit = isVariableDeclaration ? definition.node.init : null; - return ( - isVariableDeclaration && - declarationInit && - declarationInit.type === AST_NODE_TYPES.CallExpression && - declarationInit.callee.type === AST_NODE_TYPES.Identifier && - declarationInit.callee.name === 'useTranslate' - ); - }) - ); -} - /** * Gets the import fixer for a node * @param {JSXElement|JSXFragment|JSXAttribute|Property} node * @param {RuleFixer} fixer The fixer - * @param {'Trans'|'t'|'useTranslate'} importName The member to import from either `@grafana/i18n` or `@grafana/i18n/internal` + * @param {'Trans'|'t'} importName The member to import from `@grafana/i18n` * @param {RuleContextWithOptions} context * @returns {import('@typescript-eslint/utils/ts-eslint').RuleFix|undefined} The fix */ @@ -280,22 +255,9 @@ function getImportsFixer(node, fixer, importName, context) { /** Map of where we expect to import each translation util from */ const importPackage = { Trans: '@grafana/i18n', - useTranslate: '@grafana/i18n', - t: '@grafana/i18n/internal', + t: '@grafana/i18n', }; - const parentMethod = getParentMethod(node, context); - - if (importName === 't') { - // If we're trying to import `t`, - // and there's already a `t` variable declaration in the parent method that came from `useTranslate`, - // do nothing - const declarationFromUseTranslate = parentMethod ? methodHasUseTranslate(parentMethod, context) : false; - if (declarationFromUseTranslate) { - return; - } - } - const expectedImport = importPackage[importName]; const existingAppCoreI18n = body.find( @@ -354,73 +316,6 @@ const getTransFixers = (node, context) => (fixer) => { return fixes; }; -/** - * @param {string} str - */ -const firstCharIsUpper = (str) => { - return str.charAt(0) === str.charAt(0).toUpperCase(); -}; - -/** - * @param {JSXAttribute|Property} node - * @param {RuleFixer} fixer - * @param {RuleContextWithOptions} context - * @returns {import('@typescript-eslint/utils/ts-eslint').RuleFix|undefined} The fix - */ -const getUseTranslateFixer = (node, fixer, context) => { - const parentMethod = getParentMethod(node, context); - - const functionIsNotUpperCase = - parentMethod && - parentMethod.type === AST_NODE_TYPES.FunctionDeclaration && - (!parentMethod.id || !firstCharIsUpper(parentMethod.id.name)); - - const variableDeclaratorIsNotUpperCase = - parentMethod && - parentMethod.parent.type === AST_NODE_TYPES.VariableDeclarator && - parentMethod.parent.id.type === AST_NODE_TYPES.Identifier && - !firstCharIsUpper(parentMethod.parent.id.name); - - // If the node is not within a function, or the parent method does not start with an uppercase letter, - // then we can't reliably add `useTranslate`, as this may not be a React component - if ( - !parentMethod || - functionIsNotUpperCase || - variableDeclaratorIsNotUpperCase || - parentMethod.body.type !== AST_NODE_TYPES.BlockStatement - ) { - return; - } - - const returnStatement = parentMethod.body.body.find((node) => node.type === AST_NODE_TYPES.ReturnStatement); - if (!returnStatement) { - return; - } - - const returnStatementIsJsx = - returnStatement.argument && - (returnStatement.argument.type === AST_NODE_TYPES.JSXElement || - returnStatement.argument.type === AST_NODE_TYPES.JSXFragment); - - if (!returnStatementIsJsx) { - return; - } - const tDeclarationExists = getTDeclaration(parentMethod, context); - const useTranslateExists = methodHasUseTranslate(parentMethod, context); - - if (tDeclarationExists && useTranslateExists) { - return; - } - - // If we've got all this way, then: - // - There is a parent method - // - It returns JSX - // - The method name starts with a capital letter - // - There is not already a call to `useTranslate` in the parent method - // In that scenario, we assume that we can fix and add a usage of the hook to the start of the body of the method - return fixer.insertTextBefore(parentMethod.body.body[0], 'const { t } = useTranslate();\n'); -}; - /** * @param {JSXAttribute|Property} node * @param {RuleContextWithOptions} context @@ -440,14 +335,7 @@ const getTFixers = (node, context) => (fixer) => { ); } - // Check if we need to add `useTranslate` to the node - const useTranslateFixer = getUseTranslateFixer(node, fixer, context); - if (useTranslateFixer) { - fixes.push(useTranslateFixer); - } - - // Check if we need to add `t` or `useTranslate` to the imports - const importToAdd = useTranslateFixer ? 'useTranslate' : 't'; + const importToAdd = 't'; const importsFixer = getImportsFixer(node, fixer, importToAdd, context); if (importsFixer) { fixes.push(importsFixer); diff --git a/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionToolbarItemButton.test.tsx b/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionToolbarItemButton.test.tsx index 32b0b644a64..a8e6a83af32 100644 --- a/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionToolbarItemButton.test.tsx +++ b/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionToolbarItemButton.test.tsx @@ -2,7 +2,7 @@ import { render, screen, fireEvent } from '@testing-library/react'; import { ExtensionToolbarItemButton } from './ExtensionToolbarItemButton'; -// Mock the useTranslate hook +// Mock the t function jest.mock('@grafana/i18n', () => ({ t: (_: string, fallback: string, values?: Record) => { if (values) {