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

t function from useTranslation is unstable in concurrent mode #1599

Closed
AlexKrupko opened this issue Dec 25, 2022 · 12 comments
Closed

t function from useTranslation is unstable in concurrent mode #1599

AlexKrupko opened this issue Dec 25, 2022 · 12 comments

Comments

@AlexKrupko
Copy link

🐛 Bug Report

t function returned from useTranslation hook is unstable and updated in a moment after mounting if use React 18 with concurrent mode. It triggers extra renders and also it trigger update of memoized values if t is used in dependencies arrays.
Please see my Codesandbox example. If you open console tab, you can see 3 calls of useEffect and false condition when t function changed.

Expected Result

t function should be stable even in concurrent mode.

Your Environment

  • react: 18.2.0
  • i18next: 22.4.6
  • react-i18next: 12.1.1
@adrai
Copy link
Member

adrai commented Dec 25, 2022

this happens only in development... because of the StrictMode...

https://stackoverflow.com/questions/72489140/react-18-strict-mode-causing-component-to-render-twice

try:

root.render(
  <App />
);

@adrai adrai closed this as completed Dec 25, 2022
@AlexKrupko
Copy link
Author

I know that and highlighted it in the ticket title and description (StrictMode enables concurrent mode in React 18).
Component should be rendered twice in strict mode. But, it renders 3 times because your lib changes t function.
This is not a solution to provide first result of google and close the ticket. I can google as well. Moreover, I've investigated this issue before ticket creation and provided link to Codesandbox where you can see that behavior.

Also, this is not acceptable solution to disable strict mode as we need it for our project. Moreover, I guess concurrent mode will be introduced and enabled always in the next React version. This is one more reason to don't remove StrictMode and cover all similar issues now.

I hope you will pay a little bit more attention to discover provided Codesandbox example and fix this issue.
Thanks

@adrai
Copy link
Member

adrai commented Dec 26, 2022

Like said, also with StrictMode this happens only during development... during production it works as expected.
I have no idea what we can do in useTranslation to prevent this... If you have any idea feel free to share your thoughts: https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js

@jamuhl
Copy link
Member

jamuhl commented Dec 26, 2022

Guess this is really only a result of using StrictMode which runs effects twice as obviously it works fine without using StrictMode. The t function gets set on a few events and changes of passed in props (as needed to work properly - it is not like you describe unstable). Like @adrai said this affects only your development environment - not production.

The main cause (my guess setting breakpoints in useTranslation resetting t) in combiniation of the double running effects is this: https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L99 which resets t based on change of i18n or keyPrefix

What might help is changing that from just running based on the !isInitial.current to compare with previous props stored using usePrevious hook https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L5

@AlexKrupko If you like you can try changing that and providing a PR if it fixes the unneeded rendering during development.

@woodreamz
Copy link

Hi !
I also have this issue, I am surprised there is no fix/investigation. From my understanding, this is exactly the purpose of the StrictMode ("find issues caused by double rendering") and it must be fixed in useTranslation. This issue is critical for me, removing the StrictMode is not a solution, it's just hiding an issue.

I will try to investigate...

@adrai
Copy link
Member

adrai commented Sep 26, 2023

@woodreamz like already described above, this is not a direct issue (at least not in this module). It happens on purpose in React development mode (not in production). So you might try to investigate in the React code...

@woodreamz
Copy link

woodreamz commented Sep 26, 2023

@adrai I am maybe wrong but if I try the codesandbox from the bug report, using the StrictMode should render twice but it renders 3 times.

For me this is the exact case where the StrictMode help to find an issue.

@adrai
Copy link
Member

adrai commented Sep 26, 2023

it might be that is now even worse with StrictMode... (but since this is coming directly from react i have no idea)
This is without StrictMode (on production): https://codesandbox.io/s/suspicious-jepsen-nymvl4?file=/src/index.tsx

feel free to investigate

@woodreamz
Copy link

woodreamz commented Sep 26, 2023

Right, without the StrictMode, it's working well as it renders 1 time.

With the StrictMode:

  • 1st render: it's normal, t does not change, it renders once.
  • 2nd render (made by StrictMode): t seems to change internally which causes an another extra render (3 renders at the end).

We should have the exact same behavior between first and second render, t should not change on second render and should not cause an extra render.

It looks like the example issue given by React on their documentation where the second render made by StrictMode causes a third render.

That being said, I am trying to investigate but it seems to be an issue. Could we re-open this one?

@woodreamz
Copy link

woodreamz commented Sep 26, 2023

@adrai Oh, I just discovered that using i18n.t instead of t is working well, here is the Codesandbox. This confirm the issue is in useTranslation.

const { i18n } = useTranslation();

useEffect(() => {
  console.log("useEffect with i18n");
}, [i18n]);

return i18n.t("text");

2 renders with i18n, it should be the same when using the t directly.

@woodreamz
Copy link

No response, I will open a new issue.

@adrai
Copy link
Member

adrai commented Oct 5, 2023

feel free to create a PR to address this, if you have a solution for this

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

4 participants