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

Re-renders of memoized components on shallow route changes #1059

Closed
skrivanos opened this issue Mar 12, 2021 · 5 comments · Fixed by #1064
Closed

Re-renders of memoized components on shallow route changes #1059

skrivanos opened this issue Mar 12, 2021 · 5 comments · Fixed by #1064

Comments

@skrivanos
Copy link
Contributor

skrivanos commented Mar 12, 2021

Describe the bug

As we discussed in #983, components that utilise useTranslation are always re-rendered on shallow route changes, regardless of if they are memoized or not. This is caused by the fact that the i18n instance is re-created every time the App is rendered.

The problem has been exacerbated in the last two weeks: it now re-renders the memoized component twice on every shallow route change rather than once (compare 8.0.1 and 8.1.1). I suspect this is because of this MR being merged in react-i18next: i18next/react-i18next#1273 - which is also mentioned in #1023.

Occurs in next-i18next version

8.1.1

Steps to reproduce

Example repo: https://github.com/skrivanos/next-i18next-rerender-poc
https://next-i18next-rerender-poc.vercel.app

Expected behaviour

The memoized component should not re-render.

@skrivanos
Copy link
Contributor Author

@isaachinman I wanted to work on this myself, but ran into some issues with yarn link... If you could point me in the right direction (since Google isn't helping) I'd happily contribute. After running yarn link next-i18next which correctly symlinks the package to my local copy, I get these errors:

info  - ready on http://localhost:3000
event - build page: /
wait  - compiling...
warn  - ../Code/github.com/skrivanos/next-i18next/node_modules/i18next-fs-backend/cjs/readFile.js
Critical dependency: the request of a dependency is an expression

../Code/github.com/skrivanos/next-i18next/node_modules/i18next-fs-backend/cjs/readFile.js
Critical dependency: the request of a dependency is an expression

../Code/github.com/skrivanos/next-i18next/dist/commonjs/serverSideTranslations.js
Critical dependency: the request of a dependency is an expression
info  - ready on http://localhost:3000
Error: Cannot find module '/Users/alex/my-app/next-i18next.config.js'
    at webpackEmptyContext (eval at ../Code/github.com/skrivanos/next-i18next/dist/commonjs sync recursive (/Users/alex/my-app/.next/server/pages/index.js:103:1), <anonymous>:2:10)
    at eval (webpack-internal:///../Code/github.com/skrivanos/next-i18next/dist/commonjs/serverSideTranslations.js:90:156)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  code: 'MODULE_NOT_FOUND'
}

@isaachinman
Copy link
Contributor

@skrivanos Not sure how to help, as it looks like it's dependent on your filesystem setup. Does my-app/next-i18next.config.js exist?

@skrivanos
Copy link
Contributor Author

@skrivanos Not sure how to help, as it looks like it's dependent on your filesystem setup. Does my-app/next-i18next.config.js exist?

Yes, everything works perfectly after unlinking. I've even md5sum:ed the files in dist/ and they match. The only difference is that after linking my_app/node_modules/next-i18next is a symlink to my local copy (/Code/github.com/skrivanos/next-i18next). Figured you might have run into something similar before but I suppose it could be a problem with my specific version of Webpack + MacOS or something.

@isaachinman
Copy link
Contributor

Nope, I've not run into this issue.

skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 14, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 14, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 14, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 14, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
@dcodrin
Copy link

dcodrin commented Mar 22, 2021

i18n instance is re-created every time the App is rendered

@isaachinman Could this also explain why our dynamic namespaces are loaded on every route change? Which basically defeats the purpose oh having them since these dynamic components will flash the keys since the instance is recreated and new calls have to be made.

skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 23, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Mar 23, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
DanPurdy added a commit to DanPurdy/next-i18next that referenced this issue Apr 30, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
DanPurdy added a commit to DanPurdy/next-i18next that referenced this issue May 6, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix i18next#1059).

* Let Next's router locale take precedence over initialLocale (fix i18next#1023).

* Bump the minimum version of react-i18next.
skrivanos added a commit to skrivanos/next-i18next that referenced this issue May 10, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed.

* Let Next's router locale take precedence over initialLocale.

[fix i18next#1059]
skrivanos added a commit to skrivanos/next-i18next that referenced this issue Sep 13, 2021
* Memoize the i18n client and do not re-create it unless the route or
  locale has changed.

* Let Next's router locale take precedence over initialLocale.

[fix i18next#1059]
isaachinman pushed a commit that referenced this issue Sep 13, 2021
* fix: partially support shallow route changes

* Memoize the i18n client and do not re-create it unless the route or
  locale has changed.

* Let Next's router locale take precedence over initialLocale.

[fix #1059]

* refactor: do global assignment inside useMemo

* refactor: cease forwarding initialLocale from serverSideTranslations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants