Skip to content

chore: localization protection#282

Open
camrun91 wants to merge 9 commits into
mainfrom
localization_protection
Open

chore: localization protection#282
camrun91 wants to merge 9 commits into
mainfrom
localization_protection

Conversation

@camrun91

@camrun91 camrun91 commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

This adds ci, lint, and greptile rules to help gaurd against hardcoded strings.

Greptile Summary

This PR establishes the localization protection layer for @youversion/platform-react-ui: a CI locale-ownership gate, a static i18n parity check script, an ESLint rule for hardcoded strings, and documentation — plus immediately fixes four hardcoded aria-label/sr-only strings in bible-reader.tsx and popover.tsx using new keys already added to en.json.

  • CI jobs (locale-ownership, i18n-check): the ownership script now correctly requires both bot-author AND branch-prefix conditions simultaneously, closing both bypass vectors from the previous review. The parity script hard-fails only on structurally invalid states and warns on orphan/untranslated keys.
  • ESLint rule (i18next/no-literal-string): scoped to packages/ui/src/components/**/*.tsx, mode: 'jsx-only', checking aria-label, title, placeholder, and alt; tests and stories excluded.
  • Component fixes: all four new keys (decreaseFontSizeAriaLabel, increaseFontSizeAriaLabel, changeLineSpacingAriaLabel, closeAriaLabel) are present in en.json and consumed via t() consistent with existing patterns.

Confidence Score: 5/5

Safe to merge — all three protection layers are consistent with each other and the documentation, and the component fixes are backed by keys already in en.json.

The ownership script correctly enforces both conditions simultaneously, the parity script fails only on provably broken states, the ESLint rule is appropriately scoped, and the four component fixes are immediately verifiable against en.json. No functional regressions or data-loss paths were identified.

tools/eslint-config/index.js — the mode: 'jsx-only' configuration has a narrow gap for expression-wrapped string literals that the rule does not currently catch.

Important Files Changed

Filename Overview
.github/scripts/check-locale-ownership.sh New script enforcing locale file ownership — requires both bot author AND matching branch prefix simultaneously, addressing both bypass vectors from the previous review.
.github/workflows/ci.yml Adds locale-ownership (PR-only) and i18n-check (all pushes/PRs) CI jobs using the same toolchain setup pattern as existing jobs.
scripts/check-i18n-parity.mjs New static-analysis script: hard-fails on missing en.json keys, extra fr/es keys, and token mismatches; warns on orphan and untranslated keys.
tools/eslint-config/index.js Adds eslint-plugin-i18next scoped to packages/ui/src/components/**/*.tsx; mode: 'jsx-only' leaves a narrow gap for expression-wrapped string literals.
packages/ui/src/components/bible-reader.tsx Replaces three hardcoded aria-label strings with t() calls; all keys present in en.json.
packages/ui/src/components/ui/popover.tsx Adds useTranslation hook and replaces hardcoded 'Close' sr-only label with t('closeAriaLabel'); key exists in en.json.
docs/i18n-guidelines.md New documentation covering the full localization workflow, dynamic-key escape hatches, and CI job descriptions.
greptile.json Adds two new Greptile rules for hardcoded strings and locale ownership, plus a scoped reference to docs/i18n-guidelines.md.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    PR[Pull Request opened] --> LO{locale-ownership job
pull_request only}
    PR --> I18N[i18n-check job
all pushes and PRs]
    PR --> LINT[lint job]
    LO --> BOT{is_sync_bot_author?
AND is_sync_branch?}
    BOT -->|Yes - both true| SKIP[Skip check]
    BOT -->|No - either false| CHANGED{Locale files changed?}
    CHANGED -->|No| PASS1[Pass]
    CHANGED -->|Yes| FAIL1[Fail - Locale files are upstream-owned]
    I18N --> PARITY[check-i18n-parity.mjs]
    PARITY --> MISSING{Keys used in source missing from en.json?}
    MISSING -->|Yes| FAIL2[Fail]
    MISSING -->|No| EXTRA{Extra keys in fr/es not in en?}
    EXTRA -->|Yes| FAIL2
    EXTRA -->|No| TOKEN{Interpolation token mismatch?}
    TOKEN -->|Yes| FAIL2
    TOKEN -->|No| WARN[Warn only - missing fr/es keys, orphan en keys]
    WARN --> PASS2[Pass with warnings]
    LINT --> ESLINT[eslint-plugin-i18next no-literal-string]
    ESLINT --> SCOPE[packages/ui/src/components/**/*.tsx excluding test and stories]
    SCOPE --> HARDCODE{Hardcoded string in JSX text or aria-label/title/placeholder/alt?}
    HARDCODE -->|Yes| FAIL3[Fail]
    HARDCODE -->|No| PASS3[Pass]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    PR[Pull Request opened] --> LO{locale-ownership job
pull_request only}
    PR --> I18N[i18n-check job
all pushes and PRs]
    PR --> LINT[lint job]
    LO --> BOT{is_sync_bot_author?
AND is_sync_branch?}
    BOT -->|Yes - both true| SKIP[Skip check]
    BOT -->|No - either false| CHANGED{Locale files changed?}
    CHANGED -->|No| PASS1[Pass]
    CHANGED -->|Yes| FAIL1[Fail - Locale files are upstream-owned]
    I18N --> PARITY[check-i18n-parity.mjs]
    PARITY --> MISSING{Keys used in source missing from en.json?}
    MISSING -->|Yes| FAIL2[Fail]
    MISSING -->|No| EXTRA{Extra keys in fr/es not in en?}
    EXTRA -->|Yes| FAIL2
    EXTRA -->|No| TOKEN{Interpolation token mismatch?}
    TOKEN -->|Yes| FAIL2
    TOKEN -->|No| WARN[Warn only - missing fr/es keys, orphan en keys]
    WARN --> PASS2[Pass with warnings]
    LINT --> ESLINT[eslint-plugin-i18next no-literal-string]
    ESLINT --> SCOPE[packages/ui/src/components/**/*.tsx excluding test and stories]
    SCOPE --> HARDCODE{Hardcoded string in JSX text or aria-label/title/placeholder/alt?}
    HARDCODE -->|Yes| FAIL3[Fail]
    HARDCODE -->|No| PASS3[Pass]
Loading

Reviews (8): Last reviewed commit: "fix: bypass not matching" | Re-trigger Greptile

Context used (3)

  • Rule used - No hardcoded user-facing strings: use t() or Trans... (source)
  • File used - docs/i18n-guidelines.md (source)
  • File used - docs/review-guidelines.md (source)

@changeset-bot

changeset-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 5c0319e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Comment thread scripts/check-i18n-parity.mjs
Comment thread scripts/check-i18n-parity.mjs
Comment thread .github/scripts/check-locale-ownership.sh Outdated
Comment thread .github/scripts/check-locale-ownership.sh Outdated
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.

1 participant