Usage of useTranslation hook inside components cause rerender

srgvrnv commented 4 years ago

Describe the bug

When I use useTranslation hook inside function component it causes multiple rerenders. Can't reproduce it with create-react-app and react-i18next. So it should be landed here. If I use react profiler I see that it takes 1102 render, but with create-react-app and react-i18next it takes only 2. Am I missing something or this is a bug?

Occurs in next-i18next version

4.2.1

Steps to reproduce

# i18n.js

const NextI18Next = require('next-i18next').default

module.exports = new NextI18Next({
    defaultLanguage: 'fi',
    otherLanguages: ['en', 'ru'],
    localeSubpaths: {
        en: 'en',
        ru: 'ru',
    },
})

# _app.js

import React from 'react';
import './App.css';
import Test from './components/test';
import i18n from './i18n';

const Locales = () => (
  <>
    {['fi', 'en', 'ru'].map(locale => (
      <button type="button" key={locale} onClick={() => { i18n.changeLanguage(locale); }}>
        {locale}
      </button>
    ))}
  </>
);

function App() {
  let tests = [];

  for (let index = 0; index < 100; index++) {
    let testsinner = [];
    for (let index2 = 0; index2 < 10; index2++) {
      testsinner.push(<Test key={`${index}_${index2}`} />);
    }
    tests.push(<Test key={index}>{testsinner}</Test>);
  }

  return (
    <>
      <Locales />
      <div>
        {tests}
      </div>
    </>
  );

}

export default App;

# components/test.jsx

import i18n from '../i18n';

let count = 0;

const Test = ({ children }) => {
    count++;

    const [t, { language }] = i18n.useTranslation();

    return (
        <>
            <p>{t('catalog')} + {count} + {language}</p>
            <div>{children}</div>
        </>
    )
}

export default Test;

Expected behaviour

Shouldn't be rendered so much times

OS (please complete the following information)

OS: macOS Catalina
Browser: Chrome 80.0.3987.132

isaachinman commented 4 years ago

Hi @srgvrnv - thanks for this report. Do you mind quickly setting up a demo repo that we can reproduce this and track down the issue?

srgvrnv commented 4 years ago

Hi! Sure, here it is

srgvrnv commented 4 years ago

@isaachinman Hello! Any updates on this issue?

isaachinman commented 4 years ago

@srgvrnv I don't really know what you expect, you're rendering 1100 components in a loop.

srgvrnv commented 4 years ago

@isaachinman

Size of loop is ridiculously huge just to demonstrate a problem. I face this issue while rendering nested set of 50 entities with nesting level of 2 using of useTranslation() hook inside each of them.

Anyway react-i18next have no issue to render "1100 components in a loop" while next-i18next have some problems with it

Maybe there is some misunderstanding but I expect it to work same way

Am I missing something here?

ricardobrandao commented 4 years ago

I'm having the same issue. @srgvrnv were you able to understand what the issue was?

Best,

srgvrnv commented 4 years ago

@ricardobrandao Unfortunately not. For now I solve it with using static class which I setup in _app.tsx instead of using useTranslation()

LocaleHandler.locale = language;
LocaleHandler.t = t;

chrisinajar commented 4 years ago

The example repository posted above clearly demonstrates that next-i18n results in 1 extra render per language change than react-i18n does. Each full re-render using the react variant causes 1,100 renders -- one each. The next variant causes each component to render twice, 2,200 renders total.

This seems to be caused by the entire App tree being re-rendered instead of just the components that change. You'll notice that in the example above, the core app component in the CRA version never renders again after the first render, while in the next version the App renders every single time and then re-renders all of the children once more as well (needlessly double rendering all of them).

This can be very easily observed by simply adding log statements to the render methods: Here you can see all of the components render with the new language after the change, and then the app itself re-renders causing them all to render once more. My guess is something to do with putting the language in the url...

Anyway, I hope this additional information helps. This ticket should be re-opened as there is clearly either an undocumented inconsistency or an actual bug.

isaachinman commented 4 years ago

Don't have a lot of spare time at the moment, but very happy to hear any suggestions/ideas.

SalahAdDin commented 4 years ago

Is it possible to use react-i18n instead of this package?

isaachinman commented 4 years ago

@SalahAdDin Of course.

SalahAdDin commented 4 years ago

@isaachinman It just occupy to change the language, as we are using in a plain react website we are working too.

But there is a problem: I'm getting the language by const lng = () => localStorage.getItem("language") || "en".

But when i try in this way i get: TypeError: Cannot read property 'use' of undefined, also localStorage is not defined.

Why?

ddhp commented 4 years ago

I found out whenever the language is changed, the router would be replaced which is causing the extra rerendering (source code). Probably this is the expected behavior, since next.js also handles the routing part but create-react-app doesn't?

@srgvrnv you said you fixed this issue by setting static values, but are you still using appWithTranslation to wrap your App? Also thanks for your repo which is very helpful to debug

k4mr4n commented 3 years ago

I have the same issue and whydidyourender library point it out very well. it seems like the "t" function exported from "useTranslation" hook has not been memoized.

tquiroga commented 3 years ago

Same issue pointed by whydidyourender, when you use the hook across your app and realize most re-render are caused by the useTranslation hook, ouch 😬

isaachinman commented 3 years ago

Does anyone mind testing this issue in next-i18next@8.0.0 to see if it is still relevant?

zcmgyu commented 3 years ago

I don't know why but I got the same issue with react-i18next. LOL

k4mr4n commented 3 years ago

@isaachinman upgrading to v8.0.0 from v6 is really painful. would there be any patch for the v6 to fix this issue?

isaachinman commented 3 years ago

@k4mr4n As with all previous releases, I won't be actively working on maintenance, but if users want to open PRs, I will certainly review, merge, and publish them.

asmeeee commented 3 years ago

For anyone interested, re-render is still happening to me (probably should at some point), but I ended up using next-translate and it seems to be working fine.

SalahAdDin commented 3 years ago

@andrei-scripcaru why did you switched to next-translate?

asmeeee commented 3 years ago

@andrei-scripcaru why did you switched to next-translate?

Switching the language was causing children (not sure exactly which ones) to re-render and there's no re-rendering with next-translate. I believe it has to do something with getStaticProps since if commented it works as expected.

SalahAdDin commented 3 years ago

@andrei-scripcaru why did you switched to next-translate?

Switching the language was causing children (not sure exactly which ones) to re-render and there's no re-rendering with next-translate. I believe it has to do something with getStaticProps since if commented it works as expected.

@isaachinman

isaachinman commented 3 years ago

@SalahAdDin What was the intention of the @?

If anyone can provide reproducible examples of faulty behaviour, issues will be fixed quickly.

muhammadabdulazim commented 3 years ago

any solution ?

houssemmimoun27 commented 2 years ago

Create a file Translate.js import { useTranslation } from "react-i18next";

function Translate(props) { const { t } = useTranslation(); return t(props.value); } export default Translate;

then add this function in the component

Thats solution fix the problem of rerendering the component.

Poyoman39 commented 2 years ago

There is this effect in source code causing useTranslation to refresh the "t" function (i'm using suspense)

  var isInitial = useRef(true);

  useEffect(function () {
    if (isMounted.current && !isInitial.current) {
      setT(getT);
    }

    isInitial.current = false;
  }, [i18n]);

Poyoman39 commented 2 years ago

Guys ... could it be your bug ??? (this was my one) https://stackoverflow.com/questions/61254372/my-react-component-is-rendering-twice-because-of-strict-mode

Try disabling React.StrictMode !!!! ;)

isaachinman commented 2 years ago

@Poyoman39 That is not next-i18next code.