FbtTranslations: add support for modifying registered translations #208
Conversation
facebook-github-bot
added
the
CLA Signed
|
I'm actually not opposed to exposing this for other purposes, but for what you're trying to solve, you'll still want some "merging" logic to ensure existing translations in the same locale aren't lost. In that implementation, you'd perform function mergeTranslations(newTranslations) {
Object.keys(newTranslations).forEach(locale => {
Object.assign(this.translatedFbts[locale], newTranslations[locale]);
});
}Thoughts? |
|
@jrwats Absolutely agree. This was actually my second proposal in my head (but I was thinking |
|
Yes. That'd be fine. |
mrtnzlml
changed the title
|
@jrwats I updated the PR. Could you please have a look? |
runtime/nonfb/FbtTranslations.js
Outdated
| if (translatedFbts[locale] === undefined) { | ||
| translatedFbts[locale] = {}; | ||
| } | ||
| Object.assign(translatedFbts[locale], newTranslations[locale]); |
There was a problem hiding this comment.
minor nit:
translatedFbts[locale] = Object.assign(
translatedFbts[locale] ?? {},
newTranslations[locale],
);There was a problem hiding this comment.
That's unfortunately not possible because it would not be merging the translations into translatedFbts but rather into an empty unrelated object. Tests are covering this case, you can try it.
There was a problem hiding this comment.
But Object.assign(...) will return its result and we're setting the entry in my suggestion above? Just tested in the imported diff — seems to work.
PASS fbt-runtime packages/fbt/lib/__tests__/FbtTranslations-test.js
There was a problem hiding this comment.
🙈 You are right! I completely missed the part that you are suggesting to assign the value back via translatedFbts[locale] = …
My apologies. I updated the PR with your suggestion.
| @@ -14,7 +14,7 @@ import type {FbtRuntimeCallInput, FbtTranslatedInput} from 'FbtHooks'; | |||
|
|
|||
| const FbtHooks = require('FbtHooks'); | |||
|
|
|||
| let translatedFbts = null; | |||
| let translatedFbts: TranslationDict = {}; | |||
|
@jrwats has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@mrtnzlml has updated the pull request. You must reimport the pull request before landing. |
This change adds a new function `getRegisteredTranslations` into `FbtTranslations` which allows us to retrieve registered translations. For convenience, it also adds a new `mergeTranslations` function that allows us to append the translations. Use-case: I have an application with FBT and a design system with its own FBT as well. Unfortunately, registering translations overwrites any translations previously registered so the design system (or the application) cannot register their own translations without conflicting with the other part. One solution could be to get already registered translations and re-register them modified. Another solution is to merge these two translations together. There are many other possible solutions so I am open to suggestions how to make it better.
|
@mrtnzlml has updated the pull request. You must reimport the pull request before landing. |
This change adds a new function
getRegisteredTranslationsintoFbtTranslationswhich allows us to retrieve registered translations. For convenience, it also adds a newmergeTranslationsfunction that allows us to append the translations.Use-case: I have an application with FBT and a design system with its own FBT as well. Unfortunately, registering translations overwrites any translations previously registered so the design system (or the application) cannot register their own translations without conflicting with the other part. One solution could be to get already registered translations and re-register them modified. Another solution is to merge these two translations together.
There are many other possible solutions so I am open to suggestions how to make it better.