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

fixed graceful fallback for keys #1010

Merged
merged 1 commit into from
Nov 30, 2019
Merged

fixed graceful fallback for keys #1010

merged 1 commit into from
Nov 30, 2019

Conversation

ywongau
Copy link
Contributor

@ywongau ywongau commented Nov 29, 2019

t accepts an array of keys and will use first resolved key or fallback to the last key https://www.i18next.com/overview/api#t
But when i18next instance is not ready useTranslation hook simply returns an identity function, t(['doh', 'Human friendly fallback']) gives me ['doh', 'Human friendly fallback'] instead of 'Human friendly fallback'

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 95.261% when pulling 85c60fd on ywongau:master into b2c0421 on i18next:master.

@jamuhl
Copy link
Member

jamuhl commented Nov 29, 2019

Instead of patching around getting a fallback from an array - shouldn't you fix the problem of accessing t before passing down i18n?!?

@ywongau
Copy link
Contributor Author

ywongau commented Nov 29, 2019

Instead of patching around getting a fallback from an array - shouldn't you fix the problem of accessing t before passing down i18n?!?

We are using a dictionary whose value is [key, defaultMessage] for default language and json via xhr for others, which is similar to Message Descriptor in react intl
There are many benefits: Text for default language never fails; key being passed to t can be strongly typed, and no configuration is needed for testing default language if this fix is applied

@ywongau ywongau closed this Nov 29, 2019
@ywongau ywongau reopened this Nov 29, 2019
@jamuhl
Copy link
Member

jamuhl commented Nov 29, 2019

just your fix is https://github.com/i18next/react-i18next/pull/1010/files#diff-d5c6a91c6c8060c38a8b3541ef38332fR20

if (!i18n) ---> this is a mistake in your app that needs a fix ... not a patch

either set i18n via I18nextProvider or use initReactI18next

@ywongau
Copy link
Contributor Author

ywongau commented Nov 30, 2019

ok will close

@ywongau ywongau closed this Nov 30, 2019
@jamuhl
Copy link
Member

jamuhl commented Nov 30, 2019

@ywongau look we can merge this - does not hurt - just configuring your application correctly you should never get into that code section where !i18n -> so the question stays how your code called useTranslation|withTranslation without having i18n passed yet by either I18nextProvider or use initReactI18next

@ywongau
Copy link
Contributor Author

ywongau commented Nov 30, 2019

Thanks, now reopened :)
Of course in my application react-i18next has to be configured correctly otherwise it wouldn't work
Having the message in default language as the last item in the key array and this patch is just to allow us to start using t to handle messages as soon as possible. Messages for default language will just work without any setup upfront

@ywongau ywongau reopened this Nov 30, 2019
@jamuhl jamuhl merged commit b8b6d5e into i18next:master Nov 30, 2019
@jamuhl
Copy link
Member

jamuhl commented Nov 30, 2019

published in react-i18next@11.2.5

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 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants