mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(client): stop requiring plural categories no count can select
The locale validator demanded every CLDR plural category, so each new ICU plural key took the daily Crowdin sync red until a `many` branch was hand-added for Spanish and French. That branch can never render: `many` selects at a million and over in Spanish, French, Italian and Portuguese, and only for fractional counts in Czech and Slovak, while every catalog plural counts whole items well below a million and intl-messageformat falls back to `other` regardless. Require only the categories an integer count 0..1000 can select, which is the same set every integer below a million selects, and keep `other` always so the categories everyday counts reach stay mandatory. Drop a target message that still fails validation from the sparse catalog instead of failing the whole export, so one broken string can no longer block the other 23 languages for days. Report the rejections in the run log and the pull request body, and keep the retention guard fatal when a rejected message would replace a translation that is already in Git.
This commit is contained in:
@@ -15,6 +15,18 @@ export function flattenCatalog(value, prefix = '', output = new Map()) {
|
||||
return output
|
||||
}
|
||||
|
||||
function messageErrors({ key, locale, message, referenceMessage, slotCountKeys }) {
|
||||
if (referenceMessage === undefined) return [`${locale}: unexpected key ${key}`]
|
||||
|
||||
const errors = []
|
||||
if (message.length === 0) errors.push(`${locale}: empty message ${key}`)
|
||||
if (message.includes('\u2014')) errors.push(`${locale}: Unicode em dash is not allowed in ${key}`)
|
||||
if (/<[^>]+>/.test(message)) errors.push(`${locale}: HTML is not allowed in ${key}`)
|
||||
errors.push(...validateLocaleMessage({ key, locale, message, referenceMessage }))
|
||||
if (slotCountKeys.has(key)) errors.push(...validateSlotCountMessage({ key, locale, message }))
|
||||
return errors
|
||||
}
|
||||
|
||||
export function validateCatalogs({ catalogs, referencedKeys = new Set(), slotCountKeys = new Set() }) {
|
||||
const reference = catalogs.get('en')
|
||||
if (!reference) throw new Error('English reference catalog is required')
|
||||
@@ -26,18 +38,25 @@ export function validateCatalogs({ catalogs, referencedKeys = new Set(), slotCou
|
||||
|
||||
for (const [locale, catalog] of catalogs) {
|
||||
for (const [key, message] of catalog) {
|
||||
const referenceMessage = reference.get(key)
|
||||
if (referenceMessage === undefined) {
|
||||
errors.push(`${locale}: unexpected key ${key}`)
|
||||
continue
|
||||
}
|
||||
if (message.length === 0) errors.push(`${locale}: empty message ${key}`)
|
||||
if (message.includes('\u2014')) errors.push(`${locale}: Unicode em dash is not allowed in ${key}`)
|
||||
if (/<[^>]+>/.test(message)) errors.push(`${locale}: HTML is not allowed in ${key}`)
|
||||
errors.push(...validateLocaleMessage({ key, locale, message, referenceMessage }))
|
||||
if (slotCountKeys.has(key)) errors.push(...validateSlotCountMessage({ key, locale, message }))
|
||||
errors.push(...messageErrors({ key, locale, message, referenceMessage: reference.get(key), slotCountKeys }))
|
||||
}
|
||||
}
|
||||
|
||||
return errors
|
||||
}
|
||||
|
||||
export function findInvalidTargetMessages({ catalogs, slotCountKeys = new Set() }) {
|
||||
const reference = catalogs.get('en')
|
||||
if (!reference) throw new Error('English reference catalog is required')
|
||||
|
||||
const invalid = []
|
||||
for (const [locale, catalog] of catalogs) {
|
||||
if (locale === 'en') continue
|
||||
for (const [key, message] of catalog) {
|
||||
const errors = messageErrors({ key, locale, message, referenceMessage: reference.get(key), slotCountKeys })
|
||||
if (errors.length > 0) invalid.push({ locale, key, errors })
|
||||
}
|
||||
}
|
||||
|
||||
return invalid
|
||||
}
|
||||
|
||||
@@ -36,9 +36,6 @@ describe('locale catalog validation', () => {
|
||||
books: { count: '{count, plural, one {# kniha} other {# knih}}' },
|
||||
}
|
||||
|
||||
expect(validateCatalogs({ catalogs: catalogs(target) })).toEqual([
|
||||
'cs: ICU plural category few missing for books.count',
|
||||
'cs: ICU plural category many missing for books.count',
|
||||
])
|
||||
expect(validateCatalogs({ catalogs: catalogs(target) })).toEqual(['cs: ICU plural category few missing for books.count'])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -101,8 +101,17 @@ export function analyzeIcuMessage(message) {
|
||||
}
|
||||
}
|
||||
|
||||
// Every plural in the catalogs counts whole items, and a library tops out well below a million.
|
||||
// CLDR still lists categories that only fractional counts (Czech and Slovak `many`) or counts of a
|
||||
// million and over (Spanish, French, Italian and Portuguese `many`) can ever select, so demanding a
|
||||
// translation for them rejects otherwise correct Crowdin exports for a branch that cannot render.
|
||||
// Sampling 0..1000 selects exactly the categories every integer below a million selects.
|
||||
const REACHABLE_COUNT_SAMPLE = Array.from({ length: 1001 }, (_, index) => index)
|
||||
|
||||
function pluralCategories(locale, type) {
|
||||
return new Intl.PluralRules(locale, { type }).resolvedOptions().pluralCategories
|
||||
const rules = new Intl.PluralRules(locale, { type })
|
||||
const reachable = new Set(REACHABLE_COUNT_SAMPLE.map((count) => rules.select(count)))
|
||||
return rules.resolvedOptions().pluralCategories.filter((category) => reachable.has(category))
|
||||
}
|
||||
|
||||
function exactSelectors(selectors) {
|
||||
|
||||
@@ -86,6 +86,27 @@ describe('locale message validation', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('accepts cardinal categories that only fractional or million-scale counts select', () => {
|
||||
const reference = '{count, plural, one {# book} other {# books}}'
|
||||
|
||||
expect(validate('es', reference, '{count, plural, one {# libro} other {# libros}}')).toEqual([])
|
||||
expect(validate('fr', reference, '{count, plural, one {# livre} other {# livres}}')).toEqual([])
|
||||
expect(validate('cs', reference, '{count, plural, one {# kniha} few {# knihy} other {# knih}}')).toEqual([])
|
||||
expect(validate('es', reference, '{count, plural, one {# libro} many {# de libros} other {# libros}}')).toEqual([])
|
||||
})
|
||||
|
||||
it('still requires categories that everyday counts select', () => {
|
||||
const reference = '{count, plural, one {# book} other {# books}}'
|
||||
|
||||
expect(validate('ru', reference, '{count, plural, one {# книга} other {# книги}}')).toEqual([
|
||||
'ru: ICU plural category few missing for books.count',
|
||||
'ru: ICU plural category many missing for books.count',
|
||||
])
|
||||
expect(validate('cs', reference, '{count, plural, one {# kniha} other {# knih}}')).toEqual([
|
||||
'cs: ICU plural category few missing for books.count',
|
||||
])
|
||||
})
|
||||
|
||||
it('reports invalid ICU and rejects legacy branches for migrated keys', () => {
|
||||
const reference = '{count, plural, one {One book} other {# books}}'
|
||||
|
||||
|
||||
@@ -4,7 +4,7 @@ import { isIP } from 'node:net'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
import path from 'node:path'
|
||||
import { TARGET_CATALOGS, assertCrowdinTargetConfiguration } from './locale-configuration.mjs'
|
||||
import { flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs'
|
||||
import { findInvalidTargetMessages, flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs'
|
||||
|
||||
const API = 'https://api.crowdin.com/api/v2'
|
||||
const SOURCE_PATH_SUFFIX = '/client/src/locales/en.json'
|
||||
@@ -260,7 +260,7 @@ export function parseAllowedTranslationLosses(value = '') {
|
||||
return allowed
|
||||
}
|
||||
|
||||
export function findTranslationLosses({ locale, reference, current, exported }) {
|
||||
export function findTranslationLosses({ locale, reference, current, exported, rejected = new Map() }) {
|
||||
const legacyComplete = current.size === reference.size && [...reference.keys()].every((key) => current.has(key))
|
||||
const losses = []
|
||||
|
||||
@@ -270,7 +270,8 @@ export function findTranslationLosses({ locale, reference, current, exported })
|
||||
|
||||
const exportedMessage = exported.get(key)
|
||||
if (exportedMessage === undefined) {
|
||||
losses.push({ locale, key, reason: 'missing from Crowdin export' })
|
||||
const rejection = rejected.get(key)
|
||||
losses.push({ locale, key, reason: rejection ? `rejected by catalog validation - ${rejection[0]}` : 'missing from Crowdin export' })
|
||||
} else if (currentMessage !== referenceMessage && exportedMessage === referenceMessage) {
|
||||
losses.push({ locale, key, reason: 'replaced by English source text' })
|
||||
}
|
||||
@@ -285,13 +286,20 @@ export function assertTranslationRetention({
|
||||
exportedCatalogs,
|
||||
allowedLosses = new Set(),
|
||||
targetCatalogs = TARGET_CATALOGS,
|
||||
rejections = [],
|
||||
}) {
|
||||
const rejectedByLocale = new Map()
|
||||
for (const { locale, key, errors } of rejections) {
|
||||
if (!rejectedByLocale.has(locale)) rejectedByLocale.set(locale, new Map())
|
||||
rejectedByLocale.get(locale).set(key, errors)
|
||||
}
|
||||
|
||||
const losses = []
|
||||
for (const { locale } of targetCatalogs) {
|
||||
const current = currentCatalogs.get(locale)
|
||||
const exported = exportedCatalogs.get(locale)
|
||||
if (!current || !exported) throw new Error(`Translation retention comparison is missing the ${locale} catalog`)
|
||||
losses.push(...findTranslationLosses({ locale, reference, current, exported }))
|
||||
losses.push(...findTranslationLosses({ locale, reference, current, exported, rejected: rejectedByLocale.get(locale) }))
|
||||
}
|
||||
|
||||
const detected = new Set(losses.map(({ locale, key }) => `${locale}:${key}`))
|
||||
@@ -308,6 +316,35 @@ export function assertTranslationRetention({
|
||||
throw new Error(`Crowdin export would lose existing translations:\n${details.join('\n')}`)
|
||||
}
|
||||
|
||||
const MAX_REPORTED_REJECTIONS = 50
|
||||
|
||||
export function formatRejectionReport(rejections) {
|
||||
if (rejections.length === 0) return ''
|
||||
|
||||
const listed = rejections.slice(0, MAX_REPORTED_REJECTIONS)
|
||||
const lines = [
|
||||
`### Rejected Crowdin messages (${rejections.length})`,
|
||||
'',
|
||||
'These translations did not pass catalog validation and were omitted, so the English source renders instead. Fix them in Crowdin.',
|
||||
'',
|
||||
...listed.map(({ errors }) => `- ${errors[0]}`),
|
||||
]
|
||||
if (rejections.length > listed.length) lines.push(`- ...and ${rejections.length - listed.length} more`)
|
||||
return `${lines.join('\n')}\n`
|
||||
}
|
||||
|
||||
async function reportRejections(rejections, reportPath) {
|
||||
if (rejections.length === 0) {
|
||||
if (reportPath) await writeFile(reportPath, '')
|
||||
return
|
||||
}
|
||||
|
||||
console.log(`Rejected ${rejections.length} invalid Crowdin messages; the English source renders instead:`)
|
||||
for (const { errors } of rejections.slice(0, MAX_REPORTED_REJECTIONS)) console.log(` ${errors[0]}`)
|
||||
if (rejections.length > MAX_REPORTED_REJECTIONS) console.log(` ...and ${rejections.length - MAX_REPORTED_REJECTIONS} more`)
|
||||
if (reportPath) await writeFile(reportPath, formatRejectionReport(rejections))
|
||||
}
|
||||
|
||||
export async function syncCrowdinTranslations({
|
||||
token,
|
||||
projectId = '912891',
|
||||
@@ -317,6 +354,7 @@ export async function syncCrowdinTranslations({
|
||||
allowedLosses = new Set(),
|
||||
targetCatalogs = TARGET_CATALOGS,
|
||||
assertTargetConfiguration = assertCrowdinTargetConfiguration,
|
||||
reportPath = process.env.CROWDIN_REJECTION_REPORT || '',
|
||||
}) {
|
||||
if (!token) throw new Error('CROWDIN_TOKEN is required')
|
||||
await assertTargetConfiguration()
|
||||
@@ -350,6 +388,13 @@ export async function syncCrowdinTranslations({
|
||||
const catalogs = new Map([['en', referenceMessages]])
|
||||
for (const { locale, catalog } of downloaded) catalogs.set(locale, flattenCatalog(catalog))
|
||||
|
||||
const rejections = findInvalidTargetMessages({ catalogs })
|
||||
const rejectedLocales = new Set(rejections.map(({ locale }) => locale))
|
||||
for (const { locale, key } of rejections) catalogs.get(locale).delete(key)
|
||||
for (const entry of downloaded) {
|
||||
if (rejectedLocales.has(entry.locale)) entry.catalog = orderedSparseCatalog(reference, catalogs.get(entry.locale))
|
||||
}
|
||||
|
||||
const errors = validateCatalogs({ catalogs })
|
||||
if (errors.length > 0) throw new Error(`Crowdin export validation failed:\n${errors.join('\n')}`)
|
||||
assertTranslationRetention({
|
||||
@@ -358,13 +403,16 @@ export async function syncCrowdinTranslations({
|
||||
exportedCatalogs: catalogs,
|
||||
allowedLosses,
|
||||
targetCatalogs,
|
||||
rejections,
|
||||
})
|
||||
|
||||
await mkdir(outputDirectory, { recursive: true })
|
||||
await Promise.all(
|
||||
downloaded.map(({ locale, catalog }) => writeFile(path.join(outputDirectory, `${locale}.json`), `${JSON.stringify(catalog, null, 2)}\n`)),
|
||||
)
|
||||
await reportRejections(rejections, reportPath)
|
||||
console.log(`Synchronized ${downloaded.length} sparse translation catalogs from Crowdin`)
|
||||
return { rejections }
|
||||
}
|
||||
|
||||
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
|
||||
|
||||
@@ -284,10 +284,42 @@ describe('Crowdin translation synchronization', () => {
|
||||
await expect(access(outputDirectory)).rejects.toThrow()
|
||||
})
|
||||
|
||||
it('validates every downloaded catalog before writing any files', async () => {
|
||||
it('omits an invalid downloaded message instead of failing the whole export', async () => {
|
||||
const targetCatalogs = TARGET_CATALOGS.slice(0, 2)
|
||||
const catalogDirectory = await createCatalogFixture(targetCatalogs)
|
||||
const outputDirectory = path.join(catalogDirectory, 'output')
|
||||
const reportPath = path.join(catalogDirectory, 'rejections.md')
|
||||
const catalogs = new Map([[targetCatalogs[0].languageId, { common: { save: 'Bad \u2014 value' } }]])
|
||||
|
||||
const { rejections } = await syncCrowdinTranslations({
|
||||
token: 'secret',
|
||||
fetchImpl: createSynchronizationFetch({ catalogs }),
|
||||
catalogDirectory,
|
||||
outputDirectory,
|
||||
targetCatalogs,
|
||||
reportPath,
|
||||
assertTargetConfiguration: async () => {},
|
||||
})
|
||||
|
||||
expect(rejections).toEqual([
|
||||
{
|
||||
locale: targetCatalogs[0].locale,
|
||||
key: 'common.save',
|
||||
errors: [`${targetCatalogs[0].locale}: Unicode em dash is not allowed in common.save`],
|
||||
},
|
||||
])
|
||||
expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[0].locale}.json`), 'utf8'))).toEqual({})
|
||||
expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[1].locale}.json`), 'utf8'))).toEqual({
|
||||
common: { save: `Translated ${targetCatalogs[1].languageId}` },
|
||||
})
|
||||
expect(await readFile(reportPath, 'utf8')).toContain('Unicode em dash is not allowed in common.save')
|
||||
})
|
||||
|
||||
it('fails when a rejected message would drop an existing translation', async () => {
|
||||
const targetCatalogs = TARGET_CATALOGS.slice(0, 2)
|
||||
const currentCatalogs = new Map([[targetCatalogs[0].locale, { common: { save: 'Ulozit' } }]])
|
||||
const catalogDirectory = await createCatalogFixture(targetCatalogs, currentCatalogs)
|
||||
const outputDirectory = path.join(catalogDirectory, 'output')
|
||||
const catalogs = new Map([[targetCatalogs[0].languageId, { common: { save: 'Bad \u2014 value' } }]])
|
||||
|
||||
await expect(
|
||||
@@ -299,7 +331,7 @@ describe('Crowdin translation synchronization', () => {
|
||||
targetCatalogs,
|
||||
assertTargetConfiguration: async () => {},
|
||||
}),
|
||||
).rejects.toThrow('Unicode em dash is not allowed')
|
||||
).rejects.toThrow('rejected by catalog validation')
|
||||
await expect(access(outputDirectory)).rejects.toThrow()
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user