Add language metadata and enhance language validation tests #2748
Conversation
WalkthroughThis PR introduces a metadata-driven architecture for language handling, moving from static embedded language data to dynamic async loading. A new metadata.json file centralizes language information, LangSelector.ts is refactored to load languages asynchronously with caching, and test coverage is updated to validate metadata consistency with resource files. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant LS as LangSelector
participant MetaFile as metadata.json
participant LangFiles as Language JSONs
participant FlagFiles as Flag SVGs
rect rgb(240, 248, 255)
Note over App,FlagFiles: Initialization Phase
App->>LS: initializeLanguage()
LS->>MetaFile: Load metadata.json
MetaFile-->>LS: Language metadata entries
par
LS->>LangFiles: Fetch user language JSON
LS->>LangFiles: Fetch default (en) JSON
and
Note over LS: Compute supported codes<br/>from metadata
end
LangFiles-->>LS: Translation data
LS->>LS: Cache translations
LS->>LS: Apply translations to UI
end
rect rgb(245, 245, 220)
Note over App,FlagFiles: Language Change Flow
App->>LS: changeLanguage(lang)
LS->>LS: getClosestSupportedLang()
LS->>LangFiles: loadLanguage(closestLang)
LangFiles-->>LS: Translation data
LS->>LS: Cache & apply translations
LS->>LS: Update languageList from metadata
LS->>LS: Re-render with new language
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-07-23T12:36:35.354ZApplied to files:
📚 Learning: 2025-06-02T14:27:37.609ZApplied to files:
🔇 Additional comments (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
tests/LangMetadata.test.ts (2)
10-23: Consider using explicit test skip instead of silent return.Using
console.logandreturnmakes the test appear as "passed" when metadata is missing or empty. If the file should always exist, consider failing the test. If skipping is intentional, use Jest's skip mechanism for clearer test reports.🔎 Option: Fail explicitly if metadata is required
if (!fs.existsSync(metadataFile)) { - console.log( - "No resources/lang/metadata.json file found. Skipping check.", - ); - return; + throw new Error("resources/lang/metadata.json is required but not found"); } const metadata = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); if (!Array.isArray(metadata) || metadata.length === 0) { - console.log( - "No language entries found in metadata.json. Skipping check.", - ); - return; + throw new Error("metadata.json must contain a non-empty array of languages"); }
17-17: Wrap JSON.parse in try-catch for clearer error on malformed JSON.If
metadata.jsoncontains invalid JSON, this will throw a generic parse error. A try-catch would let you provide a clearer failure message.🔎 Suggested improvement
- const metadata = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + let metadata: unknown; + try { + metadata = JSON.parse(fs.readFileSync(metadataFile, "utf-8")); + } catch (err) { + throw new Error(`Failed to parse metadata.json: ${err}`); + }src/client/LangSelector.ts (5)
44-65: Potential race condition on concurrent calls.If
getLanguageMetadata()is called twice before the first fetch completes, both calls will trigger a network request. Consider storing the pending promise to deduplicate concurrent requests.🔎 Suggested fix using promise caching
private languageMetadata: LanguageMetadata[] | null = null; + private languageMetadataPromise: Promise<LanguageMetadata[]> | null = null; private languageCache = new Map<string, Record<string, string>>(); private async getLanguageMetadata(): Promise<LanguageMetadata[]> { if (this.languageMetadata) return this.languageMetadata; + if (this.languageMetadataPromise) return this.languageMetadataPromise; + this.languageMetadataPromise = this.fetchLanguageMetadata(); + return this.languageMetadataPromise; + } + + private async fetchLanguageMetadata(): Promise<LanguageMetadata[]> { try { const response = await fetch("/lang/metadata.json"); // ... rest of implementation } catch (err) { console.error("Failed to load language metadata:", err); this.languageMetadata = []; } return this.languageMetadata; }
54-57: Type assertion trusts server response shape.The cast
as LanguageMetadata[]assumes the response matches the expected shape. Since metadata.json is a controlled static file, this is likely fine. For extra safety, you could validate that each entry has the required fields.
105-123: Same race condition pattern as metadata loading.Similar to
getLanguageMetadata, concurrent calls for the same language before the first completes will trigger duplicate fetches. Consider caching the pending promise in a Map.🔎 Suggested fix
private languageCache = new Map<string, Record<string, string>>(); + private languageLoadPromises = new Map<string, Promise<Record<string, string>>>(); private async loadLanguage(lang: string): Promise<Record<string, string>> { if (!lang) return {}; const cached = this.languageCache.get(lang); if (cached) return cached; + const pending = this.languageLoadPromises.get(lang); + if (pending) return pending; + const promise = this.fetchLanguage(lang); + this.languageLoadPromises.set(lang, promise); + return promise; + } + + private async fetchLanguage(lang: string): Promise<Record<string, string>> { try { const response = await fetch(`/lang/${encodeURIComponent(lang)}.json`); // ... rest } catch (err) { console.error(`Failed to load language ${lang}:`, err); return {}; + } finally { + this.languageLoadPromises.delete(lang); } }
17-17: Consider using typed union instead ofany[].
languageList: any[]loses type safety. You already haveLanguageMetadatatype - consider using it or a similar type for the list.🔎 Suggested fix
- @state() private languageList: any[] = []; + @state() private languageList: LanguageMetadata[] = [];Then update the debug entry to match the type, and remove other
anyannotations inloadLanguageList.
261-265: Modal opens before language list finishes loading.Setting
showModal = truebefore awaitingloadLanguageList()could briefly show an empty list. If the list is cached, this is likely fast enough. Consider loading before showing for smoother UX, or accept current behavior.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
resources/lang/metadata.jsonsrc/client/LangSelector.tstests/LangCode.test.tstests/LangMetadata.test.tstests/LangSvg.test.ts
💤 Files with no reviewable changes (2)
- tests/LangCode.test.ts
- tests/LangSvg.test.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/metadata.json
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
Repo: openfrontio/OpenFrontIO PR: 1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.
Applied to files:
resources/lang/metadata.json
📚 Learning: 2025-07-23T12:36:35.354Z
Learnt from: Aotumuri
Repo: openfrontio/OpenFrontIO PR: 1534
File: src/client/LangSelector.ts:97-106
Timestamp: 2025-07-23T12:36:35.354Z
Learning: In OpenFrontIO's LangSelector.ts, the getClosestSupportedLang method always joins language code parts with underscores ("_") because all keys in the languageMap use underscore format (e.g., pt_BR, sv_SE, zh_CN). This normalization ensures consistency regardless of whether the input language code uses hyphens or underscores as delimiters.
Applied to files:
src/client/LangSelector.ts
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
Applied to files:
src/client/LangSelector.ts
🧬 Code graph analysis (1)
tests/LangMetadata.test.ts (1)
eslint.config.js (1)
__dirname(10-10)
🔇 Additional comments (7)
resources/lang/metadata.json (1)
1-206: Structure looks good.The metadata file has a clean, consistent schema across all 35 language entries. Each entry correctly includes
code,native,en, andsvgfields. The test file will validate these against actual resource files.tests/LangMetadata.test.ts (1)
25-60: Good error collection pattern.Collecting all validation errors before failing with
expect(errors).toEqual([])is a clean approach. This shows all missing files at once rather than failing on the first issue.src/client/LangSelector.ts (5)
5-10: Clean type definition.The
LanguageMetadatatype matches the metadata.json structure well. Simple typed object, no class hierarchy - good choice.
67-83: Good language matching logic.The fallback to base code matching with preference for more specific codes (sorting by length) is a sensible approach. Returns "en" as safe default.
85-103: Good parallel loading pattern.Using
Promise.allto load default and user translations concurrently is efficient. The flow is clear: resolve language → load translations → apply.
186-192: Clean language switching implementation.Saves preference, loads translations, updates state, applies changes. Simple and correct.
312-331: Good recursive flattening implementation.Clean, pure function that handles nested translation objects. Warning on unexpected types helps catch data issues.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
src/client/LangSelector.ts
Outdated
| if (this.languageMetadata) return this.languageMetadata; | ||
|
|
||
| try { | ||
| const response = await fetch("/lang/metadata.json"); |
There was a problem hiding this comment.
I think /lang/metadata.json and en.json should be built into the bundle itself since we always need to fetch it anways.
…language handling
4 participants
Resolves #2739
Description:
Introduce language metadata handling and refactor existing language checks to validate the existence of language JSON and corresponding SVG files. Add tests to ensure the integrity of the new metadata structure and its references.
The lang field is intentionally kept in each language file.
This is because the files are frequently regenerated by Crowdin, and the field also serves as a hint for management and maintenance.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
aotumuri