feat(i18n): add native Lunaria key merging#1777
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
yanthomasdev
changed the title
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR removes many Lunaria locale JSON files under lunaria/files, simplifies CONTRIBUTING.md by deleting the manual translation-tracking copy step, removes the exported Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (1)
lunaria.config.ts (1)
27-36: Consider removing the non-null assertion for stricter type safety.Although
baseLangoriginates fromObject.keys()of the same object (making it safe in practice), the!assertion at line 31 can be replaced with a guard for consistency with the project's strict typing guidelines.♻️ Proposed refactor
for (const baseLang of Object.keys(countryLocaleVariants)) { if (baseLang === sourceLocale.lang) continue if (!localeSet.has(baseLang)) { - // Use the first variant's name or the base code as label - const variants = countryLocaleVariants[baseLang]! - const label = variants[0]?.name ?? baseLang + const variants = countryLocaleVariants[baseLang] + const label = variants?.[0]?.name ?? baseLang localeSet.add(baseLang) locales.push({ label, lang: baseLang }) } }As per coding guidelines: "Ensure you write strictly type-safe code".
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
CONTRIBUTING.mdconfig/i18n.tslunaria.config.tslunaria/files/ar-EG.jsonlunaria/files/az-AZ.jsonlunaria/files/bg-BG.jsonlunaria/files/bn-IN.jsonlunaria/files/cs-CZ.jsonlunaria/files/de-DE.jsonlunaria/files/en-GB.jsonlunaria/files/en-US.jsonlunaria/files/es-419.jsonlunaria/files/es-ES.jsonlunaria/files/fr-FR.jsonlunaria/files/hi-IN.jsonlunaria/files/hu-HU.jsonlunaria/files/id-ID.jsonlunaria/files/it-IT.jsonlunaria/files/ja-JP.jsonlunaria/files/mr-IN.jsonlunaria/files/nb-NO.jsonlunaria/files/ne-NP.jsonlunaria/files/pl-PL.jsonlunaria/files/pt-BR.jsonlunaria/files/ru-RU.jsonlunaria/files/ta-IN.jsonlunaria/files/te-IN.jsonlunaria/files/uk-UA.jsonlunaria/files/zh-CN.jsonlunaria/files/zh-TW.jsonlunaria/lunaria.tslunaria/prepare-json-files.tspackage.jsonscripts/compare-translations.ts
💤 Files with no reviewable changes (21)
- lunaria/files/id-ID.json
- lunaria/files/ja-JP.json
- lunaria/files/bn-IN.json
- lunaria/files/hi-IN.json
- lunaria/files/az-AZ.json
- lunaria/files/cs-CZ.json
- lunaria/files/en-GB.json
- lunaria/files/es-419.json
- lunaria/files/de-DE.json
- lunaria/files/ar-EG.json
- lunaria/files/nb-NO.json
- lunaria/files/es-ES.json
- config/i18n.ts
- lunaria/files/fr-FR.json
- lunaria/files/it-IT.json
- lunaria/files/en-US.json
- lunaria/files/hu-HU.json
- lunaria/files/ne-NP.json
- lunaria/files/mr-IN.json
- lunaria/files/pl-PL.json
- lunaria/files/bg-BG.json
|
The inconsistency between list and table seems to be this expression: https://github.com/yanthomasdev/npmx.dev/blob/3f2dd7a329c97f4c984f02bb0938489c56ab1d95/lunaria/components.ts#L267 Because the list view just uses raw JSON files to determine whether keys are missing, but the table views uses the raw Lunaria status, which includes git timestamps. So alto I'm not totally sure how Lunaria is supposed to use all the available info in the different views, but I would suggest extracting the logic for the correct status into a separate function, so it can consistently be reused. |
# Conflicts: # lunaria/files/az-AZ.json # lunaria/files/bg-BG.json # lunaria/files/cs-CZ.json # lunaria/files/de-DE.json # lunaria/files/en-GB.json # lunaria/files/en-US.json # lunaria/files/es-419.json # lunaria/files/es-ES.json # lunaria/files/fr-FR.json # lunaria/files/hu-HU.json # lunaria/files/id-ID.json # lunaria/files/it-IT.json # lunaria/files/ja-JP.json # lunaria/files/pl-PL.json # lunaria/files/ru-RU.json # lunaria/files/uk-UA.json # lunaria/files/zh-CN.json # lunaria/files/zh-TW.json # package.json
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lunaria/components.ts (1)
270-283: Extract a shared status resolver to avoid future table/list mismatches.The status branching added here is valid, but similar rules are also implemented in
LocaleDetailsandSvgLocaleSummary. Centralising this logic would prevent drift.Refactor sketch
+type DisplayStatus = 'missing' | 'outdated' | 'up-to-date' + +function resolveDisplayStatus( + localization: StatusEntry['localizations'][number], + fileType?: string, +): DisplayStatus { + const isMissingKeys = 'missingKeys' in localization && localization.missingKeys.length > 0 + if (fileType === 'dictionary') { + if (localization.status === 'missing') return 'missing' + return isMissingKeys ? 'outdated' : 'up-to-date' + } + return isMissingKeys ? 'outdated' : localization.status +} + export const TableContentStatus = ( localizations: StatusEntry['localizations'], lang: string, lunaria: LunariaInstance, fileType?: string, ): string => { const localization = localizations.find(localization => localization.lang === lang)! - const isMissingKeys = 'missingKeys' in localization && localization.missingKeys.length > 0 - const status = - fileType === 'dictionary' - ? isMissingKeys - ? 'outdated' - : localization.status === 'missing' - ? 'missing' - : 'up-to-date' - : isMissingKeys - ? 'outdated' - : localization.status + const status = resolveDisplayStatus(localization, fileType) const links = lunaria.gitHostingLinks() const link = status === 'missing' ? links.create(localization.path) : links.source(localization.path) return html`<td>${EmojiFileLink(link, status)}</td>` }
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CONTRIBUTING.mdapp/composables/useI18nStatus.tsconfig/i18n.tslunaria/components.tspackage.json
💤 Files with no reviewable changes (1)
- config/i18n.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- CONTRIBUTING.md
# Conflicts: # lunaria/files/az-AZ.json # lunaria/files/bg-BG.json # lunaria/files/cs-CZ.json # lunaria/files/de-DE.json # lunaria/files/en-GB.json # lunaria/files/en-US.json # lunaria/files/es-419.json # lunaria/files/es-ES.json # lunaria/files/fr-FR.json # lunaria/files/hu-HU.json # lunaria/files/id-ID.json # lunaria/files/ja-JP.json # lunaria/files/pl-PL.json # lunaria/files/ru-RU.json # lunaria/files/tr-TR.json # lunaria/files/uk-UA.json # lunaria/files/zh-CN.json # lunaria/files/zh-TW.json
|
thank you so much @yanthomasdev ❤️❤️❤️ |
4 participants
🔗 Linked issue
🧭 Context
Before, we were using a custom system to add key merging from base locale to variants (e.g. en -> en-US -> en-GB, etc). This PR uses the equivalent feature recently added to Lunaria to do it instead.
📚 Description
It solves the issue we were having with the confusing setup of both
i18nandlunaria/filesthat was creating confusion among contributors.This PR completely removes
lunaria/filesand changes the other i18n scripts to adapt to the new structure.Note: there's a small issue in the dashboard that is making en-US and en-GB be considered complete in the dashboard's progress by locale but not in the table (i.e. the outdated icon -> 🔄), I'll have to look into this yet, but if someone is able to look before me, I appreciate it.
We'll also need to look into every i18n file and see if there's nothing we are missing from other PRs.