refactor: reuse existing instance

[wjaykim]

This PR changes how next-i18next loads resources on page or locale change.

Instead of creating a new instance on page or locale change, the change uses addResourceBundle method to load newly changed resources if instance was already created.

I made this change because I found an edge-case:

1. Enter page with getStaticProps and serverSideTranslations configured.
2. Move to page without serverSideTranslations, instead load translations in client-side with i18next-http-backend;
   Also I used suspense option to true
3. Component with useTranslation hook got suspended, and it suspends forever because i18next instance is created repeatedly, thus not initialized forever.

After applying this patch, the infinite suspending issue was removed.

Checklist

• only relevant code is changed (make a diff before you submit the PR)
• run tests npm run test
• tests are included
• commit message and code follows the Developer's Certification of Origin

[adrai]

Can you please provide a minimal example repo I can use to verify the issue first?

[wjaykim]

While creating reproduction, I found that when using suspense in layout component(located in _app, higher than page level), the issue is always reproduced.

Please see https://github.com/wjaykim/next-i18next-infinite-suspense for reproduction, and also I added patch branch which you can see patched version of next-i18next works well compared to main branch.

Also I think maybe #2095 is caused by this.

[wjaykim]

I accidentally closed PR.

[adrai]

The question I have is why do you want to prevent it from being rendered on the server?
As soon as you don't use your SSRSafeSuspense component it works without problems...

[wjaykim]

The question I have is why do you want to prevent it from being rendered on the server?

I don't want to add getStaticProps each page to render header.

As soon as you don't use your SSRSafeSuspense component it works without problems...

What do you mean? It doesn't works, actually..
Anyway, the reason I wrapped with SSRSafeSuspense is without it, nextjs will cause hydration mismatch related error.

[adrai]

But why do you set useSuspense to true?
next-i18next was designed in a way that no suspense should be used.

[adrai]

btw: using your "fixed" version in this example you can see the language change does not work properly anymore: https://github.com/i18next/next-i18next/tree/master/examples/simple

[wjaykim]

next-i18next was designed in a way that no suspense should be used.

Didn't know that at first, but if this change can make next-i18next be compatible with Suspense, why shouldn't we apply this fix?

btw: using your "fixed" version in this example you can see the language change does not work properly anymore

I didn't know that, so updated code (adding instance.changeLanguage) to fix the issue.

[adrai]

if that will not hurt the existing behaviour for the existing users, we can merge it (maybe also behind a new option), but right now it seems it still gets issues with the "normal" usage of next-i18next:
Run the examples, change language, navigate to other pages and check the console logs:

[wjaykim]

if that will not hurt the existing behaviour for the existing users

Yes that's right. I want you to check if my change doesn't change any existing behavior, and I'm willing to make this PR perfect so it can be merged because suspense is really fascinating feature to me.

To find out this PR has no problem, what's the best way to prove it? Running official examples of this repo?

[adrai]

To find out this PR has no problem, what's the best way to prove it? Running official examples of this repo?

Yes, exactly the examples of this repo are the minimum, and afterwards also this example: https://github.com/i18next/i18next-http-backend/tree/master/example/next

[adrai]

not good yet.... (this happens when navigating between pages)

the addResourceBundle call is done too late... the t function is already called before that...

maybe move it here: https://github.com/i18next/next-i18next/pull/2226/files#diff-dc78a43079a28bdd9c1d6b6edb580f0131b8627926f905054069b15470bd6553R73 ?

[wjaykim]

Right.. I'll reach you after find more good way and when all tests passed (including e2e)

[wjaykim]

Latest code passes all tests including e2e in local. Can you take a look?

[adrai]

looks good

[adrai]

I hope this does not influence some other edge case usage....
I suggest, we merge that and create a new major version.
If there is any issue from the community that we can't address, we may add an option to be able to switch back to the old instance creation.
What do you think?

[wjaykim]

Yes, if we concern more about stability, we can make the code works same as before for default, and add option to reuse instance optionally.

[adrai]

but only if issues arise

[adrai]

Thank you very much for your contribution.
v15.0.0 was just released.