search

gilbsgilbs / babel-plugin-i18next-extract

Babel plugin that statically extracts i18next and react-i18next translation keys.
https://i18next-extract.netlify.com
MIT License
161 stars 37 forks source link

Fix processing of spaces in Trans component. #114

Closed gilbsgilbs closed 4 years ago

gilbsgilbs commented 4 years ago

Previously, if a JSXText node in a Trans component consisted in only one whitespace, the extractor would strip it and ignore it for the indices increment. This is wrong as react-i18next does take them into account. However, react-i18next indeed ignores JSXText nodes that consist in only whitespaces AND contains at list one linefeed. This commit makes the extractor reflect the behavior of react-i18next.

Fixes #113

@pcorpet it appears that it was not a regression introduced in #111. This bug existed from the very beginning and is not related to basic html nodes. Thanks for bringing it to my attention. Could you have a look at my fix?

gilbsgilbs commented 4 years ago

I think it would be nice to share some of your test files with react-i18next, to make sure they stay in sync. I don't have a quick way of doing that, I was just raising the thought.

Yes, that also applies to i18next actually. I sometimes notice regressions either because some test was all wrong from the beginning or because [react-]i18next did a (subtle) breaking change at some point. We currently have no automatic way of finding such regression because I couldn't find a way to implement it without weakening the tests in respect of parsing. One solution I had in mind at the time was to create a custom backend for i18next that would extract the keys directly from the test files (using the JS runtime we have in Jest) so that we can compare them with the keys obtained through the plugin. Yet, this solution had the downside of preventing you to make use of any type of complex structure (such as conditions or functions) which you do want to test in a parser.

Therefore it would be possible to have such test, but might require way more work that I'm currently able to provide in my spare time. I think the more the plugin will be used, the more we'll tend to a status quo situation where we know (from the community) that the tests (and as the consequence the plugin) do reflect the reality. Not a perfect solution, but probably good enough (unless somebody wants to improve :angel:). That's why bug reports such as #113 matters :+1: .

gilbsgilbs commented 4 years ago

Thanks for the prompt review btw. Just released this on NPM (v0.4.2).