Bind i18n.t #1682
Merged
Bind i18n.t #1682
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This facilitates using t via destructuring or as a callback. Several people have run into this; see #1287.
This fixes some test failures in some time zones (such as Eastern US).
|
I would take @adrai approach and bind all functions https://github.com/adrai/i18next-next/blob/main/src/i18next.js#L624 On the other hand (my personal opinion): I'm still in the opinion class functions should not be bound but called in the context of the used instance. But - If people love to deconstruct everything - I won't get in the way. |
|
@joshkel please have a look at this: #1681 (comment) |
|
@joshkel would you like to adapt your PR accordingly? |
|
@adrai Done. I apologize for the delayed response. |
|
Thank you, it's included in v21.4.0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I recently ran into #1287, and based on comments there, I saw that I'm not the only one. (In my case, I was trying to write a lower-level function that could take a
tfunction either fromuseTranslationor from a I18n instance.) Since this does seem to be a somewhat common error, and since destructuringtor trying to use it as a callback can be useful, I believe it would make sense to bind thetfunction within the I18n constructor (similar to how React class components can bind their callbacks) so thatthisis always defined, regardless of how thetfunction is used.While working on this PR, I fixed some typos, and I noticed that tests were failing, so I set a fixed timezone to help address that. (I can split these changes into separate PRs if you prefer.)
Thank you.
Checklist
npm run test