Skip to content

Enforce lazy translations: add no-module-level-translations ESLint rule - #4336

Merged
sejas merged 9 commits into
trunkfrom
stu-1771-add-lint-rule-enforce-translating-strings
Aug 10, 2026
Merged

Enforce lazy translations: add no-module-level-translations ESLint rule#4336
sejas merged 9 commits into
trunkfrom
stu-1771-add-lint-rule-enforce-translating-strings

Conversation

@sejas

@sejas sejas commented Jul 24, 2026

Copy link
Copy Markdown
Member

Related issues

Translatable strings are sometimes evaluated at module-load time (e.g. const LABEL = __( 'Label' )). In the long-lived desktop renderer the user can change the app language at runtime, but a string captured at import time stays frozen in the language that was active when the module first loaded — so parts of the UI silently fail to translate after a language switch.

How AI was used in this PR

AI assistance authored the ESLint rule and applied the repetitive "wrap the translation in a lazy getter" refactor across the affected files. Every change was reviewed by hand and verified with npm run lint, npm run typecheck, and the affected npm test suites.

Proposed Changes

This adds a lint rule that catches the mistake at author time and makes the existing offenders lazy so they react to language changes:

  • New studio/no-module-level-translations ESLint rule flags __(), _x(), _n(), and _nx() calls that run at module scope. It allows translations wrapped in a function (evaluated at render/call time) and ignores bare discarded statements used only to feed the translation extractor.
  • The rule is turned off where a stale string is impossible: apps/cli (one-shot process that loads its locale before importing modules) and apps/ui (the agentic UI reloads the window on language change). It stays enforced for the long-lived desktop renderer and the shared packages/common code it imports.
  • Documents the convention in AGENTS.md.
  • Makes all flagged strings lazy (widget/control labels, settings, skills constants, shared packages/common helpers, etc.), so they now follow the active language after a runtime switch. Shared config labels (e.g. supportedEditorConfig[ … ].label, terminalConfig[ … ].name) became functions, and all their consumers — including code that landed on trunk while this branch was open — were updated to call them.

No user-visible text changes; the user-facing effect is that affected labels now translate correctly when the language is changed without a restart.

Testing Instructions

  • npm run lint — passes; try adding const X = __( 'Test' ) at module scope in a renderer file and confirm the rule reports it, then wrap it as const getX = () => __( 'Test' ) and confirm it passes.
  • npm run typecheck — passes.
  • npm test -- tools/eslint-plugin-studio — passes (includes the rule's unit tests).
  • Manually: switch the app language in Settings and confirm the affected labels (widget controls, settings options) update without restarting.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

…y translations

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sejas sejas self-assigned this Jul 24, 2026
@sejas
sejas marked this pull request as ready for review July 24, 2026 15:53
@sejas
sejas requested a review from a team July 24, 2026 15:53
@wpmobilebot

wpmobilebot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing 1cb127e vs trunk

app-size

Metric trunk 1cb127e Diff Change
App Size (Mac) 1406.20 MB 1406.20 MB +0.00 MB ⚪ 0.0%

site-editor

Metric trunk 1cb127e Diff Change
load 1077 ms 1046 ms 31 ms ⚪ 0.0%

site-startup

Metric trunk 1cb127e Diff Change
siteCreation 7539 ms 7533 ms 6 ms ⚪ 0.0%
siteStartup 2869 ms 2867 ms 2 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

@sejas
sejas requested review from bcotrim and nightnei July 27, 2026 10:12

@bcotrim bcotrim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a bug while testing: in Settings → AI, the Instructions textarea loses focus after every keystroke.
It comes from the getFields() change in studio-code-panel.tsx — the new array identity on each render makes DataForm remount the field.

Which leads to something to consider: apps/ui reloads the window on language change, so module-level translations can't go stale there. The rule's premise only holds for apps/studio, which swaps locale data live without a reload. Could we exempt apps/ui alongside apps/cli and revert the changes in that app?
That would fix the focus bug for free.

Longer term we could explore dropping the reload, but imo the impact doesn't justify the work.

What do you think?

@nightnei nightnei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well and LGTM, also confirm that the reported issue by Bernardo is resolved 👍
Screenshot 2026-08-07 at 18 43 41

sejas and others added 2 commits August 10, 2026 17:26
…le-enforce-translating-strings

# Conflicts:
#	eslint.config.mjs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sejas
sejas enabled auto-merge (squash) August 10, 2026 15:47
@sejas
sejas merged commit 86190e3 into trunk Aug 10, 2026
14 checks passed
@sejas
sejas deleted the stu-1771-add-lint-rule-enforce-translating-strings branch August 10, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants