consistent I18nProvider and enable client side translation loading

[DanPurdy]

This follows on and includes the great work of @skrivanos in #1064 I've rebased from his PR and then applied the current master changes too (conflicts galore - happy to tidy this up)

notes from that PR

• Memoize the i18n client and do not re-create it unless the route or
  locale has changed (fix Re-renders of memoized components on shallow route changes #1059).

• Let Next's router locale take precedence over initialLocale (fix Shallow routing does not work #1023).

• Bump the minimum version of react-i18next.

Will happily rebase again if that PR is merged as it provides a great way forward but its been untouched for a few weeks

On top of that i've hopefully addressed #1049 and #1075 and also provided a path for those that want to easily load their translations client side using http-backend (not included as a dep here but with an example to show how, going to investigate the repercussions this has for fallback pages too but should provide a path forward there also).

While serverSideTranslations should still be the default for this plugin i and from the issues list a few others are looking for ways to make it easier to load translations client side to avoid the sometimes jarring user experience for an SPA of having to load routes using getServerSideProps.

Added a few more errors and fallbacks to config values plus tests around those.

I guess this PR is by no means complete but looking for your feedback around anything you're not happy with before i go much further

TODO

• documentation in readme on the changes and how to use client side
• Update cypress tests? Maybe publish the withHttp example
• More examples for fallback routes
• Tidy up the littering of 'ready' from useTranslation in the client-page example

As per the discussion in #1064 where you were talking about removing the initialLocale prop from serverSideTranslations, I believe this should remain unless we can find a good way to ensure the language is correct on first render as your serverSideTranslations are called there before appWithTranslation has had time to create a global i18n instance. Open to thoughts and ideas here about what you had in mind if you want this prop removed.

fix #1049
fix #1075

Closed by #1726.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/isaachinman/next-i18next/9AC9MtYdz3PN1vKFw9Wg4RchB7Gp
✅ Preview: https://next-i18next-git-fork-danpurdy-featureconsistent-clien-912c39.vercel.app

[skrivanos]

Cool! Perhaps it would be preferable to get my PR merged and then build on top of that? I just haven't had the time/energy to make the needed adjustments yet due to a deadline coming up next week.

Regarding initialLocale, I think the idea was that it shouldn't be needed anymore since we can just access the locale from the ServerRouter instance that is passed to AppWithTranslation by Next. I don't think my branch uses it anywhere anymore.

[isaachinman]

Hey @DanPurdy and @skrivanos – I'd definitely rather get the initial PR merged first. This PR in its current state is just too large to review properly. Thanks!

[DanPurdy]

Hey both, thanks. Yes definitely agree that getting the initial PR in first is best, happy to help move that along if needs be.

Would be good to get your thoughts on adding extra examples too, big bulk of the listed changes here are just a new yarn.lock and duplicated components, the failing check is for duplication in the components used as it's just a extra few pieces on top of the simple example.

Thanks again

[isaachinman]

I'm not sure we need an entire duplicated example dir. Can we not achieve the same result with normal documentation?

[DanPurdy]

We could just through documentation yeah, the readme i added to that example directory would just need to be pulled across and a bit of extra information added i guess? I suppose I figured that an example with it in place working follows on from how the next team have all their individual examples and makes it easier for someone to copy the implementation and due to the differences needed in the config file its not really something we can add to the simple example without making it drastically less simple 😄

Happy to do whatever is needed here to help out though.
I'll test out that other branch and see if we can get away with removing the initial locale prop as i did come across issues early on removing it myself in this branch. Will report back there if i can see anything

[isaachinman]

My general concern with having multiple example dirs is maintenance – I am a sole maintainer and have limited time. Thanks @DanPurdy!

[DanPurdy]

Fully understandable having been there myself. I'll remove the example dir and update the Readme instead with the example then. Thanks!

[kfazinic]

Hi, excuse my drop-in here, just wondering if this will be merged soon, as it would mean a lot in our project? :) Thanks for great work @isaachinman @DanPurdy @skrivanos

[DanPurdy]

Hi, excuse my drop-in here, just wondering if this will be merged soon, as it would mean a lot in our project? :) Thanks for great work @isaachinman @DanPurdy @skrivanos

Hey, once #1064 is merged in then i'll be updating this branch as this has a lot of the changes already made there too.

[aNyMoRe0505]

@DanPurdy @isaachinman @skrivanos

Hello, thanks for great work!
I'm new to i18n
After searching some issues and discussion, I still can't understand some points about loading translation from CDN(maybe) in client side, I am not sure if it is suitable to post my question here. I'm sorry if there is any offense

According to this commnet #1049 (comment)
It seems that we can fetch our translations files via CDN with i18next-http-backend, so we probably will have config like

  const i18nextHttpBackend = require('i18next-http-backend');
  (module.exports..)
  i18n: {
    defaultLocale: 'en-us',
    locales: ['en-us', 'zh-hk'],
  },
  preload: ['en-us', 'zh-hk'],
  ns: ['common', 'commn2'],
  defaultNS: 'common',
  localePath: 'public/static/locales',
  partialBundledLanguages: true,
  serializeConfig: false,
  react: {
    useSuspense: false,
  },
  ...(typeof window !== 'undefined'
    ? {
        use: [i18nextHttpBackend],
        backend: {
          loadPath: 'https://${CDN_URL}/{{lng}}/{{ns}}.json',
          crossDomain: true,
          withCredentials: false,
        },
      }
    : {}),

and in _app.js we will pass config to sec parameter

appWithTranslation(MyApp, nextI18NextConfig)

and in page component

export const getServerSideProps = async ({ locale }) => {
  return {
    props: {
      ...(await serverSideTranslations(locale, ['common2'])),
    },
  };
};

Although I encounter many issues about this strategy

• will fetch our translation file twice in first landing (SSR) from CDN (observe from network tab), maybe this is because appWithTranslation and getServerSideProps both will do createConfig and createClient (? I am not sure
• not only fetch the locale specific translation file we want, will fetch both 'zh-hk' and 'en-us' translation files.
• only fetch the namespaces we didn't put into serverSideTranslations. like if we use useTranslation('common') in page component, and we only put common2 in serverSideTranslations, in this scenario will only fetch common translation file via CDN

I want to understand the logic behind first.

1. From my understanding, serverSideTranslations should only work in server side then why we need to handle client situation in createConfig (line 125) ? !process.browser && typeof window === 'undefined' should always be true (?)

2. We only include the i18nextHttpBackend plugin and backend options when typeof window !== 'undefined' (mean in client side). does this mean when we use serverSideTranslations, actually it will load translation files from the original logic (hasCustomBackend always be false) ? if this is a unnecessary logic (implement i18next-http-backend) because we actually load the translation files in serverSideTranslations?

3. Following the previous question, I think my concept is wrong. With this strategy mean we don't want to load all translation files in synchronize, so I can fetch some translation files via CDN which only needed by some dynamic component. for example, we want common2 in synchronize and common loaded asynchronous in client side so we put common2 in serverSideTranslations and we also can use common translation file by useTranslation(common). that is why CDN only fetch common. Am I right ?

4. and this PR is mainly for full client side translation loading, we can fully load our translation files asynchronous without getServerSideProps || getStaticProps (https://github.com/DanPurdy/next-i18next/tree/feature_consistent-client-backend-example-b/examples/withClientsideHttp). if we use this strategy, it mean we will see translation key before translation files downloaded, it will hurt SEO. Am I right ?

thanks for your time and attention, I really want to understand these concept and confirm if I understand correctly or not

Thanks a million!!

[DanPurdy]

@aNyMoRe0505 This PR isn't really the place to discuss this, best to open a discussion if you want to continue this but basically you shouldn't do both server side and client side loading yes. You kind of hit the nail on the head with point 4, you wouldn't use this client side loading strategy for SEO pages in those cases static render is the way to go if possible, server side is probably your next bet and client side being your last option (this is by no means a universal rule though!).

The purpose of this PR is just to give you the option, for pages like account pages or in my use case some sort of private booking flow that will never be crawled by google and can't be statically or server rendered. In these cases you can use the isReady property from the useTranslation hook to hide your content or show some sort of loading state while your translations are loaded and then once i18n has initialised then you can show the translated content. Once #1064 is merged in then i'll be rebasing and updating this branch to make it a little clearer on why and when you might want to use this method - even though its still best to use serverSideTranslations. Hope that makes it clearer

[natterstefan]

Hi @DanPurdy,

thanks for creating this PR. I am looking forward to getting this feature, after reading lots of comments about people wanting this feature back (example), issues (e.g. this one), and proposed solutions (this one or this one for instance) in this repo.

I've one question though that is in the back of my head when seeing what you've created here. Especially after reading the "Client side loading of translations via HTTP" Readme.

Do I need to be concerned about the maintainability of this solution, as it depends on using i18next-* packages, which are implicitly used in next-i18next? What do you think about that?

Thanks for your time and help. I appreciate it a lot.

[peter-jozsa]

any chance this is going to be merged and published in the near future? App remounts mentioned in #1075 is a real blocker I think.

[nicolasconnault]

Ping! This is an important PR, please merge

[isaachinman]

Merging is predicated on someone rebasing this PR against latest changes, and owning any further work necessary.

[DanPurdy]

Hey all, no worries I'll be back to rebase and take a look soon, just a busy week!

[natterstefan]

Hey @DanPurdy,

I am sorry to bother you, do you think you'll take a look at this any time soon again?

Thanks for your work in advance.

[cmaerz]

Any Updates here?

[Javimtib92]

Same here, I have the _app remount issue and I'm waiting for this to be merged, any chance this is going to be merged any time soon? thanks

[hoangtrung99]

Same here, I have the _app remount issue and I'm waiting for this to be merged, any chance this is going to be merged any time soon? thanks

I have same issse, Looking forward to the PR merged.

[maurobalbi]

#1075 has caused us quite a bit of headaches, firstly with next-auth sessions and secondly framer-motion causing animations not to render. Looking forward to the PR being merged aswell.

[isaachinman]

Anyone able to step up and help get this PR across the line?

[adrai]

@isaachinman I tried: #1726

[isaachinman]

Thanks @adrai – that's a huge help!

I would appreciate if everyone who has bumped this PR can go to #1726 and give a review, and help test.

[isaachinman]

We've now merged #1726.