feat: move i18n configuration #263
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
serhalp
left a comment
serhalp
left a comment
There was a problem hiding this comment.
🙌🏼 Thank you! Ship it. Some suggestions inline.
Are there changes we should make here to https://github.com/npmx-dev/npmx.dev/blob/main/CONTRIBUTING.md#localization-i18n? It doesn't mention Lunaria atm, so we should probably add discoverable, actionable instructions there.
Would you like me to merge this and follow up in another PR?
| for (let i = 1; i < locale.files.length; i++) { | ||
| currentSource = await loadJsonFile(locale.files[i] as string) |
There was a problem hiding this comment.
nit:
| for (let i = 1; i < locale.files.length; i++) { | |
| currentSource = await loadJsonFile(locale.files[i] as string) | |
| for (const file of locale.files.slice(1)) { | |
| currentSource = await loadJsonFile(file) |
There was a problem hiding this comment.
Slice will create a new array, the code is the same, feel free to apply suggestion if you prefer that notation, I prefer imperative over functional
| "@intlify/core-base": "^11.1.12", | ||
| "@intlify/shared": "^11.2.8", |
There was a problem hiding this comment.
nit: these can be dev deps, right?
There was a problem hiding this comment.
We dont use it at runtime so can be a dev dependency (only in future pr for types)
| import { currentLocales } from '../config/i18n.ts' | ||
| import { deepCopy } from '@intlify/shared' | ||
|
|
||
| const destFolder = path.resolve('lunaria/files') |
There was a problem hiding this comment.
suggestion: how about we add this path to .gitattributes to mark it as containing generated files? https://docs.github.com/en/repositories/working-with-files/managing-files/customizing-how-changed-files-appear-on-github
diffs will get collapsed automatically on GitHub
| @@ -0,0 +1,545 @@ | |||
| { | |||
There was a problem hiding this comment.
@userquin is it possible to use .json5 or .jsonc with lunaria? it would be great to include a comment at the top of generated files, indicating that it's a generated file and giving a clue as to where to look to make i18n contributions
There was a problem hiding this comment.
Iirc we can use [jt]s, json, json5 or yaml, I need to review the docs
There was a problem hiding this comment.
Beware about lunaria, I have bo idea if can handle other fornats
There was a problem hiding this comment.
And we should update the lunaria logic to merge json5 files, maybe adding some external deps (until lunaria supports files entry)
| ...data, | ||
| code: l.code, | ||
| name: l.name, | ||
| files: [data.file as string, `${l.code}.json`], |
There was a problem hiding this comment.
🤔 why is this type assertion necessary?
There was a problem hiding this comment.
File can be an string or an object, we're using string
I need to update the contributing file adding some hints from elk.zone: we have some entries for i18n, rtl and unocss rules. I will send the changes once we have this pr merged, maybe tomorrow. https://github.com/elk-zone/elk/blob/main/CONTRIBUTING.md
Yes, I was talking with @danielroe on discord, we need to check how to solve html valitator and hydration missmath errors once we store the language on localstorage at client side to restorr it on page refresh: since now we dont persist locale from settings page, we lost the locale on page refresh (and so no missmatch errors). Once we restore locale, we will have client missmatch errors until we fix hydration strategy, Daniel has some idea to fix it. |
3 participants
This PR includes:
rtl,numberFormats,dateTimeFormatsandpluralRulees-ESandes-419