Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i18next-json-sync used in i18n:sync-keys script removes correct plurals from translations #175

Closed
javnik36 opened this issue Feb 24, 2025 · 8 comments · Fixed by #180
Closed
Labels
bug Something isn't working

Comments

@javnik36
Copy link
Contributor

Not the bug with the app itself, but worth noting imo. Running i18n:sync-keys script removes correct plural forms from translations.

E.g. polish has 3 plural forms - so adding them manually to the translation json and running the script afterwards will remove them if they were never present in English original. Git diff as example:
Image

Not entirely sure what is going wrong in this case - my manual additions, module used in the script or something else 😆 .

@javnik36 javnik36 added the bug Something isn't working label Feb 24, 2025
@fspoettel
Copy link
Owner

Good catch, seems like this is an issue with the cli we use: jwbay/i18next-json-sync#22 I'll check out the alternative mentioned in the thread and see if we can migrate to it.

@fspoettel
Copy link
Owner

fspoettel commented Feb 25, 2025

The alternative is even worse in a brief test 😅 ... guess we need to roll our own here.

@fspoettel
Copy link
Owner

@javnik36 I created a little script to do this for us in #180. Feel free to give it a test and lmk if this works for you.

@javnik36
Copy link
Contributor Author

Ok, I dug deep into this while checking your new script's output...maybe too deep :D Sorry for massive amount of text...

First of all, I will write from my perspective what I would expect while syncing original English file to my locale:

  • script knows what plurals suffixes my locale uses and therefore:
    • adds keys for all suffixes my locale uses for given key (regardless of how many they are specified in original),
    • removes keys for suffixes my locale don't use - also removes key without a suffix (afaik used as fallback translation instead switching to English original - imo, it will be hard to spot that mistake later on)
  • (bonus: throws warnings when there's missing plural form in original)

So, for example:

#en.json
"key1_one": "one",
"key2_one": "two",
"key2_other": "two_other",
"nonplural_key": "nonplkey"
#pl.json
"key1_one": "one_pl",
"key1_few": "one_few_pl",
"key1_many": "one_many_pl",
"key1_other": "one_other_pl",
"key2": "two_pl",
"key2_other": "two_other_pl"

I expect after syncing:

#pl.json
"key1_one": "one_pl",
"key1_few": "one_few_pl",
"key1_many": "one_many_pl",
"key1_other": "one_other_pl",
-"key2": "two_pl",
+"key2_one": "two",
+"key2_few": "two_other",
+"key2_many": "two_other",
"key2_other": "two_other_pl",
+"nonplural_key": "nonplkey"

I was looking for this behavior on locales there are right now available on the app on i18-locales-sync and script from #180. I didn't check original i18-json-sync script, as we already know it sucks xD

For me it looks like the i18-locales-sync almost fit my expectations (of course except bonus warning), but for some reason it doesn't like korean - it always removed all keys that have with plurals (according to this korean has 1 form - so I would expect either create/leave forms with "_other" suffix or without any suffix - reported this as felixmosh/i18next-locales-sync#40). Another downside is that list of synced languages must be specified in the command, so it will require a little attention while introducing a new translation.

#180 script as far I can tell, leaves all translated suffixes as is, but do not add them based on target locale.

Of course it is up to expectations - your script is undoubtedly better than the original i18-json-sync, as does not removes good stuff. It is up to you what's next - I would propose merging #180 immediately and maybe switch to i18-locales-sync long term - if my issue ends up being an issue and will be fixed 😄 .

@javnik36
Copy link
Contributor Author

Oh shoot, I did took me so long that you already did that 😅 Hope that whatever I wrote is still somehow relevant xD

@fspoettel
Copy link
Owner

@javnik36 yeah it's relevant, thanks for the deep dive and opening an issue on the locales project. It mangled the ko locale for me as well, but I did not look into it further. 😅

I see your point re: creating the plurals automatically, that would be ideal. Can you ping back here once the issue gets solved upstream? I think the DX issue with adding locales we can solve quite easily with a wrapper script that reads the LOCALES constant.

@fspoettel
Copy link
Owner

btw in the mean-time, another PR was opened that relates to this #181 - yesterday I learned that russian plurals are quite special, so I think this is relevant for more locales than pl.

@javnik36
Copy link
Contributor Author

Sure thing, will let you know.

Oh yeah, I discovered today that formally in pl we have more plurals than I though - in some cases plural for fraction numbers can be different than any other 😅 (though I have no idea if 18next supports that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants