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

Memorized Components Being Unnecessarily Re-rendered #979

Closed
mwskwong opened this issue Oct 26, 2019 · 11 comments
Closed

Memorized Components Being Unnecessarily Re-rendered #979

mwskwong opened this issue Oct 26, 2019 · 11 comments

Comments

@mwskwong
Copy link

mwskwong commented Oct 26, 2019

Describe the bug
When using useTranslation in a memorized component (either memo or PureComponent), the component is being unnecessarily re-rendered. For me, it seems that this only happens if i18next-xhr-backend is used. In the codesandbox sample, 1 extra render is fired. While the worst I have met (in rare situation) is 7 extra renders.

Occurs in react-i18next version
react-i18next@11.0.0

To Reproduce
Just use useTranslation in a memorized component
https://codesandbox.io/s/react-i18next-test-wzj08
https://codesandbox.io/s/react-i18next-test-z4jx0 (extended by @jamuhl)

Expected behaviour
A memorized component should behave the same as a normal component in terms of utilizing react-i18next.

Additional context
Original discussion: #710 (comment)

@jamuhl Please read the console output for details (and forget whether the backend locates the translation.json or not).
** ONLY causes unnecessary re-renders when using i18next-xhr-backend**

https://codesandbox.io/s/react-i18next-test-wzj08

If you believe it is easier for you guys to trace and maintain, I can open a new issue instead of continuing the discussion on this issue.

Wow...that is so weird - I extended your sample a little: https://codesandbox.io/s/react-i18next-test-z4jx0

(making sure no saveMissing, ... gets in the way, also adding a counter to the MemorizedButton to see it gets rendered to DOM only once - while called twice)

I moved the sample to local so I could add some more logs to the useTranslation:

// ...
  if (!ready && !useSuspense) return ret; // not yet loaded namespaces -> load them -> and trigger suspense

  throw new Promise(function(resolve) {
    console.warn(`${props.name} throw Promise (trigger Suspense)`);
    loadNamespaces(i18n, namespaces, function() {
      console.warn(`${props.name} set state (loaded namespace -> sets new t function)`);
      setT(getT());
      console.warn(`${props.name} resolves Promise (resume Suspense)`);
      resolve();
    });
  });

image

The SimpleButton gets triggered it's rerender by the setT. Immediately after the MemorizedButton also renders - honestly I have no idea why. Then Suspense get resolved by the SimpleButton - removing the fallbackUI and showing the real content again. Then setT gets called for the MemorizedButton - and it rerenders (calls the function internally) while the rendered result stays the memoized memorizedButton1 (counter says 2)...

Change the order of the 2 Buttons MemorizedButton before SimpleButton -> only one render as SimpleButton gives a shit about MemorizedButton gets setT called.

Move every Component in own Suspense -> both render only once

So...if this is related to react-i18next - feel free to provide a PR...as I don't know which magic triggers a rerender on the memorized component...I have no clue how to fix this...

To make it more clear - I got no idea what is triggering this render:

image

I just know it's not useTranslation (neither the loading done as it comes later nor the bound events)

@jamuhl
Copy link
Member

jamuhl commented Oct 26, 2019

Fun fact...removing memo:

image

So with memo we get a rerender on Suspense triggered in the component, without not. The cause seems to be within memo...which asserts a Component using useState gets triggered a rerender on state change...

@mwskwong
Copy link
Author

mwskwong commented Oct 26, 2019

That is kind of a workaround and I actually have noticed that before. The problem is, sometimes I have to use memo + useTranslation to improve the performance of loading a heavy component.
e.g. an SVG chart with localized title, axes and legend labels.

@jamuhl
Copy link
Member

jamuhl commented Oct 26, 2019

I think there is no fix for this...we use Suspense in a very experimental way - outside of the only use case of react.lazy...

a) there is a workaround moving memo in an own Suspense
b) pass in t as props from the outer Component...
c) setting useSuspense: false and handle the ready flag yourself with rendering alternative fallback if ready is false

@mwskwong
Copy link
Author

mwskwong commented Oct 26, 2019

@jamuhl Ok, definitely a problem with useTranslation, have no such problem while using withTranslation
https://codesandbox.io/s/react-i18next-test-wzj08

@jamuhl
Copy link
Member

jamuhl commented Oct 26, 2019

funfact -> withTranslation uses useTranslation

@mwskwong
Copy link
Author

Then it is really weird. As you can see in codesanbox, withTranslation indeed does not causes any extra rendering.

@jamuhl
Copy link
Member

jamuhl commented Oct 27, 2019

The only way to find out what happens would be to debug the react-dom reconciler as memo HOC gets handled there... 🤷‍♂️but that is a rabbit hole too deep for me 🥺

@jamuhl
Copy link
Member

jamuhl commented Oct 27, 2019

Barebone sample to reproduce: https://codesandbox.io/s/great-grothendieck-oof2w

@takakobem
Copy link
Contributor

@matthewkwong2 This issue might be resolved by #1174

@tigerabrodi
Copy link
Contributor

@matthewkwong2 This issue might be resolved by #1174

@jamuhl @adrai It seems like this issue was resolved already. I'd propose we close it 🎉.

Side question: I was wondering if there are more things I can do to contribute to this project? Also, do you have any plans for next year and the future of react-i18next? I'd love to be apart of it 🥰

@jamuhl
Copy link
Member

jamuhl commented Dec 6, 2020

@tigerabrodi for react-i18next there are currently no bigger plans. For i18next we're somehow waiting for an outcome of https://github.com/unicode-org/message-format-wg . If there will be a new standard come out for i18n we might adapt that in future...

In general...like said...we're more than happy for any help

@jamuhl jamuhl closed this as completed Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants