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

Is it really possible to access t() from outside a React component the way it's described? #1757

Open
igorsantos07 opened this issue May 30, 2024 · 7 comments

Comments

@igorsantos07
Copy link

igorsantos07 commented May 30, 2024

Documentation issue

I noticed a couple of issues with the config section of the Step-by-step guide:

  • [more of a question/suggestion] does the example config file must really re-export i18n which was got "raw" from i18next? Wouldn't it be simpler to just import i18n from 'i18next' everywhere you want that?
  • and....... would it even work? Right now, my application is screaming badly at me, stating that "something IS WRONG in my setup". First, it makes me feel sad to be screamed at 😆 Second, it took me a while to figure it all out, as I explain below, but when I find (one of the many) culprits, I get back to docs to check the correct way to do it... And I realize I AM doing what's suggested. At the bottom of that section, there's an info box stating you can simply import i18next and t()-away. But... that doesn't work that easy, given i18next.init() is async, right? What should be the correct way to do it outside React components - e.g. an API model class, or maybe a Redux action, in the (unlikely but possible) scenario of a race condition between the translation and init() resolving? Or, maybe, even using it raw like in the exact example (which is precisely what I do in a lot of places, as initially I thought it was fine).
  • lastly, a bonus: the codesandbox linked in the README is 404'ing 👀

Motivation

Simply put, I wrote something stupid in my codebase and only realized it caused a mysterious bug quite later. Then I postponed it for, IDK, two years?? And now I'm looking into it.

I even tried creating a codesandbox (forking from an ancient one I had) to display the bug on i18next, but then, with the simpler codebase, I realized the bug was typed by my own keyboard: even IF there wasn't a race condition between i18next.init() async nature and my calls, further language changes would never change the translated text 🤡

And here I am.

To be more specific about my main "problem", before I i18next'ed everything, there were some components with auxiliary text placed outside the component - e.g. tab titles used in actual tabs and a shortcuts sidebar. I'll try to move them all into the component, and maybe "lazify" it with a function call that can be awaited, somehow. I mean, there's no way to even fire a suspense event from outside a component, right? hahah

In case you want to see the stupid result of my codesandbox which cannot translate the title once the language changes, have a free laugh here. But it doesn't seem to cause the screaming warning I mentioned, probably because it doesn't use(resourcesToBackend([...])), as in my scenario, so init() always wins the race.

@jamuhl
Copy link
Member

jamuhl commented May 30, 2024

a) you can just import i18n from 'i18next makes no difference...only thing needed is calling i18next.init which does not happen if there is no import of the i18n.js file

b) your app is screaming at you because "someting IS WRONG" and enough devs in the past believed they did everything right (believing is not knowing)

c) using outside of a component...would say best solution is avoiding it as much as possible

@adrai idk...we get this question a lot. Devs reasonable struggle to solve the async nature challenge of i18next if using it outside of components. It is a challenge to solve - my guess it's even unsolvable 100% right without avoiding doing it and only translate in the UI renderer:

first idea: use an async function

const myTranslatedObject = await i18next.ready(t => ({ foo: t('bar') }));
  • solves waiting for init
  • does not update on language change
  • does not really work if used in some file that get's imported...(there is no async export foo)

We would have to do something like: #1058 (comment)

import i18n from '../../../i18n';

const data = [
  {
    code: 'ew.bad-request#storageSettings#storage-full',
    message: i18n.t('employee-details.messages.photo-update-error')
  },
];

i18next.on('languageChanged', function(lng) {
   data.message = i18n.t('employee-details.messages.photo-update-error')
})

export default data;
  • update possible on init, language change
  • initial import might contain no translations
  • language change event might get triggered after language changed gets triggered for UI (possible race condition)
  • not immutable
  • no notification that the data changed

So I really struggle to suggest a solution to this beside telling don't do it - only use t inside components...everywhere else use keys that get passed to t as late as possible (rendering, sending email, ...)

d) codesandbox...might be a deleted codesandbox (might be a result of introduced limit of free sandboxes)...thank you for reporting

@jamuhl
Copy link
Member

jamuhl commented May 30, 2024

