`useTranslation` hook violates React's rules of hooks by conditionally calling inner hooks

[alespergl]

🐛 Bug Report

When useTranslation is called before initReactI18next, it returns early with the "not ready" result object. Later, after the init function gets called, the hook continues past that early return and calls other hooks (e.g. useState) which it didn't call before.

This behaviour is in violation of React's rules of hooks.

• Early return from useTranslation: src/useTranslation.js:53
• Call to useMemoizedT after early return: src/useTranslation.js:79

[adrai]

The question is why is useTranslation called before initReactI18next?

[alespergl]

@adrai That is a question, which doesn't invalidate the bug. Since useTranslation has a pretty explicit code path for this scenario, apparently it has an ambition to handle it correctly.

In the case where I encountered the issue, the reason for calling useTranslation before initialization is that there's a React component which renders during initialization. It monitors the ready value returned from the hook and only calls t when the value flips to true. If this is not meant to be supported, there probably is no point in having the ready state in the first place.

[jamuhl]

You can't use useTranslation before having init > so we could even throw for that case.

ready can be true|false depending on the namespace loaded already (like triggering suspense).

So, yes...react hooks has that warning - but for a good reason there is a conditional for the wrong usage case.

[adrai]

@alespergl this change might address this: 36d477f
but I'm not sure this will not break something else... like #1716, #1720 or similar...

// cc: @timheilman @medihack @jaredhan418 @schontz you may also have a look at this?

[alespergl]

@jamuhl In that case I think the current behaviour is confusing. It returns a valid value, giving an illusion that it's a supported scenario. If it's not, I would suggest to throw instead.

BTW from what I can tell, the the base i18next API can be safely called even before init - it just does nothing. It would be a nice consistency feature if react-i18next would work in similar way.

[adrai]

@alespergl can you test if this solves your issue?

[alespergl]

@adrai Sure, I will try it and will report back. Thanks.

[alespergl]

@adrai I tested the patch and it fixes the React violation. However, there seems to be another problem - when i18n gets initialized, the returned ready flag flips to true before the returned t function changes from notReadyT. In other words, there is a short period of time when the caller of useTranslation() receives { ready: true, t: notReadyT } which should never happen.

I tried to fix this by changing getT and getNewT to be dependent on ready instead of i18n, but that didn't help. It seems that both ready and t need to be calculated in-sync by the same logic, not separately as they are now.

[adrai]

@alespergl Can you try with this version: https://github.com/i18next/react-i18next/blob/try-fix-react-violation/src/useTranslation.js

These are the new changes: 5664416

[alespergl]

@adrai Yes, this one works as expected. The setForceUpdate logic seems a bit hacky, since I feel that taking a dependency on mount state or at which point an effect fires could be brittle, but I will leave that up to your judgement.
Thanks for the effort! :-)

[adrai]

@adrai Yes, this one works as expected. The setForceUpdate logic seems a bit hacky, since I feel that taking a dependency on mount state or at which point an effect fires could be brittle, but I will leave that up to your judgement. Thanks for the effort! :-)

ok, if so it would be nice if someone (@timheilman @medihack @jaredhan418 @schontz) could also have a look at it, so we can merge that and hope it does not break anything else...

[schontz]

I'm not sure why I was cc'd on this one, but it does sound vaguely familiar. I have not been able to test this.

If issues like this persist, one thing worth considering is using useSyncExternalStore. I've found it to be extremely useful for things that maintain their own lifecycle and/or are shared across hooks. Then I can put them in a class or similar container of logic and just notify my hooks via the subscription mechanism. I was dealing with some pretty bad edge case bugs on tiptap, and the author moved to an external store approach, which was much easier to reason with.

That said, if it ain't broke, don't fix it!

[schontz]

I would note that we did see some perf issues with useTranslation and have implemented a wrapper hook that performs more caching. Looking at the latest version of useTranslation.js, if it could take an external store approach, you could essentially limit most of that logic to running once (in a singleton of sorts) instead of in every component on every render. It adds up quick. And not having to solve your problem through the lens of memo/render is very nice. Libraries like redux take this approach, IIRC.

[adrai]

With this commit I tried useSyncExternalStore. In this branch you can find a version with the useSyncExternalStore approach.

Option 0 vs. Option 1 vs. Option 2

• Option 0: do nothing
• Option 1: try to address (edge case) violations
• Option 2: try to address (edge case) violations by using useSyncExternalStore => this will work only for React > v18 (or we need to use the shim => use-sync-external-store/shim)