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

Suspense fallback triggered in the first language change, but not in the others #1031

Closed
crash7 opened this issue Jan 3, 2020 · 12 comments
Closed

Comments

@crash7
Copy link
Contributor

@crash7 crash7 commented Jan 3, 2020

Describe the bug

I have an SSR app with three languages: ES, EN, DE.
In the first load, it will render the default language, EN.

When I select one of the other languages, let's say ES, it will trigger the Suspense fallback.

The strange behavior appears when I select the third language, DE. In this case, it doesn't trigger the fallback, despite it's loading the required namespaces.

This only happens when I try to use not-default namespaces (in my example, alternate ns).

Here is a PR with the updated example where you can see this issue: #1030

Occurs in react-i18next version
"react-i18next": "^11.2.7",

To Reproduce
Steps to reproduce the behavior:

  1. Load the example #1030
  2. Click on 'Switch to ES'
  3. Note that it will flicker, because it will trigger the suspense fallback
  4. Click on 'Switch to DE'
  5. Note that it will change the language without the flicker

Expected behaviour
Either suspense gets triggered in all the switches or it doesn't trigger for any of them.

(I would like the second option for my case, but if it's configurable I don't mind).

Screenshots
N/A

OS (please complete the following information):

  • Device: MBP 2017 15"
  • Browser: Chrome 79.0.3945.88
@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jan 5, 2020

Untested but seems like testing for loaded here: https://github.com/i18next/react-i18next/blob/master/src/utils.js#L80 fails...as adding resources via initialStore does not set the backend load state: https://github.com/i18next/react-i18next/blob/master/src/utils.js#L57

Changing https://github.com/i18next/react-i18next/blob/master/src/utils.js#L80 to

if (loadNotPending(lng, ns) && (!fallbackLng || loadNotPending(lastLng, ns) || i18n.hasResourceBundle(lastLng, ns))) return true;

might solve this (yet not sure it's the best solution). Could you please test if this helps?

An alternative would be adding the load state by looping lng-ns here: https://github.com/i18next/react-i18next/blob/master/src/useSSR.js#L15

@crash7

This comment has been minimized.

Copy link
Contributor Author

@crash7 crash7 commented Jan 6, 2020

Tried that change, the fallback is still being triggered :(

Regarding your second idea, I believe that the problem is that something is not being initialized on SSR but it's initialized after the first language change. Is that what you are referring to? On a language change you loop through all the lng-ns? Even the ones that you are not changing to?

On SSR we only send the current language to the user (in this case, the default one, EN) so if I do a loop, all the other languages (DE, ES) should have a load state "pending".

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jan 6, 2020

I referred to eg. serverside renders in ES (not passing down EN) -> EN is the fallbackLng -> so changing to DE would trigger the suspense as neither DE nor EN are there

Could also be removing the language detector and not setting an initial language
i18n.init({ lng: 'en'}) (only defining a fallbackLng) causes the issue....hard to tell...really would have to debug through it...

@crash7

This comment has been minimized.

Copy link
Contributor Author

@crash7 crash7 commented Jan 6, 2020

Ok, in my case serverside renders EN and I'm passing down EN so the store gets populate with the EN lng.
Tried with i18n.init({ lng: 'en'}) in addition to the fallback but the behavior is still the same.

Will continue debugging this, it must be some initialization..

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jan 6, 2020

Watch out for two things:

  1. what triggers the the update: https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L70 --> should only be a valid languageChanged event EN -> ES

  2. why we get a false ready state here: https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L43 based on the code here: https://github.com/i18next/react-i18next/blob/master/src/utils.js#L43

@crash7

This comment has been minimized.

Copy link
Contributor Author

@crash7 crash7 commented Jan 7, 2020

I think I got it, if I don't populate the store with the SSR language (EN), I see the fallback only for the EN language (on load basically) but not when changing to the others, so probably we are missing something when hydrating the store.

Do you remember any particular step that we do when using the backend? The only difference I can find is that it uses the addResourceBundle to add the data to the internal store.
https://github.com/i18next/i18next/blob/master/src/BackendConnector.js#L90

So maybe this https://github.com/i18next/react-i18next/blob/master/src/useSSR.js#L16 it's the problem.

@crash7

This comment has been minimized.

Copy link
Contributor Author

@crash7 crash7 commented Jan 7, 2020

Found it, if I set the i18n.options.ns to the namespaces in the initialI18nStore, changing the language won't trigger the suspense fallback.

Hacky test:

if (initialI18nStore && !i18n.initializedStoreOnce) {
    i18n.services.resourceStore.data = initialI18nStore;
+    i18n.options.ns = Object.keys(initialI18nStore['en']);
    i18n.initializedStoreOnce = true;
    i18n.isInitialized = true;
  }

Which is the same the addNamespaces is doing for all the data loaded through the backend connector https://github.com/i18next/i18next/blob/master/src/ResourceStore.js#L16-L20

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jan 7, 2020

Ok, that seems to make sense...i18n.options.ns basically is a current list of all namespaces (on init it contains all to load initial and during usage we add namespaces lazy loaded on demand)...

so in this case a languageChange is triggered but the list is incomplete - so not all namespaces get loaded for the new language. The useTranslation will trigger a check on languageChanged event but see a namespace is missing triggering a load for that which throws the Promise and triggers Suspense...

Ok...will need to extract the list of namespaces there and append them to the list of namespaces...

awesome finding...thanks a lot for digging that deep 👏

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jan 8, 2020

could you please test if this would work (more generic code):

  // nextjs / SSR: getting data from next.js or other ssr stack
  if (initialI18nStore && !i18n.initializedStoreOnce) {
    i18n.services.resourceStore.data = initialI18nStore;

    // add namespaces to the config - so a languageChange call loads all namespaces needed
    i18n.options.ns = Object.values(initialI18nStore).reduce((mem, lngResources) => {
      Object.keys(lngResources).forEach(ns => {
        if (mem.indexOf(ns) < -1) mem.push(ns);
      });
      return mem;
    }, i18n.options.ns);

    i18n.initializedStoreOnce = true;
    i18n.isInitialized = true;
  }

if that works I will publish an update containing this

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jan 13, 2020

Any update on this?

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented Jan 13, 2020

change was published in react-i18next@11.3.0

@crash7

This comment has been minimized.

Copy link
Contributor Author

@crash7 crash7 commented Jan 13, 2020

@jamuhl I'm sorry for the delay, I couldn't find the time last week to check :(
Your proposed code works fine.

Thanks for fixing this!

@crash7 crash7 closed this Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.