image

I agree that this is misleading by using the word simply

@jamuhl
Copy link
Member

jamuhl commented May 30, 2024

removed the codesandbox link in readme

@igorsantos07
Copy link
Author

igorsantos07 commented May 30, 2024

a) if there's no need to export it, I guess I can submit a PR simplifying that part of the example config? I mean, it must import the config file otherwise it won't even configure, but there's no need to import i18next from the config file. It requires some extra braincycles from the dev to understand it is redundant - specially when the info box states you should import "your exported i18next instance", but... it's not even an instance, right?

b) yep, I just mentioned the screaming error as a reference to what message I was talking about :) not complaing about it, I thought it would be clear from the grimacing emoji. Sorry for the misunderstanding on the joke.

c) And yep, from my new understanding it looks impossible to use i18next outside of React if you don't have global await enabled, or all your use-cases are fine with promises. The biggest side effect of doing it that way, and that might take a while to notice, is that it won't update with language changes, given that's tied to events that are not mentioned in the React documentation (what makes sense, and what a React dev will mostly be reading, besides the raw i18next config).

Overall, I would indeed suggest simply removing that info box from the docs, or replacing it with a warning of the side-effects of doing it that way. It's not just a problem with "simply importing", it is that importing and using that way will cause subtle and unexpected errors... Unless you use extra stuff that increase the time init() takes to wrap - which is my case, and it still mostly works, just causes a screaming warning. For instance, it took me two years to realize some small bits of the UI weren't being translated in some cases.

Bottom-line: people think they're doing everything right because the docs tell them to do something that won't really work as it seems so.

d) sad on the codesandbox. It was really useful!


And thanks for the fast response!

@jamuhl
Copy link
Member

jamuhl commented May 31, 2024

a) it's the way we use to do it - as it's a matter of preference - neither option seems to be preferable

I believe it should be simple enough to understand that:

// File A
import i18n from 'i18next';

export default i18n;

// File B
import i18n1 from 'i18next';
import i18n2 from '.i18n';

i18n1 === i18n2

specially when the info box states you should import "your exported i18next instance", but... it's not even an instance, right?

Sure it is an instance https://github.com/i18next/i18next/blob/master/src/i18next.js#L631 we decided 12 years ago that most devs only will need one instance -> so per default exported an instance instead of a function to create an instance

b) No worry...did only want to make a point the console.log of i18next is doing what it is doing

c) Agree...best to update that info box and eventually add an extra section beside the useTranslation section that explains the problems of using outside of components

d) might come back if we find a codesandbox that is the pure initial sample (wanted to remove that deadlink and had no time to search)

@jamuhl
Copy link
Member

jamuhl commented May 31, 2024

Just had your last comment in the email...and was thinking about the code posted there...

let TAP = 'tap'
let CLICK = 'click'
i18next.on('languageChanged', () => {
	TAP = i18next.t('tap')
	CLICK = i18next.t('click')
})
//[...]
export function clickOrTap() {
	return IS_MOBILE? TAP : CLICK
}

First thought we could provide some helper like const myMemTrans = buildATranslatedObject([ 'key1', 'key2' ]) it does the update on languageChange. We could even add some support for nested values or interpolation...myMemTrans.get(key, options)

Then I realised - that is more or less what i18next.t already does...

So your sample could just be:

export function clickOrTap() {
	return IS_MOBILE? i18next.t('tap') : i18next.t('click')
}

I mean we do hundreds of calls to t each render - does it hurt to call it in that file...

@VIKTORVAV99
Copy link
Contributor

The way we are doing it over at Electricity Maps is by passing the t function as a parameter to the function that need it outside of react components. That way we can get it like normal from a hook, pass it as a prop and use it as we normally would.

Like this:

function thatNeedT(t) {
  return t('some.translation-string')
}

function ReactComponent(){
  const {t} = useTranslation()
  const translatedString = thatNeedT(t);
  return (
    <div>
      {translatedString}
    </div>
  )
}

Not sure if that is of any help here but it seems like a simple solution to, and if you have any feedback why we should not use it like this I'd be very interested in that as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants