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

Memoization of server side instance causes pages to be rendered in the wrong language #1063

Closed
skrivanos opened this issue Mar 14, 2021 · 9 comments

Comments

@skrivanos
Copy link
Contributor

Describe the bug

Pages are only rendered in one language on the server when using getServerSideProps OR when using the dev server with getStaticProps.

Caused by:
https://github.com/isaachinman/next-i18next/blob/c6fc2e3bdbee07b53cb4a0871cf1abcd372c7b26/src/createClient/node.ts#L9-L11

Added in commit:
e7c5fad

Occurs in next-i18next version

8.1.1

Steps to reproduce

Run example/simple, click switch locale and reload the page.

Expected behaviour

To use the correct locale.

Screenshots

Screenshot 2021-03-13 at 19 20 07

Additional context

This doesn't cause issues in production when using getStaticProps because the translations are stored upon build, but it messes up pages using getServerSideProps (they will always render in whatever language the i18n instance was firstly created as).

@sigo
Copy link

sigo commented Mar 14, 2021

Hit the same today. I can also confirm this issue.

Temporary workaround is downgrading and pinning next-i18next@8.1.0 until bugfix.

@isaachinman
Copy link
Contributor

cc @adrai

@adrai
Copy link
Member

adrai commented Mar 15, 2021

I have not much Next.js knowledge, so I don't know why this warning exactly occurs.
Maybe this is the reason, why there was always a new instance created before....
will try to create a PR with cloneInstance... asap

@adrai
Copy link
Member

adrai commented Mar 15, 2021

@skrivanos @sigo I was not able to reproduce the warning with my PR can you also confirm?

@skrivanos
Copy link
Contributor Author

skrivanos commented Mar 15, 2021

@adrai Seems to work, however I'm not sure I'm a fan of creating a new instance each request. I tried this

  if (!instance) {
    instance = i18n.createInstance(config)
  } else if (config.lng && config.lng !== instance.language) {
    instance.changeLanguage(config.lng)
  }

Which seems to have done the trick as well... Are there any downsides to this?

Edit:
Actually, thread safety might be an issue. But I suppose it would be with your fix as well?

@adrai
Copy link
Member

adrai commented Mar 15, 2021

creating a clone on every request was also done when used i18next-http-middleware in older versions of next-i18next...
beside this, i don't know if there is more than just the language that changes, if so changeLanguage should work too

@isaachinman
Copy link
Contributor

@skrivanos I'm in favour of cloning, as I'm not sure how changeLanguage would handle concurrent reqs.

@adrai Is the entire point of cloning vs createInstance to prevent network requests to upstream data sources?

@adrai
Copy link
Member

adrai commented Mar 16, 2021

@adrai Is the entire point of cloning vs createInstance to prevent network requests to upstream data sources?

yes

@kachar
Copy link

kachar commented Mar 16, 2021

We were experiencing the same problem on 8.1.1, migrating to 8.1.2 solved it. 🎉

The server did cache the language initially, so the first language to be shown was always the initial one on SSR. The option defaultLanguage was not taken in account.

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

Successfully merging a pull request may close this issue.

5 participants