-
-
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
Pre-check namespaces to avoid unnecessary initial null
render
#336
Conversation
src/I18n.js
Outdated
@@ -29,9 +29,12 @@ export default class I18n extends Component { | |||
this.options.wait = false; | |||
} | |||
|
|||
const { language } = this.i18n; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only need to check the current language, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i18next loads more than the i18next.language -> i18next.language reflects the detected language - while i18next.languages reflects what get loaded and what will be used to lookup translations (fallbacks, whitelist,...): https://github.com/i18next/i18next/blob/master/src/i18next.js#L135
so check for i18next.languages[0] if that is set (hasResourceBundle) it should have loaded or if like to be very save check all i18next.languages
src/I18n.js
Outdated
@@ -29,9 +29,12 @@ export default class I18n extends Component { | |||
this.options.wait = false; | |||
} | |||
|
|||
const { language } = this.i18n; | |||
const ready = !!language && this.namespaces.every(ns => this.i18n.hasResourceBundle(language, ns)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some of the tests, i18n.language
was undefined. Under what real-world circumstances is that possible? Does this logic with !!language
look ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm...language should only be empty if not init with resources while initialising or when unable to detect lng
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing it's an idiosyncrasy of the tests/mocking, but if language
is null, hasResourceBundle
throws an error, so I'll keep this check 👍
awesome...published with react-i18next@6.1.0 thanks for adding this to react-i18next |
Fixes #324
I'm adding line notes with questions 👍