i18next / react-i18next

Internationalization for react done right. Using the i18next i18n ecosystem.
https://react.i18next.com
MIT License
9.25k stars 1.02k forks source link

Issue with React 16.13: Cannot update a component [ X] while rendering a different component [Y]. #1124

Closed ifmael closed 4 years ago

ifmael commented 4 years ago

🐛 Bug Report

React show a warning when a function component is updated during another component's render phase. https://fb.me/setstate-in-render

To Reproduce

The codesandbox to reproduce the issue:

Expected behavior

I don't expect to see a warning in the console

Your Environment

jamuhl commented 4 years ago

Looks like more related to your sample than to what react-i18next provides?

ifmael commented 4 years ago

If I remove const { t } = useTranslation(); from my components and use normal string , it doesn't happen. In this message https://github.com/facebook/react/issues/18178#issuecomment-595846312 , gaeron shows how to identify the source of the issue.

i18n-issue

In my component locale-provier.js, I only change the locale based on the page context

import React from 'react'
import LocaleContext from './locale-context'
import { useTranslation } from "react-i18next"

const LocaleProvider = ({ children, locale}) =>{
  const { i18n } = useTranslation()

  i18n.language !== locale && i18n.changeLanguage(locale);

  return <LocaleContext.Provider value={locale}>{ children }</LocaleContext.Provider>
}

export default LocaleProvider
jamuhl commented 4 years ago

Still, the problem does not exist in our samples...so it's specific to your setup, configuration...

ifmael commented 4 years ago

Ok. How can i identify where is the problem? Because I don't have idea what is it going on

jamuhl commented 4 years ago

try:

chopfitzroy commented 4 years ago

@ifmael any luck? I am seeing this to, is to do with isMounted being changed in the useTranslation hook during a render.

jamuhl commented 4 years ago

@CrashyBang isMounted is set based on useRef -> https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L44 so no setState no rerender

Can you provide a reproducible sample on codesandbox - really would love to get this solved.

ifmael commented 4 years ago

@jamuhl I've tried everything that you told me but I get the same error. The error only shows the first time that I changed the language. All subsequent navigations work correctly

jamuhl commented 4 years ago

Can't reproduce it on a non-gatsby sample - and won't invest hours to debug through the provided sample.

ifmael commented 4 years ago

Finally, I've solved my issue, but to be honest, I'm not pretty sure why the below code solves the problem:

import React, { useEffect } from 'react'
import LocaleContext from './locale-context'
import { useTranslation } from "react-i18next"

const LocaleProvider = ({ children, locale}) =>{
  const { i18n } = useTranslation()

  useEffect( () =>{
    i18n.language !== locale && i18n.changeLanguage(locale);
  }, [locale]);

  return <LocaleContext.Provider value={locale}>{ children }</LocaleContext.Provider>
}

export default LocaleProvider

It's like changeLanguage method had some side-effect (probably changes the shared context), is it?

jamuhl commented 4 years ago

changeLanguage has no side-effects (it's not even react-i18next) -> it sets language on the i18next instance and sends an event that is used in useTranslation to trigger a rerender

I'm not even sure why you need a LocaleProvider...but anyway...if it works...it works

ifmael commented 4 years ago

Maybe I don't need it...it's the first time I'm working with i18n and language so I am a little confused. I'm building a blog site with gatsby + sanity.io. Also, there are several pages that I generate from static fille inside gatsby (I'm only using sanity to write the post) like index, contact, about me. From each file, I generate 2 pages (one for Spanish and another for English) with different slug and pass it a specific locale for each one.

With this locale, I feed the LocaleProvider Component in order to get in its children component the current locale for the website. Probably, there is a better approach to do that.

jamuhl commented 4 years ago

All useTranslation, withTranslation also give you the i18n instance -> i18n.language and you have the current language

ifmael commented 4 years ago

Nice!. Thanks!

an-parubets commented 4 years ago

For my same case of problem, i was resolved it using useSuspense: false like:

const { t } = useTranslation('my-namespace', {
    useSuspense: false,
});

Thank!

rmedaer commented 4 years ago

I got the same issue while doing some test with react@experimental.

I fixed the issue by removing the line 88 of src/useTranslation.js:

if (isMounted.current) setT(getT());

If I well understand the flow, it's not mandatory. Indeed (with useSuspense: true), when the namespace is loaded, it will resolve the promise and react will go through useTranslation function again. Then the hasLoadedNamespace will be true and everything will be ready.

Btw I ~maybe~ probably missed something. I'm not familiar with react-i18next code.

jamuhl commented 4 years ago

@rmedaer might indeed not be needed...honestly, I never tested if it works without that line...as of the time of writing that code Suspense was still rather new.

If you like you can provide a PR removing that line...

rmedaer commented 4 years ago

Hi there,

You probably saw the fix and PR I did yesterday. I double checked and I can confirm it fix my issues (same setState-in-render error).

Could you test the release 11.7.2 and confirm it works as well on your side ? @ifmael @an-parubets

Kind regards,

re-thc commented 4 years ago

This change breaks with an error t is not a function

rmedaer commented 4 years ago

@hc-codersatlas could you give us a minimal piece of code which reproduce the issue. Currently the tests are passing. If there is indeed an issue, it will help us to debug and we will be able to add a new test in the suite.

jamuhl commented 4 years ago

@hc-codersatlas any update on this...not sure how getting a t is not a function is possible at all...so a codesandbox for reproduction would be awesome  🙏

re-thc commented 4 years ago

Bumped the version number from 11.7.1 to 11.7.2 and it came up. Haven't been able to replicate it as a separate project yet.

jamuhl commented 4 years ago

should be closed by https://github.com/i18next/react-i18next/pull/1165

trumbitta commented 4 years ago

Even with v11.7.2, still had to set useSuspense to false to get rid of the warning. But I don't really want to handle the issues mentioned in https://react.i18next.com/latest/usetranslation-hook#not-using-suspense

I'm not sure what my next steps should be...

jamuhl commented 4 years ago

@trumbitta a codesandbox with reproduction case might be a good first step

trumbitta commented 4 years ago

@jamuhl my stacktrace lead to code similar to the one #1165 addresses, but in a different part of the file:

    function boundReset() {
      if (isMounted.current) setT(getT());
    }
jamuhl commented 4 years ago

still...this not yet happened ever to me...so how should I fix something that is not reproducible...

trumbitta commented 4 years ago

@jamuhl I hear you, and I will do my best to come back with a reproduction... meanwhile, going further with my investigations, if I disable the localStorage backend then the warning goes away.

Still not a feasible solution for my case, but at least I have one more clue for what to look for while I try to put a reproduction together.

cichelero commented 4 years ago

I'm also experiencing the same issue, with stacktrace also pointing to boundReset.

trumbitta commented 4 years ago

Update: I wasn't able to reproduce on codesandbox, but my tests on my own project all point to the localstorage backend. I fixed my issue by disabling suspense support and calling it a day.

logemann commented 3 years ago

same for me. Playing around with suspense: false didnt change anything unfortunately. Understanding the code near the boundReset() is clearly not a no-brainer ;-)

And when i see that the nextjs guys also hitting the problem with their impl, i think there is something there. They reported that using useTranslation() might be an issue but i use it all over the place and not i18n.t(). Since this is only a development mode issue with gatsby and not in the generated pages on prod, i dont know how much time i will give this topic.

stevenfukase commented 1 year ago

I still have this issue on Next.js 13