-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Suspence is fired during lang change when useTranslation called in between #975
Comments
I guess the assumptions was that However it can be called simply due to other reasons, and then during namespace loading we end up with triggering suspence at https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L91 I don't know what is the best way to fix that, but we need another condition just before throwing promise, like |
You mix things: bindI18n -> tells when to force a rerender (default rerender always when language was changed) In which cases does this happen...? https://github.com/i18next/react-i18next/blob/master/src/utils.js#L68 The check returns ready if at least fallback lng was loaded...in your case do you have no fallback or do a languageChange with additional load of new namespace?!? |
This is my i18n setting, I have lng and fallback lang both set, and this issue is not for initial language load, thats fine that we have spinning fallback rendered in this scenario. I just don't want to have spinning on language change. I just edited the official React example from this repository: import i18n from 'i18next';
import Backend from 'i18next-xhr-backend';
import { initReactI18next } from 'react-i18next';
i18n
.use(Backend)
.use(initReactI18next)
.init({
fallbackLng: 'en',
lng: 'en',
debug: true,
interpolation: {
escapeValue: false,
},
react: {
bindI18n: 'languageChanged', // i am changing this as I dont wanna any flickering on language change
},
}); I also updated all libs to latest versions. Now, the default example implementation works fine, as in this video https://nimb.ws/OHdSfi However, once I update this component in this way: function Page() {
const { t, i18n } = useTranslation();
const [x, incX] = useState(0);
const changeLanguage = lng => {
incX(x + 1);
i18n.changeLanguage(lng);
};
return (
<div className="App">
<div className="App-header">
<img src={logo} className="App-logo" alt="logo" />
<Welcome />
<button onClick={() => changeLanguage('de')}>de</button>
<button onClick={() => changeLanguage('en')}>en</button>
</div>
<div className="App-intro">
<MyComponent />
</div>
<div>{t('description.part2')}</div>
</div>
);
} the outome is flickering https://s.nimbusweb.me/share/3459712/7y4m64vr5lwr0s68eyt6 |
why you set state inside changeLanguage...remove that should work |
let me try to reproduce this - will have to see what's going on |
@jamuhl this is just an example showing the problem, it could also happen that
thx, let me know if you have any problems in reproduction |
This bug also would happen even if I didnt close dropdown inside
Generally this bug is happening when:
Looking at the code this is not suprising, but I guess that surprise is that |
ok...this is correct -> ready is correctly false as that namespace is not loaded in the new language -> binding but I also see your wish to not trigger it on language change... the problem lies in the check for ready -> i18next sets i18next.language and load resources after setting so (this was requested as some users rely on reading out i18next.language immediately). Some code there also depends on that language being set. Handling the previousLanguage check on react-i18next seems overly complicated - so what I might try now is adding a mode to i18next that allows that i18next.language gets set after loading those translations -> which would result in a rerender triggered outside of our code to be still checking on the old language set |
I agree that handling this in React would be complicated, it potentially would require keeping some additional state inside I18nProvider itself. Personally I would consider just to add another field to Then, we could have another check inside Potentially we could have another React option, so some people would like to retain current behaviour, and others would like to show previous language content until current is being loaded. |
Currently, I got the solution up...setting i18next.language after loaded...so this runs your use case well - which is what I would prefer too in my app - but loosing the ability we had in the past to trigger a Suspense during languageChanging event... Guess I need to add an attribute to i18next Hm...looks like this will come out with major versions on i18next and react-i18next |
Good you figured it out so quickly, thanks! I have 2 use cases I would like to ask though, generally about loading new namespaces (those which werent loaded for any language yet. Please confirm it will work like I described. With those adjustment, what would happen in the following case:
And also in this case:
|
first case: yes, Suspense will be triggered |
Can you try with: react-i18next@11.0.0 |
It works now on example repository, thanks! I will try to update the whole app to be sure now, it also has multiple namespaces plus lazy loaded components. Nice idea with adding |
Great to hear...please report any issues you encounter...👍 |
This works so great now, I used 1 suspence like in example to catch common translations which is only triggered on load, plus extra suspenses for components which can have additional namespaces. No flickering at all, it should boost performance very much as I had many useless rerenders (I mitigated this issue before by fallback unfound keys i18n to empty string :) The only thing which stopped working is my AdblockDetector for some reason ; o const AdblockWarning = () => {
const addWrapper = useRef(null);
const dispatch = useDispatch();
useEffect(() => {
if (addWrapper.current && addWrapper.current.clientHeight === 0) {
dispatch(addMessage('addBlockDetected', null, null, 20000));
}
}, []);
return (
<div ref={addWrapper}>
<div className="adBanner" style={{ height: 1 }} />
</div>
);
}; But I dont think I can blame this library for it :) Probably this is Suspence affecting this component. Anyway, I am closing as really I cannot find anything wrong now, I dont have any warning for missing i18n either. I will reopen if I find anything new. |
Awesome...and: If you like this module don’t forget to star this repo. Make a tweet, share the word or have a look at our https://locize.com to support the devs of this project -> there are many ways to help this project 🙏 |
I starred this and many i18n addons already, you deserve it! Keep doing great job :) Btw, I am having some ideas regarding boosting performance, like creating some plugin for generating unique translation names based on content so we could use browser caching with infinitive TTL. Or maybe a plugin to allow collocating translation files with components. I cannot wait to contribute, will do after I finish my opensource library. |
@jamuhl Recently I added SSR to one of my app and it turned out that Suspense again is triggered on lang change. It works without SSR, but once this is added, then there are 2 strange behaviours I noticed:
If that is not enough information I could provide some demo, but generally adding SSR should reproduce this. If you prefer to raise this as a separate issue, let me know too. And, which might be helpful: // client
i18next
.use(initReactI18next)
.use(i18nextXHRBackend)
.init({
fallbackLng: false,
lng: lang,
defaultNS: 'common',
ns: [],
debug: process.env.NODE_ENV !== 'production',
load: 'languageOnly',
backend: {
loadPath: '/static/{{lng}}/{{ns}}.json',
},
interpolation: {
escapeValue: false,
},
react: {
bindStore: '',
},
});
// server
i18next
.use(backend)
.use(LanguageDetector)
.init({
whitelist: ['en', 'de'],
fallbackLng: false,
preload: ['en', 'de'],
debug: process.env.NODE_ENV !== 'production',
load: 'languageOnly',
ns: ['common', 'anotherNamespace'],
defaultNS: 'common',
backend: { loadPath: 'src/locales/{{lng}}/{{ns}}.json' },
detection: {
order: ['path'],
},
react: {
bindStore: '',
},
}); |
Hard to say...SSR works but is "hard" to get right...you might have a look at next-i18next for inspiration... do you pass down initialLanguage, initialStore? https://react.i18next.com/latest/ssr |
yes, everything works apart from this Suspense triggering on lang change. So I guess I should prepare minimal reproducing repo right? |
Would help...just currently do not expect a fast resolution as I will be offline more or less the next two weeks (starting on monday). |
Ok, no problem, thx for the info. I will prepare the demo in the meantime and try to see what's going on |
@jamuhl Here you can see minimal example, for now showing problem even with 1 namespace, instructions in readme -> https://github.com/klis87/react-i18n-ssr-example Probably there is sth wrong with |
I will dig in the code, maybe I will find a potential culprit |
thank you for providing the sample - will dig into it tomorrow - already late here now. |
Indeed it is quite late, so I am finishing for today too. At first glance I cannot see anything, but logically the following must be the issue: https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L41 for some reason must be |
Can you retry with react-i18next@11.3.3 The languageChanged event was triggered too early as there was no namespace to load on languageChange -> there was a little mistake when copying namespaces over: 0828bfa#diff-3f70de7c1ba388c739ba6409eaaca576L1012 now with this change on languageChange loading should happen and the event for languageChanged will be triggered after having loaded the needed translations -> no Suspence triggered as hasLoadedNamespace will return true now |
@jamuhl looks like it works, even with multiple namespaces, thx! |
If you like this module don’t forget to star this repo. Make a tweet, share the word or have a look at our https://locize.com to support the devs of this project. If you liked my support / work - I would enjoy a coffee sponsored using the “:purple_heart:Sponsor Button”. There are many ways to help this project 🙏 |
Describe the bug
I don't want Suspence to render
fallback
during language change, hence I usebindI18n: 'languageChanged'
. It does not work properly though, as in practise Suspense still rendersfallback
, which causes app flickering on lang change. It happens when for exampleuseTranslation
is called during namespaces for new lang are loading due to other reason than lang change, like state update for instance.Just imagine oficial example https://github.com/i18next/react-i18next/blob/master/example/react/src/App.js#L25
with additional hook
const [x, incX] = useState(0);
andSuspence will be triggered despite the
bindI18n: 'languageChanged'
option. It works fine without incrementingx
though.Occurs in react-i18next version
"react-i18next": "10.1.2+"
To Reproduce
Steps to reproduce the behavior:
Expected behaviour
Suspence should not be hit in this case.
OS (please complete the following information):
The text was updated successfully, but these errors were encountered: