i18next / i18next-parser

Parse your code to extract translation keys/values and manage your catalog files
MIT License
486 stars 198 forks source link

Fix unescape logic, expression props, and ICU format for Trans component #892

Closed nicegamer7 closed 1 year ago

nicegamer7 commented 1 year ago

Why am I submitting this PR

After #871, it seems I introduced a regression, though it did help find a bug.

It seems that react-i18next does indeed default the key to the defaults prop (I added a comment to prevent anyone from making the same mistake again). I've reverted the changes from #871 to match react-i18next's behavior. After reporting this on react-i18next's bug tracker (i18next/react-i18next#1663), it looks like this was unintended. I fixed that, so no need to revert #871.

Otherwise, it seems we had a bug where we were unescaping values in reverse (i.e. unescaping when it wasn't needed, and not unescaping when it was needed). I corrected this, and also made it so that we always unescape the key, since this matches react-i18next's behavior as well.

Does it fix an existing ticket?

Closes #886.

Checklist

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% :tada:

Comparison is base (5fdd776) 93.95% compared to head (e7c7325) 93.97%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #892 +/- ## ========================================== + Coverage 93.95% 93.97% +0.01% ========================================== Files 11 11 Lines 1836 1841 +5 ========================================== + Hits 1725 1730 +5 Misses 111 111 ``` | [Files Changed](https://app.codecov.io/gh/i18next/i18next-parser/pull/892?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next) | Coverage Δ | | |---|---|---| | [src/lexers/jsx-lexer.js](https://app.codecov.io/gh/i18next/i18next-parser/pull/892?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next#diff-c3JjL2xleGVycy9qc3gtbGV4ZXIuanM=) | `98.49% <100.00%> (+0.02%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nicegamer7 commented 1 year ago

Actually, I'm going to put this on hold until i18next/react-i18next#1663 is resolved, since it might mean we don't have to revert #871.

edit: i18next/react-i18next#1663 was fixed, so there's no need to revert #871 anymore.

nicegamer7 commented 1 year ago

Hey @karellm, I just pushed another commit that fixes two more issues that I noticed. The two issues are:

  1. Extracting Trans components with the ICU format was broken
  2. Extracting props with object/array expressions was broken

Take a look when you have a chance and let me know if there are any concerns!

karellm commented 1 year ago

LGTM! Thanks for the great work and it is now deployed as 8.7.0