diff --git a/.betterer.eslint.config.js b/.betterer.eslint.config.js index 1be44601a3c..349ee612837 100644 --- a/.betterer.eslint.config.js +++ b/.betterer.eslint.config.js @@ -113,6 +113,7 @@ module.exports = [ ignores: ['public/app/plugins/**', '**/*.story.tsx', '**/*.{test,spec}.{ts,tsx}', '**/__mocks__/', 'public/test'], rules: { '@grafana/no-untranslated-strings': 'error', + '@grafana/no-translation-top-level': 'error', }, }, ]; diff --git a/.betterer.results b/.betterer.results index d5ac5c018d3..1e0cc343e3b 100644 --- a/.betterer.results +++ b/.betterer.results @@ -549,6 +549,11 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], + "packages/grafana-ui/src/components/DataSourceSettings/DataSourceHttpSettings.tsx:5381": [ + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "1"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "2"] + ], "packages/grafana-ui/src/components/DataSourceSettings/types.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] @@ -3044,7 +3049,8 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use export all (\`export * from ...\`)", "0"] ], "public/app/features/connections/tabs/ConnectData/Search/Search.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"] + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"] ], "public/app/features/connections/tabs/ConnectData/Search/index.tsx:5381": [ [0, 0, 0, "Do not use export all (\`export * from ...\`)", "0"] @@ -4568,9 +4574,18 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], [0, 0, 0, "No untranslated strings. Wrap text with ", "3"] ], + "public/app/features/explore/RichHistory/RichHistorySettingsTab.tsx:5381": [ + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "1"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "2"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "3"] + ], "public/app/features/explore/RichHistory/RichHistoryStarredTab.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], + "public/app/features/explore/ShortLinkButtonMenu.tsx:5381": [ + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"] + ], "public/app/features/explore/SupplementaryResultError.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], @@ -4929,8 +4944,14 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "4"] ], "public/app/features/inspector/InspectJSONTab.tsx:5381": [ - [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "1"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "2"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "3"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "4"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "5"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "6"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "7"] ], "public/app/features/inspector/InspectStatsTab.tsx:5381": [ [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"] diff --git a/packages/grafana-eslint-rules/README.md b/packages/grafana-eslint-rules/README.md index 2f73d46a529..e57e2ec09a1 100644 --- a/packages/grafana-eslint-rules/README.md +++ b/packages/grafana-eslint-rules/README.md @@ -156,7 +156,18 @@ const SearchTitle = ({ term }) => ( #### How to translate props or attributes -Right now, we only check if a string is wrapped up by the `Trans` tag. We currently do not apply this rule to props, attributes or similar, but we also ask for them to be translated with the `t()` function. +This rule checks if a string is wrapped up by the `Trans` tag, or if certain props contain untranslated strings. +We ask for such props to be translated with the `t()` function. + +The below props are checked for untranslated strings: + +- `label` +- `description` +- `placeholder` +- `aria-label` +- `title` +- `text` +- `tooltip` ```tsx // Bad ❌ @@ -168,3 +179,50 @@ return ; ``` Check more info about how translations work in Grafana in [Internationalization.md](https://github.com/grafana/grafana/blob/main/contribute/internationalization.md) + +### `no-translation-top-level` + +Ensure that `t()` translation method is not used at the top level of a file, outside of a component of method. +This is to prevent calling the translation method before it's been instantiated. + +This does not cause an error if a file is lazily loaded, but refactors can cause errors, and it can cause problems in tests. +Fix the + +```tsx +// Bad ❌ +const someTranslatedText = t('some.key', 'Some text'); +const SomeComponent = () => { + return
; +}; + +// Good ✅ +const SomeComponent = () => { + const someTranslatedText = t('some.key', 'Some text'); + return
; +}; + +// Bad ❌ +const someConfigThatHasToBeShared = [{ foo: t('some.key', 'Some text') }]; +const SomeComponent = () => { + return ( +
+ {someConfigThatHasToBeShared.map((cfg) => { + return
{cfg.foo}
; + })} +
+ ); +}; + +// Good ✅ +const someConfigThatHasToBeShared = () => [{ foo: t('some.key', 'Some text') }]; +const SomeComponent = () => { + const configs = someConfigThatHasToBeShared(); + return ( +
+ {configs.map((cfg) => { + return
{cfg.foo}
; + })} +
+ ); +}; +``` diff --git a/packages/grafana-eslint-rules/index.cjs b/packages/grafana-eslint-rules/index.cjs index 21c3bfa739e..11be6258993 100644 --- a/packages/grafana-eslint-rules/index.cjs +++ b/packages/grafana-eslint-rules/index.cjs @@ -2,6 +2,7 @@ const noAriaLabelSelectors = require('./rules/no-aria-label-e2e-selectors.cjs'); const noBorderRadiusLiteral = require('./rules/no-border-radius-literal.cjs'); const noUnreducedMotion = require('./rules/no-unreduced-motion.cjs'); const noUntranslatedStrings = require('./rules/no-untranslated-strings.cjs'); +const noTranslationTopLevel = require('./rules/no-translation-top-level.cjs'); const themeTokenUsage = require('./rules/theme-token-usage.cjs'); module.exports = { @@ -11,5 +12,6 @@ module.exports = { 'no-border-radius-literal': noBorderRadiusLiteral, 'theme-token-usage': themeTokenUsage, 'no-untranslated-strings': noUntranslatedStrings, + 'no-translation-top-level': noTranslationTopLevel, }, }; diff --git a/packages/grafana-eslint-rules/rules/no-translation-top-level.cjs b/packages/grafana-eslint-rules/rules/no-translation-top-level.cjs new file mode 100644 index 00000000000..2290a980cf1 --- /dev/null +++ b/packages/grafana-eslint-rules/rules/no-translation-top-level.cjs @@ -0,0 +1,59 @@ +// @ts-check +const { ESLintUtils, AST_NODE_TYPES } = require('@typescript-eslint/utils'); + +/** + * @typedef {import('@typescript-eslint/utils').TSESTree.Node} Node + * @typedef {import('@typescript-eslint/utils').TSESLint.RuleContext} RuleContext + */ + +const createRule = ESLintUtils.RuleCreator( + (name) => `https://github.com/grafana/grafana/blob/main/packages/grafana-eslint-rules/README.md#${name}` +); + +/** + * @param {RuleContext} context + * @param {Node} node + * @returns {boolean} + */ +const isInFunction = (context, node) => { + const ancestors = context.sourceCode.getAncestors(node); + return ancestors.some((anc) => { + return [ + AST_NODE_TYPES.ArrowFunctionExpression, + AST_NODE_TYPES.FunctionDeclaration, + AST_NODE_TYPES.FunctionExpression, + AST_NODE_TYPES.ClassDeclaration, + ].includes(anc.type); + }); +}; + +const noTranslationTopLevel = createRule({ + create(context) { + return { + CallExpression(node) { + if (node.callee.type === AST_NODE_TYPES.Identifier && node.callee.name === 't') { + if (!isInFunction(context, node)) { + context.report({ + node, + messageId: 'noMethodOutsideComponent', + }); + } + } + }, + }; + }, + name: 'no-translation-top-level', + meta: { + type: 'suggestion', + docs: { + description: 'Do not use translation functions outside of components', + }, + messages: { + noMethodOutsideComponent: 'Do not use the t() function outside of a component or function', + }, + schema: [], + }, + defaultOptions: [], +}); + +module.exports = noTranslationTopLevel; diff --git a/packages/grafana-eslint-rules/tests/no-translation-top-level.test.js b/packages/grafana-eslint-rules/tests/no-translation-top-level.test.js new file mode 100644 index 00000000000..88326b500d4 --- /dev/null +++ b/packages/grafana-eslint-rules/tests/no-translation-top-level.test.js @@ -0,0 +1,58 @@ +import { RuleTester } from 'eslint'; + +import noTranslationTopLevel from '../rules/no-translation-top-level.cjs'; + +RuleTester.setDefaultConfig({ + languageOptions: { + ecmaVersion: 2018, + sourceType: 'module', + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + }, +}); + +const expectedErrorMessage = 'Do not use the t() function outside of a component or function'; + +const ruleTester = new RuleTester(); + +ruleTester.run('eslint no-translation-top-level', noTranslationTopLevel, { + valid: [ + { + code: ` +function Component() { + return
{t('some.key', 'Some text')}
; +} + `, + }, + { + code: `const foo = () => t('some.key', 'Some text');`, + }, + { + code: `const foo = ttt('some.key', 'Some text');`, + }, + { + code: `class Component { +render() { + return t('some.key', 'Some text'); +} +}`, + }, + ], + invalid: [ + { + code: `const thing = t('some.key', 'Some text');`, + errors: [{ message: expectedErrorMessage }], + }, + { + code: `const things = [t('some.key', 'Some text')];`, + errors: [{ message: expectedErrorMessage }], + }, + { + code: `const objectThings = [{foo: t('some.key', 'Some text')}];`, + errors: [{ message: expectedErrorMessage }], + }, + ], +});