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-icu interoperability with Trans #439

Closed
beheh opened this issue May 20, 2018 · 13 comments
Closed

i18next-icu interoperability with Trans #439

beheh opened this issue May 20, 2018 · 13 comments

Comments

@beheh
Copy link

@beheh beheh commented May 20, 2018

We have a project where we use react-i18next, i18next-icu and key-based fallbacks. This means we extract keys from code as <Trans>Hello world!</Trans> to a language file in the following fashion: {"Hello World!": "Hallo Welt!"}.

This mostly worked fine until we introduced i18next-icu. While usage of t works as expected (i.e. t("You have {msgCount, plural, one {# message} other {# messages}}")), I'm having issues with getting the Trans to work. Specifically <Trans>some {{data}}</Trans> leads to a the translateable string some {{data}}, which is invalid in ICU (throws a syntax error due to the unexpected second opening brace).

I tried running with <Trans>some {data}</Trans>, but obviously the variable resolution fails here without the object syntax, so I think I'll definitely need to use the double brace syntax. As I understand this is just a quirk of how react-i18next works, so I'm interested in getting th

This leaves me with three places this could be fixed:

  1. Make react-i18next request valid ICU strings, with an option or by autodetecting it. If enabled, <Trans>some {{stuff}}</Trans> should resolve to/lookup some {stuff}. If this would be accepted I'd be happy to work on a PR.
  2. Make i18next-icu tolerate i18next's default syntax (so while my app might request t(some {{stuff}}), it would rewrite it internally to some {stuff} before running the ICU parser). In that case I would close the issue here and work with i18next-icu.
  3. Come up with my own compatability layer inbetween as a separate package. I think this is the ideal way (as this is a very special case) but I don't know how I'd hook this up or whether it's even possible. Could one introduce any kind of hooks into react-18next to provide this behaviour?

Thoughts?

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 21, 2018

correct some {{thing}} won't be valid for mixing Trans and ICU as those are incompatible ways to interpolate -> this is specific to i18next.

You will need to pass options to t function via prop tOptions for ICU like: https://github.com/i18next/react-i18next/blob/master/example/react_icu_withHOC/src/App.js#L29

With string: https://github.com/i18next/react-i18next/blob/master/example/react_icu_withHOC/public/locales/en/translations.json#L8

@beheh

This comment has been minimized.

Copy link
Author

@beheh beheh commented May 21, 2018

Yeah, I saw that last example, but in our case this doesn't work with our solution to key-based fallbacks as we use the Trans children as the key. Our codebase would contain snippets such as:

t("some {thing}")

and there's simply no such way to do that with Trans, as

<Trans>some {thing}</Trans>

results in the incorrect key some <0></0>.

Furthermore,

