Trans component only works if i18n is explicitly passed in

[isaachinman]

Following reports in #33 and #40, I've investigated the use of Trans in next-i18next projects, despite our codebase not touching it - the component comes directly from react-i18next.

It does appear there's a genuine issue here, and it appears it's related to context.

To explain very concisely:

{/* Works... */}
<Trans i18n={i18n} i18nKey='h1' />

{/* Does not work unless i18n is explicitly passed in */}
<Trans i18nKey='h1' />

I created a branch called trans-comp-broken-context to demonstrate the problem. Feel free to do a git checkout.

This is despite the Trans component definitely being inside a I18NextProvider component, which is successfully providing context for our withNamespaces HOC (just NamespacesConsumer under the hood):

Not sure what's different about the Trans component, and also not sure if this is the fault of next-i18next or not.

@jamuhl Sorry to bother, but do you possibly have an idea?

[jamuhl]

@isaachinman i guess it's not related to next-i18next - but not yet sure why this happens...there is some wild context providing/consuming by the nature of an optional Provider (each NamespaceConsumer acts as a Provider too - so might be somewhere it gets messy with updating: overall using the reactI18nextModule before i18next.init seems more reliable.)

Will need to create a pure react sample and try to reproduce. Currently i'm already on my xmas holidays but will try to get on this asap.

[isaachinman]

@jamuhl I can try to reproduce with just React as well. This is a server side exception, though. Do you think there might be multiple i18n instances somehow, and that's the root of the problem?

Enjoy your holidays first and foremost!

[jamuhl]

just tried to reproduce here: https://github.com/i18next/react-i18next/blob/master/example/react/src/index.js#L14 with changing to using the provider and removing use reactI18nextModule...but no luck...like you said might be a serverside thing only...

[isaachinman]

Probably related to #54.

@jamuhl I'm not sure how "expert" you feel you are with React, but we could use a second opinion!

[isaachinman]

I've reverted the tree-walking approach with v0.11.0, but am still seeing this bug.

Here's the specific exception:

TypeError: Cannot read property 't' of undefined
    at TransComponent.render

The stack trace points to:

var t = tFromContextAndProps || i18n.t.bind(i18n);

So it seems this tFromContextAndProps is not defined, thus falling back to i18n, which is also undefined. It definitely appears to be a context issue, despite there being a provider further up the tree.

[kachkaev]

Interestingly, but <Tans /> component works for me in https://gitlab.com/kachkaev/website-frontend/merge_requests/6 (<Tans /> worked well even in next-i18next@0.8.0). That's a TypeScript project, which suggests some discrepancies between commonjs modules and esnext modules somewhere in i18next ecosystem.

react-i18next has multiple exports, the same is for i18next itself. There's probably a hidden glitch in the ream of module.exports(.default?) somewhere 🤔

[jamuhl]

@kachkaev that can be...not easy to get es modules, commonjs and typescript under one hat.

[isaachinman]

Where does the issue lie? Inside next-i18next or react-i18next?

[jamuhl]

in typescript 😂 - no somewhere in i18next, react-i18next - but sadly no typescript user ever could tell me what to do to get it right for them without breaking the world for non typescript users...

[isaachinman]

@jamuhl Did you see the broken branch I created? It's not working in the next-i18next example, and that contains zero typescript.

[jamuhl]

@isaachinman yes...but could that not reproduce on non serverside -> so not really sure where it comes from?!?? as v10 will completely remove using context i guess this won't be a problem there...might currently be safer to .use(reactI18nextModule) instead of Provider - as i'm sure the issue is related to context...but never got to deep into why.

[isaachinman]

might currently be safer to .use(reactI18nextModule) instead of Provider

You mean when initialising the i18next instance? Is there an example of this somewhere? How would this get passed to NamespacesConsumer and Trans without a provider?

[jamuhl]

sample: https://github.com/i18next/react-i18next/blob/master/example/react/src/i18n.js#L15

it's a set/get (https://github.com/i18next/react-i18next/blob/master/src/context.js#L26)...only drawback you can't have multiple instances in one app

[isaachinman]

Yes, but how do we handle initialLanguage and initialI18nStore?

[isaachinman]

It seems like doing .use doesn't fix the Cannot read property 't' of undefined error anyways.

[jamuhl]

Yes, but how do we handle initialLanguage and initialI18nStore?

Hm...that is done in the Provider - my guess is as it only gets missed somewhere deeper in the tree out of unknown reason doing both is safe

[isaachinman]

Okay. When does v10 land, do you think?

[jamuhl]

fully depends on https://reactjs.org/blog/2018/11/27/react-16-roadmap.html 16.(7)

[isaachinman]

Alright well, I've got no idea what the problem is here. I am 95% sure it's a context bug that is within the react-i18next source, but for the time being, I have written a quick export of the Trans component from next-i18next that binds the correct i18n instance, getting around the problem (a2c09ea).

For users that want to use Trans, the next-i18next constructor now returns that component as well.

[kachkaev]

Using a version of <Trans /> that's provided by next-i18next is the right way to go IMO, so thanks for solving the problem this way @isaachinman! If the library docs do not require to install i18next / react-i18next packages, there should be no imports directly from them such as for Trans (otherwise, linters won't like it). In future, next-i18next can just return the original Trans component if the context issue gets resolved upstream, but next-i18next users would not have to do anything with this implementation detail.

I shared similar thoughts in #42 – next-i18next provides all required bits and pieces in that proposal too.