Handle array values in i18nrc (#169637)

This commit is contained in:
Coen Warmer 2023-10-25 18:04:02 +02:00 committed by GitHub
parent 310ae23378
commit 8938a5778a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 256 additions and 59 deletions

View file

@ -9,13 +9,14 @@ tags: ['kibana', 'dev', 'contributor', 'operations', 'eslint', 'i18n']
`@kbn/eslint-plugin-i18n` is an ESLint plugin providing custom rules for validating JSXCode in the Kibana repo to make sure they are translated. `@kbn/eslint-plugin-i18n` is an ESLint plugin providing custom rules for validating JSXCode in the Kibana repo to make sure they are translated.
Note: At the moment these rules only work for apps that are inside `/x-pack/plugins`. Note: At the moment these rules only work for apps that are inside `/x-pack/plugins`.
If you want to enable this rule on code that is outside of this path, adjust `/helpers/get_i18n_identifier_from_file_path.ts`. If you want to enable this rule on code that is outside of this path, adjust `/helpers/get_i18n_identifier_from_file_path.ts`.
## `@kbn/i18n/strings_should_be_translated_with_i18n` ## `@kbn/i18n/strings_should_be_translated_with_i18n`
This rule warns engineers to translate their strings by using i18n.translate from the '@kbn/i18n' package. It provides an autofix that takes into account the context of the translatable string in the JSX tree to generate a translation ID. This rule warns engineers to translate their strings by using i18n.translate from the '@kbn/i18n' package. It provides an autofix that takes into account the context of the translatable string in the JSX tree to generate a translation ID.
It kicks in on JSXText elements and specific JSXAttributes (`label` and `aria-label`) which expect a translated value.
## `@kbn/i18n/strings_should_be_translated_with_formatted_message` ## `@kbn/i18n/strings_should_be_translated_with_formatted_message`
This rule warns engineers to translate their strings by using `<FormattedMessage>` from the '@kbn/i18n-react' package. It provides an autofix that takes into account the context of the translatable string in the JSX tree and to generate a translation ID. This rule warns engineers to translate their strings by using `<FormattedMessage>` from the '@kbn/i18n-react' package. It provides an autofix that takes into account the context of the translatable string in the JSX tree and to generate a translation ID.
It kicks in on JSXText elements and specific JSXAttributes (`label` and `aria-label`) which expect a translated value.

View file

@ -14,6 +14,10 @@ const testMap = [
['x-pack/plugins/observability/foo/bar/baz/header_actions.tsx', 'xpack.observability'], ['x-pack/plugins/observability/foo/bar/baz/header_actions.tsx', 'xpack.observability'],
['x-pack/plugins/apm/public/components/app/correlations/correlations_table.tsx', 'xpack.apm'], ['x-pack/plugins/apm/public/components/app/correlations/correlations_table.tsx', 'xpack.apm'],
['x-pack/plugins/cases/public/components/foo.tsx', 'xpack.cases'], ['x-pack/plugins/cases/public/components/foo.tsx', 'xpack.cases'],
[
'x-pack/plugins/synthetics/public/apps/synthetics/components/alerts/toggle_alert_flyout_button.tsx',
'xpack.synthetics',
],
[ [
'packages/kbn-alerts-ui-shared/src/alert_lifecycle_status_badge/index.tsx', 'packages/kbn-alerts-ui-shared/src/alert_lifecycle_status_badge/index.tsx',
'app_not_found_in_i18nrc', 'app_not_found_in_i18nrc',

View file

@ -24,6 +24,8 @@ export function getI18nIdentifierFromFilePath(fileName: string, cwd: string) {
const i18nrc = JSON.parse(i18nrcFile); const i18nrc = JSON.parse(i18nrcFile);
return i18nrc && i18nrc.paths return i18nrc && i18nrc.paths
? findKey(i18nrc.paths, (v) => v === path) ?? 'app_not_found_in_i18nrc' ? findKey(i18nrc.paths, (v) =>
Array.isArray(v) ? v.find((e) => e === path) : typeof v === 'string' && v === path
) ?? 'app_not_found_in_i18nrc'
: 'could_not_find_i18nrc'; : 'could_not_find_i18nrc';
} }

View file

@ -5,12 +5,12 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server * in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1. * Side Public License, v 1.
*/ */
import { TSESTree } from '@typescript-eslint/typescript-estree'; import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree';
import { lowerCaseFirstLetter, upperCaseFirstLetter } from './utils'; import { lowerCaseFirstLetter, upperCaseFirstLetter } from './utils';
export function getIntentFromNode(originalNode: TSESTree.JSXText): string { export function getIntentFromNode(value: string, parent: TSESTree.Node | undefined): string {
const value = lowerCaseFirstLetter( const processedValue = lowerCaseFirstLetter(
originalNode.value value
.replace(/[?!@#$%^&*()_+\][{}|/<>,'"]/g, '') .replace(/[?!@#$%^&*()_+\][{}|/<>,'"]/g, '')
.trim() .trim()
.split(' ') .split(' ')
@ -19,8 +19,6 @@ export function getIntentFromNode(originalNode: TSESTree.JSXText): string {
.join('') .join('')
); );
const { parent } = originalNode;
if ( if (
parent && parent &&
'openingElement' in parent && 'openingElement' in parent &&
@ -30,11 +28,25 @@ export function getIntentFromNode(originalNode: TSESTree.JSXText): string {
const parentTagName = String(parent.openingElement.name.name); const parentTagName = String(parent.openingElement.name.name);
if (parentTagName.includes('Eui')) { if (parentTagName.includes('Eui')) {
return `${value}${parentTagName.replace('Eui', '')}Label`; return `${processedValue}${parentTagName.replace('Eui', '')}Label`;
} }
return `${lowerCaseFirstLetter(parentTagName)}.${value}Label`; return `${lowerCaseFirstLetter(parentTagName)}.${processedValue}Label`;
} }
return `${value}Label`; if (
parent &&
'parent' in parent &&
parent.parent &&
'name' in parent.parent &&
typeof parent.parent.name !== 'string' &&
'type' in parent.parent.name &&
parent.parent.name.type === AST_NODE_TYPES.JSXIdentifier
) {
const parentTagName = String(parent.parent.name.name);
return `${lowerCaseFirstLetter(parentTagName)}.${processedValue}Label`;
}
return `${processedValue}Label`;
} }

View file

@ -95,6 +95,18 @@ function YetAnotherComponent() {
) )
}`, }`,
}, },
{
filename: 'x-pack/plugins/observability/public/test_component.tsx',
code: `
import React from 'react';
import { FormattedMessage } from '@kbn/i18n-react';
function TestComponent() {
return (
<SomeChildComponent label={<FormattedMessage id="app_not_found_in_i18nrc.testComponent.someChildComponent.thisIsATestLabel" defaultMessage="This is a test" />} />
)
}`,
},
]; ];
const invalid = [ const invalid = [
@ -160,6 +172,25 @@ function YetAnotherComponent() {
], ],
output: valid[2].code, output: valid[2].code,
}, },
{
filename: valid[3].filename,
code: `
import React from 'react';
import { FormattedMessage } from '@kbn/i18n-react';
function TestComponent() {
return (
<SomeChildComponent label="This is a test" />
)
}`,
errors: [
{
line: 7,
message: `Strings should be translated with <FormattedMessage />. Use the autofix suggestion or add your own.`,
},
],
output: valid[3].code,
},
]; ];
for (const [name, tester] of [tsTester, babelTester]) { for (const [name, tester] of [tsTester, babelTester]) {

View file

@ -37,7 +37,7 @@ export const StringsShouldBeTranslatedWithFormattedMessage: Rule.RuleModule = {
const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd); const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd);
const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration; const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration;
const functionName = getFunctionName(functionDeclaration); const functionName = getFunctionName(functionDeclaration);
const intent = getIntentFromNode(node); const intent = getIntentFromNode(value, node.parent);
const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel' const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel'
@ -72,6 +72,64 @@ export const StringsShouldBeTranslatedWithFormattedMessage: Rule.RuleModule = {
}, },
}); });
}, },
JSXAttribute: (node: TSESTree.JSXAttribute) => {
if (node.name.name !== 'aria-label' && node.name.name !== 'label') return;
let val: string = '';
// label={'foo'}
if (
node.value &&
'expression' in node.value &&
'value' in node.value.expression &&
typeof node.value.expression.value === 'string'
) {
val = node.value.expression.value;
}
// label="foo"
if (node.value && 'value' in node.value && typeof node.value.value === 'string') {
val = node.value.value;
}
if (!val) return;
// Start building the translation ID suggestion
const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd);
const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration;
const functionName = getFunctionName(functionDeclaration);
const intent = getIntentFromNode(val, node);
const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel'
// Check if i18n has already been imported into the file.
const {
hasI18nImportLine,
i18nPackageImportLine: i18nImportLine,
rangeToAddI18nImportLine,
} = getI18nImportFixer({
sourceCode,
mode: 'FormattedMessage',
});
// Show warning to developer and offer autofix suggestion
report({
node: node as any,
message:
'Strings should be translated with <FormattedMessage />. Use the autofix suggestion or add your own.',
fix(fixer) {
return [
fixer.replaceTextRange(
node.value!.range,
`{<FormattedMessage id="${translationIdSuggestion}" defaultMessage="${val}" />}`
),
!hasI18nImportLine
? fixer.insertTextAfterRange(rangeToAddI18nImportLine, `\n${i18nImportLine}`)
: null,
].filter(isTruthy);
},
});
},
} as Rule.RuleListener; } as Rule.RuleListener;
}, },
}; };