<Trans>There are {numPersons, plural, =0 {no persons} =1 {one person} other {# persons}}</Trans>

simply results in invalid JSX.

For me this is just seems to be an incompatability between ICU + Trans + key-based fallback, and I'd like to find a solution (at the very least for basic interpolation).

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 21, 2018

hm...might need some different key generation. Would need to test more...but currently lack the time.

What if you escape the curly brackets?? facebook/react#1545

@beheh

This comment has been minimized.

Copy link
Author

@beheh beheh commented May 28, 2018

The issue is the opposite - I need "real" double curbly brackets for react-i18next to pick up the variable name (<Trans>some {{param}}</Trans>, which is <Trans>some {{param: param}}</Trans>). react-i18next sees the children "some " and an object with a single key "param", which is then resolved to the key some {{param}}:

if (format && keys.length === 1) {
mem = `${mem}<${elementKey}>{{${keys[0]}, ${format}}}</${elementKey}>`;
} else if (keys.length === 1) {
mem = `${mem}<${elementKey}>{{${keys[0]}}}</${elementKey}>`;
} else if (console && console.warn) {
// not a valid interpolation object (can only contain one value plus format)
console.warn(`react-i18next: the passed in object contained more than one variable - the object should look like {{ value, format }} where format is optional.`, child)
}

For react-icu one would need a way to override this to use single brackets instead.

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 29, 2018

so you want to have jsx <Trans>some {{thing}}</Trans> and react-i18next Trans component to write a defaultValue of some {thing} and we would have to extract all the vars and pass it to icu like in the v2 mode: phiresky@422fd56#diff-e2b8d52a3ffb2c5b61049216c69ff761R51

what i would do is:

jsx: <Trans tOptions={{thing}}>some {'{thing}'}</Trans> to escape the curly brackets

i see no other way. Doing a transform of double curly won't work as you could not write complex plural statements that way...only other option would be the creation of some babel-transform which takes <Trans>some {thing}</Trans> and transforms it to <Trans defaultValue="some {thing}" vars={{ thing }} components={[]} />

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 29, 2018

Awesome would be to create babel-macro a https://github.com/kentcdodds/babel-plugin-macros/ doing that conversion

but would take some time or better someone with skills writing babel-plugins

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jun 4, 2018

@beheh sorry for the late update...was blocked with other topics.

with v7.7.0 you now can:

<Trans defaults="There are {numPersons, plural, =0 {no persons} =1 {one person} other {# persons}}" values={{ numPersons: 10 }} />
// -> There are 10 persons

or interpolate:

<Trans defaults="some {thing}" values={{ thing: 'awesomeness' }} />
// -> some awesomeness

or components:

<Trans defaults="hello <0>{{what}}</0>" values={{ what: 'world'}} components={[<strong>univers</strong>]}> />
// -> hello <strong>world</strong>

from here it should be rather easy to create a babel makro converting

<TransMakro values={{ thing: 'awesomeness' }}>some {thing}</TransMakro>

to

<Trans defaults="some {thing}" values={{ thing: 'awesomeness' }} />
@kachkaev

This comment has been minimized.

Copy link

@kachkaev kachkaev commented Jun 5, 2018

@beheh I've been using <Trans/> with i18nKey prop and it worked (see example). This requires manual editing of locales/DEFAULT_LANG/namespace.json though, but it's not too hard and you get neat keys as a bonus.

I guess that unless there is some babel transform, defaults prop will propagate to the production copy of the website, which will double the size of translations in it. Nevertheless, it's great that this prop now exists! Thanks @jamuhl!

@beheh

This comment has been minimized.

Copy link
Author

@beheh beheh commented Jun 6, 2018

This is great, thanks! I think we might just be skipping the babel plugin here and using the defaults prop. I guess the only thing that's weird now is mapping the index to the number.

I did notice that I need to fill the components with some children for it to render, I'd expected something like <Trans defaults="Hellow <0>World</0>" components={[<a href="#" />]}> to just work. I guess I'll file another issue for that.

Great work!

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jun 7, 2018

Yes...those components needs childrens right now - might optimize that in future so another issue for that might be a good idea.

Wound't hurt as the Trans component used this way works rather different from the way having the content in the childrens (jsx tree) as the indexes for those are different. But guess thats ok.

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jul 28, 2018

@beheh @kachkaev just made a non-so-quick try and created a babel-plugin-macros (https://github.com/kentcdodds/babel-plugin-macros/blob/master/other/docs/author.md) for using Trans and ICU:

Just a quick and dirty implementation for a macro: https://github.com/i18next/react-i18next/blob/master/example/react_icu_withHOC/src/Trans.macro.js

Usage: https://github.com/i18next/react-i18next/blob/master/example/react_icu_withHOC/src/ComponentUsingMacro.js

let's you basically write something like:

<Trans>Trainers: <strong>{ trainersCount, number }</strong>!</Trans>

<Select
        switch={gender}
        male={<Trans><strong>He</strong> avoids bugs.</Trans>}
        female={<Trans><strong>She</strong> avoids bugs.</Trans>}
        other={<Trans><strong>They</strong> avoid bugs.</Trans>}
/>

<Plural
        count={itemsCount1}
        $0="There is no item."
        one="There is # item."
        other="There are # items."
/>

Visual Result:
screen shot 2018-07-28 at 17 40 29

Before cleaning that up feedback would be very welcome...

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jul 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.