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

Add useTranslation alternative with dynamic loading #847

Open
wants to merge 2 commits into
base: canary
Choose a base branch
from

Conversation

MoltenCoffee
Copy link

It's basically a combination of the logic inside the DynamicNamespaces component and the useTranslation hook.

Usecase

The t function returned from useTranslation is (in my opinion) the most developer-friendly way to use next-translate, but lacks the functionality of the DynamicNamespaces component. Especially when using localized strings as attributes instead of text nodes, this addition is more natural than using the Trans component.

Content

It's signature is practically the same as the useTranslation hook, except for an additional dynamic parameter like in DynamicNamespaces, and returns an extra ready value, indicating if the namespace has been loaded or not (the developer can use this to implement a fallback themselves).

Readme has been updated accordingly.

aralroca
aralroca approved these changes Apr 29, 2022
Copy link
Owner

@aralroca aralroca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @MoltenCoffee for your contribution! Interesting feature 😊

It would be great to add tests of this functionality! Thanks!

@MoltenCoffee
Copy link
Author

I was busy working on tests for this feature, and whilst most of them are just the same as for the 'regular' useTranslation hook, some involve functionality of the DynamicNamespaces component, of course. I haven't been able to write tests using the dynamic functionality, as I wouldn't really know how to mock that. The DynamicNamespaces doesn't have any tests either, I assume for the same reason.

Any ideas would be appreciated!

@aralroca
Copy link
Owner

As an idea, maybe it would work using the dynamic prop that you added with () => Promise.resolve({ namespace: { text: 'Test'}}). It's not necessary to test the loadLocaleFrom.

I find it interesting to test the differences with useTranslation, the ready and that after ready === true translation works well . It's not necessary to test all the behavior of useTranslation. It's more to verify that what you have implemented nobody breaks it unintentionally in the future. Thanks @MoltenCoffee !

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.

2 participants