fix(mobile): close localization review findings

This commit is contained in:
Brennan Benson
2026-08-01 05:07:43 -07:00
parent 92fcb73f8a
commit 02866adbd0
20 changed files with 407 additions and 552 deletions
-359
View File
@@ -1,359 +0,0 @@
import fs from 'node:fs/promises'
import path from 'node:path'
import { createHash } from 'node:crypto'
import { pathToFileURL } from 'node:url'
import process from 'node:process'
import ts from 'typescript-api'
import { collectLocalizationCandidates } from './audit-localization-coverage.mjs'
const TRANSLATE_IMPORT = "import { t } from '@/i18n/mobile-i18n'\n"
const JSX_ENTITY_VALUES = {
amp: '&',
apos: "'",
gt: '>',
lt: '<',
quot: '"'
}
function decodeJsxEntities(value) {
return value.replace(/&(amp|apos|gt|lt|quot);/gi, (_match, name) => {
return JSX_ENTITY_VALUES[name.toLowerCase()]
})
}
function keyForCandidate(candidate, fallback = candidate.text) {
const source = `${candidate.filePath}:${fallback}`
const hash = createHash('sha1').update(source).digest().subarray(0, 5).toString('base64url')
return `m.${hash}`
}
function setCatalogValue(catalog, key, value) {
const parts = key.split('.')
let current = catalog
for (const part of parts.slice(0, -1)) {
current[part] ??= {}
current = current[part]
}
current[parts.at(-1)] = value
}
function translateCall(key, _value, options) {
if (options) {
return `t(${JSON.stringify(key)}, ${options})`
}
return `t(${JSON.stringify(key)})`
}
function isInsideJsxExpression(node) {
let current = node.parent
while (current) {
if (ts.isJsxExpression(current)) {
return true
}
current = current.parent
}
return false
}
function editForCandidate(candidate, key, translation, sourceFile) {
const call = translateCall(key, translation.fallback, translation.options)
const node = findNodeByRange(sourceFile, candidate.start, candidate.end)
if (candidate.kind === 'jsx-text') {
return { start: candidate.start, end: candidate.end, text: `{${call}}` }
}
if (candidate.kind === 'jsx-expression') {
return { start: candidate.start, end: candidate.end, text: call }
}
if (candidate.kind.startsWith('jsx-attribute:')) {
if (node?.parent && ts.isJsxAttribute(node.parent) && node.parent.initializer === node) {
return { start: node.getStart(sourceFile), end: node.getEnd(), text: `{${call}}` }
}
if (node && isInsideJsxExpression(node)) {
return { start: candidate.start, end: candidate.end, text: call }
}
return { start: candidate.start, end: candidate.end, text: `{${call}}` }
}
return { start: candidate.start, end: candidate.end, text: call }
}
function sourceKindForPath(filePath) {
return filePath.endsWith('.tsx') || filePath.endsWith('.jsx')
? ts.ScriptKind.TSX
: ts.ScriptKind.TS
}
function findNodeByRange(sourceFile, start, end) {
let match
function visit(node) {
if (node.getStart(sourceFile) === start && node.getEnd() === end) {
match = node
return
}
ts.forEachChild(node, visit)
}
visit(sourceFile)
return match
}
function translationForCandidate(candidate, sourceFile) {
if (!candidate.dynamic) {
const fallback =
candidate.kind === 'jsx-text' || candidate.kind.startsWith('jsx-attribute:')
? decodeJsxEntities(candidate.text)
: candidate.text
return { fallback }
}
const node = findNodeByRange(sourceFile, candidate.start, candidate.end)
if (!node || !ts.isTemplateExpression(node)) {
return null
}
const options = {}
let fallback = node.head.text
node.templateSpans.forEach((span, index) => {
const optionName = `value${index}`
options[optionName] = span.expression.getText(sourceFile)
fallback += `{{${optionName}}}${span.literal.text}`
})
const optionSource = `{ ${Object.entries(options)
.map(([name, expression]) => `${name}: ${expression}`)
.join(', ')} }`
return { fallback, options: optionSource }
}
function hasTranslateImport(sourceText) {
return /import\s*\{[^}]*\bt\b[^}]*\}\s*from\s*['"]@\/i18n\/mobile-i18n['"]/.test(sourceText)
}
function addTranslateImport(sourceText) {
if (hasTranslateImport(sourceText)) {
return sourceText
}
const importMatches = [...sourceText.matchAll(/^import[\s\S]*?from\s*['"][^'"]+['"]\n/gm)]
if (importMatches.length === 0) {
return `${TRANSLATE_IMPORT}${sourceText}`
}
const lastImport = importMatches.at(-1)
const insertAt = (lastImport.index ?? 0) + lastImport[0].length
return `${sourceText.slice(0, insertAt)}${TRANSLATE_IMPORT}${sourceText.slice(insertAt)}`
}
function uniqueCandidates(candidates) {
const seen = new Set()
const unique = []
for (const candidate of candidates) {
const signature = `${candidate.start}:${candidate.end}:${candidate.kind}`
if (!seen.has(signature)) {
seen.add(signature)
unique.push(candidate)
}
}
return unique
}
function hasJsxDescendant(node) {
let found = false
function visit(child) {
if (ts.isJsxElement(child) || ts.isJsxSelfClosingElement(child) || ts.isJsxFragment(child)) {
found = true
return
}
ts.forEachChild(child, visit)
}
ts.forEachChild(node, visit)
return found
}
function mixedJsxTranslation(node, filePath, sourceFile) {
if (
!ts.isJsxElement(node) ||
node.children.some(
(child) =>
ts.isJsxElement(child) || ts.isJsxSelfClosingElement(child) || ts.isJsxFragment(child)
)
) {
return null
}
const options = []
let sourceFallback = ''
let hasText = false
let hasValue = false
for (const child of node.children) {
if (ts.isJsxText(child)) {
sourceFallback += child.text
hasText ||= /[A-Za-z\u00C0-\u024F\u3040-\u30ff\u3400-\u9fff\uac00-\ud7af]/.test(child.text)
continue
}
if (!ts.isJsxExpression(child) || !child.expression) {
continue
}
const expression = child.expression
if (ts.isStringLiteralLike(expression)) {
sourceFallback += expression.text
hasText ||= /[A-Za-z\u00C0-\u024F\u3040-\u30ff\u3400-\u9fff\uac00-\ud7af]/.test(
expression.text
)
continue
}
if (hasJsxDescendant(expression)) {
return null
}
const optionName = `value${options.length}`
sourceFallback += `{{${optionName}}}`
options.push([optionName, expression.getText(sourceFile)])
hasValue = true
}
const fallback = decodeJsxEntities(sourceFallback.replace(/\s+/g, ' ').trim())
if (!hasText || !hasValue || fallback.length < 2) {
return null
}
const optionsSource = `{ ${options
.map(([name, expression]) => `${name}: ${expression}`)
.join(', ')} }`
const key = keyForCandidate({ filePath, text: fallback })
return {
key,
fallback,
start: node.openingElement.getEnd(),
end: node.closingElement.getStart(sourceFile),
text: `{${translateCall(key, fallback, optionsSource)}}`
}
}
function collectMixedJsxTranslations(filePath, sourceFile) {
const translations = []
function visit(node) {
const translation = mixedJsxTranslation(node, filePath, sourceFile)
if (translation) {
translations.push(translation)
return
}
ts.forEachChild(node, visit)
}
visit(sourceFile)
return translations
}
function applyReplacements(filePath, sourceText, candidates, catalog) {
const sourceFile = ts.createSourceFile(
filePath,
sourceText,
ts.ScriptTarget.Latest,
true,
sourceKindForPath(filePath)
)
const mixedTranslations = collectMixedJsxTranslations(filePath, sourceFile)
const unique = uniqueCandidates(candidates)
const replacements = unique
.filter(
(candidate) =>
!mixedTranslations.some(
(translation) => candidate.start >= translation.start && candidate.end <= translation.end
) &&
!unique.some(
(other) =>
other !== candidate &&
other.start <= candidate.start &&
other.end >= candidate.end &&
(other.start < candidate.start || other.end > candidate.end)
)
)
.map((candidate) => ({
candidate,
translation: translationForCandidate(candidate, sourceFile)
}))
.filter((entry) => entry.translation !== null)
.sort((left, right) => right.candidate.start - left.candidate.start)
const edits = []
for (const { candidate, translation } of replacements) {
const key = keyForCandidate(candidate, translation.fallback)
setCatalogValue(catalog, key, translation.fallback)
const edit = editForCandidate(candidate, key, translation, sourceFile)
edits.push(edit)
}
for (const translation of mixedTranslations) {
setCatalogValue(catalog, translation.key, translation.fallback)
edits.push(translation)
}
let nextSource = sourceText
for (const edit of edits.sort((left, right) => right.start - left.start)) {
nextSource = `${nextSource.slice(0, edit.start)}${edit.text}${nextSource.slice(edit.end)}`
}
return edits.length > 0 ? addTranslateImport(nextSource) : nextSource
}
async function localizeFile(root, filePath, catalog) {
const sourceText = await fs.readFile(filePath, 'utf8')
const candidates = collectLocalizationCandidates(filePath, sourceText, root)
if (candidates.length === 0) {
return 0
}
const nextSource = applyReplacements(filePath, sourceText, candidates, catalog)
if (nextSource !== sourceText) {
await fs.writeFile(filePath, nextSource)
}
return uniqueCandidates(candidates).length
}
async function collectCandidateFiles(root) {
const reports = []
const stack = [path.join(root, 'mobile', 'app'), path.join(root, 'mobile', 'src')]
while (stack.length > 0) {
const dir = stack.pop()
const entries = await fs.readdir(dir, { withFileTypes: true })
for (const entry of entries) {
const fullPath = path.join(dir, entry.name)
if (entry.isDirectory()) {
if (!['assets', 'i18n', 'node_modules'].includes(entry.name)) {
stack.push(fullPath)
}
continue
}
if (
entry.isFile() &&
/\.(?:ts|tsx|js|jsx|mts|cts)$/.test(entry.name) &&
!entry.name.endsWith('.d.ts') &&
!entry.name.includes('.test.') &&
!entry.name.includes('.spec.')
) {
reports.push(fullPath)
}
}
}
return reports
}
export async function main(root = process.cwd()) {
const catalogPath = path.join(root, 'mobile', 'src', 'i18n', 'locales', 'en.json')
const catalog = JSON.parse(await fs.readFile(catalogPath, 'utf8'))
const files = await collectCandidateFiles(root)
let count = 0
for (const filePath of files) {
count += await localizeFile(root, filePath, catalog)
}
await fs.writeFile(catalogPath, `${JSON.stringify(catalog, null, 2)}\n`)
console.log(`Localized ${count} mobile string candidates.`)
return 0
}
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
process.exit(await main())
}
@@ -1,41 +0,0 @@
import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import path from 'node:path'
import { describe, expect, it, vi } from 'vitest'
import { main as localizeMobileStrings } from './localize-mobile-strings.mjs'
function makeProject() {
const root = mkdtempSync(path.join(tmpdir(), 'orca-localize-mobile-strings-'))
const appDirectory = path.join(root, 'mobile', 'app')
const sourceDirectory = path.join(root, 'mobile', 'src')
const localeDirectory = path.join(sourceDirectory, 'i18n', 'locales')
mkdirSync(appDirectory, { recursive: true })
mkdirSync(localeDirectory, { recursive: true })
writeFileSync(path.join(localeDirectory, 'en.json'), '{}\n', 'utf8')
return { root, appDirectory, localeDirectory }
}
describe('localize-mobile-strings', () => {
it('decodes JSX entities case-insensitively without changing JavaScript string literals', async () => {
const { root, appDirectory, localeDirectory } = makeProject()
writeFileSync(path.join(appDirectory, 'Literal.ts'), "export const label = 'Use &amp; here'\n")
writeFileSync(
path.join(appDirectory, 'Rendered.tsx'),
'export function Rendered() { return <Text>Use &AMP; here</Text> }\n'
)
const log = vi.spyOn(console, 'log').mockImplementation(() => undefined)
try {
await expect(localizeMobileStrings(root)).resolves.toBe(0)
} finally {
log.mockRestore()
}
const catalog = JSON.parse(readFileSync(path.join(localeDirectory, 'en.json'), 'utf8'))
expect(Object.values(catalog.m)).toEqual(
expect.arrayContaining(['Use &amp; here', 'Use & here'])
)
})
})
@@ -1,6 +1,11 @@
// TypeScript 7 is a native CLI; AST consumers still need the legacy JavaScript API.
import ts from 'typescript-api'
import {
isAssignedToRenderedVariable,
isRenderedJsxExpression
} from './mobile-localization-rendered-variable.mjs'
const USER_VISIBLE_JSX_ATTRIBUTES = new Set([
'ariaLabel',
'aria-label',
@@ -34,6 +39,7 @@ const USER_VISIBLE_OBJECT_KEYS = new Set([
'label',
'message',
'placeholder',
'subject',
'subtitle',
'title',
'toggleDescription',
@@ -216,49 +222,6 @@ function ancestorJsxAttributeName(node) {
return undefined
}
function isRenderedJsxExpression(node) {
let current = node.parent
while (current) {
if (ts.isJsxExpression(current)) {
return (
ts.isJsxElement(current.parent) ||
ts.isJsxFragment(current.parent) ||
ts.isJsxSelfClosingElement(current.parent)
)
}
if (
ts.isConditionalExpression(current) ||
ts.isParenthesizedExpression(current) ||
ts.isTemplateExpression(current) ||
ts.isNoSubstitutionTemplateLiteral(current)
) {
if (ts.isConditionalExpression(current) && current.condition === node) {
return false
}
current = current.parent
continue
}
if (ts.isBinaryExpression(current)) {
const operator = current.operatorToken.kind
if (
![
ts.SyntaxKind.PlusToken,
ts.SyntaxKind.AmpersandAmpersandToken,
ts.SyntaxKind.BarBarToken,
ts.SyntaxKind.QuestionQuestionToken
].includes(operator) ||
(operator !== ts.SyntaxKind.PlusToken && current.left === node)
) {
return false
}
current = current.parent
continue
}
return false
}
return false
}
function nearestObjectPropertyName(node) {
let current = node.parent
while (current) {
@@ -494,10 +457,13 @@ function isComparisonOperand(node) {
}
export function classifyMobileStringNode(node, userVisibleErrorSource) {
if (
hasAncestorObjectPropertyName(node, new Set(['className', 'classNames'])) ||
isComparisonOperand(node)
) {
if (hasAncestorObjectPropertyName(node, new Set(['className', 'classNames']))) {
return undefined
}
if (isAssignedToRenderedVariable(node)) {
return 'rendered-variable'
}
if (isComparisonOperand(node)) {
return undefined
}
@@ -40,4 +40,45 @@ export const HTML = \`<!doctype html>
'Follow up'
])
})
it('finds literals assigned to variables that later render in JSX', () => {
const source = `
export function Example({ alternate }) {
const copy = alternate ? 'Primary explanation' : 'Alternate explanation'
let followUp = 'Initial recovery note'
const fileLocation = \`\${path}:L\${line}\`
if (alternate) {
followUp = 'Updated recovery note'
}
return <><Text>{copy}</Text><Text>{followUp}</Text><Text>{fileLocation}</Text></>
}
`
const candidates = collectLocalizationCandidates(
'/repo/mobile/src/components/Example.tsx',
source,
'/repo'
)
expect(candidates.map((candidate) => candidate.text)).toEqual([
'Primary explanation',
'Alternate explanation',
'Initial recovery note',
'Updated recovery note'
])
})
it('finds user-visible subject fallbacks in returned rows', () => {
const source = `
export function toRow(item) {
return { subject: item.subject || '(no commit message)' }
}
`
const candidates = collectLocalizationCandidates(
'/repo/mobile/src/source-control/mobile-git-history.ts',
source,
'/repo'
)
expect(candidates.map((candidate) => candidate.text)).toEqual(['(no commit message)'])
})
})
@@ -0,0 +1,119 @@
// TypeScript 7 is a native CLI; AST consumers still need the legacy JavaScript API.
import ts from 'typescript-api'
function isDirectDisplayExpressionParent(parent, child) {
if (
ts.isParenthesizedExpression(parent) ||
ts.isAsExpression(parent) ||
ts.isSatisfiesExpression(parent)
) {
return parent.expression === child
}
if (ts.isConditionalExpression(parent)) {
return parent.whenTrue === child || parent.whenFalse === child
}
if (ts.isBinaryExpression(parent)) {
return [
ts.SyntaxKind.PlusToken,
ts.SyntaxKind.BarBarToken,
ts.SyntaxKind.QuestionQuestionToken
].includes(parent.operatorToken.kind)
}
return ts.isTemplateSpan(parent) || ts.isTemplateExpression(parent)
}
export function isRenderedJsxExpression(node) {
let current = node.parent
while (current) {
if (ts.isJsxExpression(current)) {
return (
ts.isJsxElement(current.parent) ||
ts.isJsxFragment(current.parent) ||
ts.isJsxSelfClosingElement(current.parent)
)
}
if (
ts.isConditionalExpression(current) ||
ts.isParenthesizedExpression(current) ||
ts.isTemplateExpression(current) ||
ts.isNoSubstitutionTemplateLiteral(current)
) {
if (ts.isConditionalExpression(current) && current.condition === node) {
return false
}
current = current.parent
continue
}
if (ts.isBinaryExpression(current)) {
const operator = current.operatorToken.kind
if (
![
ts.SyntaxKind.PlusToken,
ts.SyntaxKind.AmpersandAmpersandToken,
ts.SyntaxKind.BarBarToken,
ts.SyntaxKind.QuestionQuestionToken
].includes(operator) ||
(operator !== ts.SyntaxKind.PlusToken && current.left === node)
) {
return false
}
current = current.parent
continue
}
return false
}
return false
}
function assignedVariableName(node) {
let current = node
while (current.parent && isDirectDisplayExpressionParent(current.parent, current)) {
current = current.parent
}
const parent = current.parent
if (
parent &&
ts.isVariableDeclaration(parent) &&
parent.initializer === current &&
ts.isIdentifier(parent.name)
) {
return parent.name.text
}
return parent &&
ts.isBinaryExpression(parent) &&
parent.operatorToken.kind === ts.SyntaxKind.EqualsToken &&
parent.right === current &&
ts.isIdentifier(parent.left)
? parent.left.text
: undefined
}
export function isAssignedToRenderedVariable(node) {
if (ts.isTemplateExpression(node)) {
return false
}
const name = assignedVariableName(node)
if (!name) {
return false
}
let owner = node.parent
while (owner && !ts.isFunctionLike(owner) && !ts.isSourceFile(owner)) {
owner = owner.parent
}
if (!owner) {
return false
}
let rendered = false
function visit(current) {
if (rendered) {
return
}
if (ts.isIdentifier(current) && current.text === name && isRenderedJsxExpression(current)) {
rendered = true
return
}
ts.forEachChild(current, visit)
}
visit(owner)
return rendered
}
@@ -204,12 +204,42 @@ export function collectMobileTranslationCalls(filePath, sourceText, root = proce
sourceKind
)
const calls = []
const fixedTranslatorNames = new Set()
function isEnglishFixedTranslator(node) {
const expression = unwrapExpression(node)
return (
ts.isCallExpression(expression) &&
ts.isPropertyAccessExpression(expression.expression) &&
ts.isIdentifier(expression.expression.expression) &&
expression.expression.expression.text === 'mobileI18n' &&
expression.expression.name.text === 'getFixedT' &&
expression.arguments.length === 1 &&
ts.isStringLiteralLike(expression.arguments[0]) &&
expression.arguments[0].text === 'en'
)
}
function collectFixedTranslators(node) {
if (
ts.isVariableDeclaration(node) &&
ts.isIdentifier(node.name) &&
node.initializer &&
isEnglishFixedTranslator(node.initializer)
) {
fixedTranslatorNames.add(node.name.text)
}
ts.forEachChild(node, collectFixedTranslators)
}
collectFixedTranslators(sourceFile)
function visit(node) {
if (
ts.isCallExpression(node) &&
ts.isIdentifier(node.expression) &&
node.expression.text === 't'
((ts.isIdentifier(node.expression) &&
(node.expression.text === 't' || fixedTranslatorNames.has(node.expression.text))) ||
isEnglishFixedTranslator(node.expression))
) {
const keyExpression = node.arguments[0]
if (keyExpression) {
@@ -105,6 +105,16 @@ describe('verify-mobile-localization-catalog', () => {
expect(await runFailedVerification(root)).toContain('missing English key: m.missing')
})
it('reports missing keys used by an English fixed translator', async () => {
const root = makeProject({
sourceText:
"import { mobileI18n } from '@/i18n/mobile-i18n'\nconst canonicalLabel = mobileI18n.getFixedT('en')\nexport const label = canonicalLabel('m.missing')\n",
catalogs: { en: { m: { known: 'Known' } } }
})
expect(await runFailedVerification(root)).toContain('missing English key: m.missing')
})
it('rejects translation keys that cannot be statically inspected', async () => {
const root = makeProject({
sourceText: "import { t } from '@/i18n/mobile-i18n'\nexport const label = t(runtimeKey)\n",