View file

@ -46,10 +46,10 @@ import React from 'react';
import { i18n } from '@kbn/i18n'; import { i18n } from '@kbn/i18n';
function TestComponent() { function TestComponent() {
return ( return (
<div>{i18n.translate('app_not_found_in_i18nrc.testComponent.div.thisIsATestLabel', { defaultMessage: "This is a test"})}</div> <div>{i18n.translate('app_not_found_in_i18nrc.testComponent.div.thisIsATestLabel', { defaultMessage: 'This is a test'})}</div>
) )
}`, }`,
}, },
{ {
filename: 'x-pack/plugins/observability/public/another_component.tsx', filename: 'x-pack/plugins/observability/public/another_component.tsx',
@ -58,30 +58,42 @@ import React from 'react';
import { i18n } from '@kbn/i18n'; import { i18n } from '@kbn/i18n';
function AnotherComponent() { function AnotherComponent() {
return ( return (
<EuiPanel> <EuiPanel>
<EuiFlexGroup> <EuiFlexGroup>
<EuiFlexItem> <EuiFlexItem>
<EuiButton>{i18n.translate('app_not_found_in_i18nrc.anotherComponent.thisIsATestButtonLabel', { defaultMessage: "This is a test"})}</EuiButton> <EuiButton>{i18n.translate('app_not_found_in_i18nrc.anotherComponent.thisIsATestButtonLabel', { defaultMessage: 'This is a test'})}</EuiButton>
</EuiFlexItem> </EuiFlexItem>
</EuiFlexGroup> </EuiFlexGroup>
</EuiPanel> </EuiPanel>
) )
}`, }`,
}, },
{ {
filename: 'x-pack/plugins/observability/public/yet_another_component.tsx', filename: 'x-pack/plugins/observability/public/yet_another_component.tsx',
code: ` code: `
import React from 'react';
import { i18n } from '@kbn/i18n';
function YetAnotherComponent() {
return (
<div>
<EuiSelect>{i18n.translate('app_not_found_in_i18nrc.yetAnotherComponent.selectMeSelectLabel', { defaultMessage: 'Select me'})}</EuiSelect>
</div>
)
}`,
},
{
filename: 'x-pack/plugins/observability/public/test_component.tsx',
code: `
import React from 'react'; import React from 'react';
import { i18n } from '@kbn/i18n'; import { i18n } from '@kbn/i18n';
function YetAnotherComponent() { function TestComponent() {
return ( return (
<div> <SomeChildComponent label={i18n.translate('app_not_found_in_i18nrc.testComponent.someChildComponent.thisIsATestLabel', { defaultMessage: 'This is a test'})} />
<EuiSelect>{i18n.translate('app_not_found_in_i18nrc.yetAnotherComponent.selectMeSelectLabel', { defaultMessage: "Select me"})}</EuiSelect> )
</div> }`,
)
}`,
}, },
]; ];
@ -92,10 +104,10 @@ const invalid = [
import React from 'react'; import React from 'react';
function TestComponent() { function TestComponent() {
return ( return (
<div>This is a test</div> <div>This is a test</div>
) )
}`, }`,
errors: [ errors: [
{ {
line: 6, line: 6,
@ -110,16 +122,16 @@ function TestComponent() {
import React from 'react'; import React from 'react';
function AnotherComponent() { function AnotherComponent() {
return ( return (
<EuiPanel> <EuiPanel>
<EuiFlexGroup> <EuiFlexGroup>
<EuiFlexItem> <EuiFlexItem>
<EuiButton>This is a test</EuiButton> <EuiButton>This is a test</EuiButton>
</EuiFlexItem> </EuiFlexItem>
</EuiFlexGroup> </EuiFlexGroup>
</EuiPanel> </EuiPanel>
) )
}`, }`,
errors: [ errors: [
{ {
line: 9, line: 9,
@ -131,15 +143,15 @@ function AnotherComponent() {
{ {
filename: valid[2].filename, filename: valid[2].filename,
code: ` code: `
import React from 'react'; import React from 'react';
function YetAnotherComponent() { function YetAnotherComponent() {
return ( return (
<div> <div>
<EuiSelect>Select me</EuiSelect> <EuiSelect>Select me</EuiSelect>
</div> </div>
) )
}`, }`,
errors: [ errors: [
{ {
line: 7, line: 7,
@ -148,6 +160,25 @@ function YetAnotherComponent() {
], ],
output: valid[2].code, output: valid[2].code,
}, },
{
filename: valid[3].filename,
code: `
import React from 'react';
import { i18n } from '@kbn/i18n';
function TestComponent() {
return (
<SomeChildComponent label="This is a test" />
)
}`,
errors: [
{
line: 7,
message: `Strings should be translated with i18n. Use the autofix suggestion or add your own.`,
},
],
output: valid[3].code,
},
]; ];
for (const [name, tester] of [tsTester, babelTester]) { for (const [name, tester] of [tsTester, babelTester]) {

View file

@ -37,7 +37,65 @@ export const StringsShouldBeTranslatedWithI18n: Rule.RuleModule = {
const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd); const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd);
const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration; const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration;
const functionName = getFunctionName(functionDeclaration); const functionName = getFunctionName(functionDeclaration);
const intent = getIntentFromNode(node); const intent = getIntentFromNode(value, node.parent);
const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel'
// Check if i18n has already been imported into the file
const {
hasI18nImportLine,
i18nPackageImportLine: i18nImportLine,
rangeToAddI18nImportLine,
} = getI18nImportFixer({
sourceCode,
mode: 'i18n.translate',
});
// Show warning to developer and offer autofix suggestion
report({
node: node as any,
message:
'Strings should be translated with i18n. Use the autofix suggestion or add your own.',
fix(fixer) {
return [
fixer.replaceText(
node,
`${whiteSpaces}{i18n.translate('${translationIdSuggestion}', { defaultMessage: '${value}'})}`
),
!hasI18nImportLine
? fixer.insertTextAfterRange(rangeToAddI18nImportLine, `\n${i18nImportLine}`)
: null,
].filter(isTruthy);
},
});
},
JSXAttribute: (node: TSESTree.JSXAttribute) => {
if (node.name.name !== 'aria-label' && node.name.name !== 'label') return;
let val: string = '';
// label={'foo'}
if (
node.value &&
'expression' in node.value &&
'value' in node.value.expression &&
typeof node.value.expression.value === 'string'
) {
val = node.value.expression.value;
}
// label="foo"
if (node.value && 'value' in node.value && typeof node.value.value === 'string') {
val = node.value.value;
}
if (!val) return;
// Start building the translation ID suggestion
const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd);
const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration;
const functionName = getFunctionName(functionDeclaration);
const intent = getIntentFromNode(val, node);
const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel' const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel'
@ -58,9 +116,9 @@ export const StringsShouldBeTranslatedWithI18n: Rule.RuleModule = {
'Strings should be translated with i18n. Use the autofix suggestion or add your own.', 'Strings should be translated with i18n. Use the autofix suggestion or add your own.',
fix(fixer) { fix(fixer) {
return [ return [
fixer.replaceText( fixer.replaceTextRange(
node, node.value!.range,
`${whiteSpaces}{i18n.translate('${translationIdSuggestion}', { defaultMessage: "${value}"})}` `{i18n.translate('${translationIdSuggestion}', { defaultMessage: '${val}'})}`
), ),
!hasI18nImportLine !hasI18nImportLine
? fixer.insertTextAfterRange(rangeToAddI18nImportLine, `\n${i18nImportLine}`) ? fixer.insertTextAfterRange(rangeToAddI18nImportLine, `\n${i18nImportLine}`)