Chore: create the `no-untranslated-literals` rule #88271

eledobleefe · 2024-05-24T08:59:07Z

What is this feature?

We create an ESlint rule to avoid the increase of string literals without being marked for translation. The next step is to decide whether to add this to eslint, betterer, or another option.

Why do we need this feature?

To increase the areas in which Grafana has translations available.

Who is this feature for?

Everyone using translations.

Which issue(s) does this PR fix?:

Fixes #87897

Special notes for your reviewer:

Please check that:

It works as expected from a user's perspective.
If this is a pre-GA feature, it is behind a feature toggle.
The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

joshhunt

Great job! No comments on the lint rule itself, but I think the docs could do with a little work.

joshhunt · 2024-05-24T15:09:31Z

packages/grafana-eslint-rules/README.md

+
+Check if strings are marked for translation.
+
+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 similars.


Check out the good docs from @ashharrison90 for the no-unreduced-motion rule - as these docs are linked to from the lint error message, they should be written with the goal of helping the developer fix their code.

I think they should (very briefly) explain how to use <Trans /> or t() to translate a phrase, and then link to the rest of the i18n docs like you have.

i think we could shift this to the bottom. as a consumer of the rule i don't care about the implementation details so much 🤔

packages/grafana-eslint-rules/rules/no-untranslated-literals.cjs

ashharrison90

looks great 🙌

packages/grafana-eslint-rules/rules/no-untranslated-literals.cjs

packages/grafana-eslint-rules/README.md

ashharrison90 · 2024-05-24T16:27:11Z

packages/grafana-eslint-rules/README.md

+
+Check if strings are marked for translation.
+
+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 similars.


i think we could shift this to the bottom. as a consumer of the rule i don't care about the implementation details so much 🤔

ashharrison90 · 2024-05-24T16:30:43Z

packages/grafana-eslint-rules/README.md

+<InlineToast placement="top" referenceElement={buttonRef.current}>
+  <Trans i18nKey="clipboard-button.inline-toast.success">Copied</Trans>
+</InlineToast>
+


i think we could do with a {t()} example here:

// Good ✅ <InlineToast placement="top" referenceElement={buttonRef.current}> {t('clipboard-button.inline-toast.success', 'Copied')} </InlineToast>

🤔 But we are not checking this. Would this not trigger misunderstandings?

i think this is a valid way to translate, and should be shown as an example. there might be occasions where using the t() variant makes more sense. what misunderstandings can this cause? 🤔

i still think we should have this as a t() example as well:

// Good ✅ <InlineToast placement="top" referenceElement={buttonRef.current}> {t('clipboard-button.inline-toast.success', 'Copied')} </InlineToast>

🤔 But the internationalization doc says:
Prefer using <Trans /> for JSX children, and t() for props and other javascript usage.
If we are this example, are we not giving the wrong example?

packages/grafana-eslint-rules/README.md

packages/grafana-eslint-rules/rules/no-untranslated-literals.cjs

packages/grafana-eslint-rules/README.md

ashharrison90 · 2024-05-28T09:26:18Z

packages/grafana-eslint-rules/README.md

+<InlineToast placement="top" referenceElement={buttonRef.current}>
+  <Trans i18nKey="clipboard-button.inline-toast.success">Copied</Trans>
+</InlineToast>
+


i think this is a valid way to translate, and should be shown as an example. there might be occasions where using the t() variant makes more sense. what misunderstandings can this cause? 🤔

packages/grafana-eslint-rules/README.md

ashharrison90 · 2024-05-29T08:25:44Z

packages/grafana-eslint-rules/README.md

+<InlineToast placement="top" referenceElement={buttonRef.current}>
+  <Trans i18nKey="clipboard-button.inline-toast.success">Copied</Trans>
+</InlineToast>
+


i still think we should have this as a t() example as well:

// Good ✅ <InlineToast placement="top" referenceElement={buttonRef.current}> {t('clipboard-button.inline-toast.success', 'Copied')} </InlineToast>

packages/grafana-eslint-rules/README.md

ashharrison90 · 2024-05-29T08:26:36Z

packages/grafana-eslint-rules/README.md

+
+//Good ✅
+const serviceName = service.name;
+<Trans i18nKey="login.services.sing-in-with-prefix">Sign in with {{ serviceName }}</Trans>;


also think we should have a t() example here as well

ashharrison90 · 2024-05-29T08:27:05Z

packages/grafana-eslint-rules/README.md

+return <input type="value" placeholder={placeholder} />;
+```
+
+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.


think this makes more sense as the first sentence in the section? 🤔

ashharrison90

lgtm 👍

* main: (25 commits) Update dependency @react-types/overlays to v3.8.7 LogsTable: Fix default sort by time (#88398) Update dependency @react-types/menu to v3.9.9 datatrails: in Components, `model.useState()`, instead of `model.state` (#88432) Update dependency @react-types/button to v3.9.4 Alerting docs: adds template selector docs (#88412) Update dependency @playwright/test to v1.44.1 Update dependency @kusto/monaco-kusto to v10.0.22 Update dependency @grafana/faro-web-sdk to v1.7.3 Alerting: Support `alertingDisableSendAlertsExternal` feature toggle (#88384) Update dependency @grafana/faro-web-sdk to v1.7.3 Add navigation config for `grafana-csp-app` (#88360) Chore: create the `no-untranslated-literals` rule (#88271) Alerting: Update ListAlertRulesQuery to take a slice of RuleGroups (#88385) Update dependency @grafana/faro-core to v1.7.3 Revamp tests for Add/Update Datasource (#88386) Update dependency @floating-ui/react to v0.26.16 (#88409) Update dependency cypress to v13.10.0 (#84140) Add Kubernetes icon (#86677) Update `make docs` procedure (#88404) ...

eledobleefe added 9 commits May 15, 2024 13:26

Try the existent rule

02e7b32

Create rule

4b3b4c1

Rebuild rule

1e7bf85

Add rule to the index

e5889c8

Change rule

059f85c

Check if it is a bare string

3e3f161

Modify rule

a8e0a82

Change Literal to JSXText

71b6b63

Move empty function inside the rule return block

0d02596

eledobleefe added area/frontend internal for issues made by grafanistas labels May 24, 2024

eledobleefe self-assigned this May 24, 2024

eledobleefe requested review from a team and grafanabot as code owners May 24, 2024 08:59

eledobleefe requested review from tskarhed, ashharrison90 and oshirohugo and removed request for a team May 24, 2024 08:59

Delete betterer file changes

5e98aa0

eledobleefe added no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels May 24, 2024

eledobleefe added 5 commits May 24, 2024 11:10

merge main

0516a49

Add documentation

17e7b47

Fix documentation

b7f33e6

Fix documentation

8cf2d86

Merge branch 'main' into eledobleefe/i18n-no-literals-eslint-87897

b696ef5

eledobleefe added this to the 11.1.x milestone May 24, 2024

joshhunt reviewed May 24, 2024

View reviewed changes

eledobleefe added 3 commits May 24, 2024 17:34

Add more documentation

45c9c08

Fix documentation

5e9fa7b

Modify message

e8b8b2a

ashharrison90 reviewed May 24, 2024

View reviewed changes

tskarhed reviewed May 27, 2024

View reviewed changes

packages/grafana-eslint-rules/rules/no-untranslated-literals.cjs Outdated Show resolved Hide resolved

eledobleefe added 2 commits May 27, 2024 11:38

Modify rule name

648efbf

Modify documentation according to reviews

b314caf

ashharrison90 reviewed May 28, 2024

View reviewed changes

Modify doc according to reviews

569c615

eledobleefe requested review from joshhunt, tskarhed and ashharrison90 May 28, 2024 14:41

ashharrison90 reviewed May 29, 2024

View reviewed changes

Modify doc according to reviews

474a4bb

eledobleefe requested a review from ashharrison90 May 29, 2024 10:46

ashharrison90 approved these changes May 29, 2024

View reviewed changes

Merge branch 'main' into eledobleefe/i18n-no-literals-eslint-87897

235f2a4

eledobleefe merged commit a47e71f into main May 29, 2024
16 checks passed

eledobleefe deleted the eledobleefe/i18n-no-literals-eslint-87897 branch May 29, 2024 11:04

miguelmolina95 pushed a commit to miguelmolina95/grafana that referenced this pull request Jun 3, 2024

Chore: create the no-untranslated-literals rule (grafana#88271)

c697c69

kminehart pushed a commit that referenced this pull request Jun 10, 2024

Chore: create the no-untranslated-literals rule (#88271)

c21ce04

kevinwcyu modified the milestones: 11.1.x, 11.1.0 Jun 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore: create the `no-untranslated-literals` rule #88271

Chore: create the `no-untranslated-literals` rule #88271

eledobleefe commented May 24, 2024

joshhunt left a comment

joshhunt May 24, 2024

ashharrison90 May 24, 2024

ashharrison90 left a comment

ashharrison90 May 24, 2024

ashharrison90 May 24, 2024

eledobleefe May 27, 2024

ashharrison90 May 28, 2024 •

edited

ashharrison90 May 29, 2024

eledobleefe May 29, 2024

ashharrison90 May 28, 2024 •

edited

ashharrison90 May 29, 2024

ashharrison90 May 29, 2024

ashharrison90 May 29, 2024

ashharrison90 left a comment


		Check if strings are marked for translation.

		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 similars.

Chore: create the no-untranslated-literals rule #88271

Chore: create the no-untranslated-literals rule #88271

Conversation

eledobleefe commented May 24, 2024

joshhunt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashharrison90 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashharrison90 May 28, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashharrison90 May 28, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashharrison90 left a comment

Choose a reason for hiding this comment

Chore: create the `no-untranslated-literals` rule #88271

Chore: create the `no-untranslated-literals` rule #88271

ashharrison90 May 28, 2024 •

edited

ashharrison90 May 28, 2024 •

edited