-
Notifications
You must be signed in to change notification settings - Fork 227
[React I18n] I18n for partners #2589
Comments
cc @trishrempel @movermeyer @carolopolo Summarizing the discussion we had earlier in this issue so we can keep track of the discussions. Feel free to move the issue to a more appropriate place :) |
[Disclaimer: I'm a n00b in this area] Why? Does it have to be? Any links you could share? |
This is so that the logic for figuring out what translations is available based on the configured locales (with fallback replacements) and the current user/shop locale is all encapsulated in the backend instead of being replicated on the front end for each UI Extension surface area (Checkout, Admin, POS, Customer Accounts, etc...). It's also better for performance as we only need to send back a single flattened translation blob instead of all possible languages. Note that we're only talking about flattening the translation JSON. This is not related to the actual You can read more about it in the tech design. |
I'm not sure I see how sending a single language is related to flattening the whole thing? |
Since we want to reduce the amount of data sent the translations are flattened with fallback logic for filling in missing keys. If you look at the Tech Design there's an example in there that's pretty clear. locales/en-GB.json {
"main": {
"colorPickerPrompt": "Please select a colour"
}
} locales/en.json {
"main": {
"colorPickerPrompt": "Please select a color"
},
"footer": {
"privacyPolicyLabel": "Privacy Policy"
}
} Final flattened translation when the requested language is {
"main.colorPickerPrompt": "Please select a colour",
"footer.privacyPolicyLabel": "Privacy Policy"
} |
I still don't understand the need to flatten, as this could be done as easily with the original format? {
"main": {
"colorPickerPrompt": "Please select a colour"
},
"footer": {
"privacyPolicyLabel": "Privacy Policy"
}
} Which is what we do in customer accounts. The backend takes all the sources (defaults, themes overrides, fallback, etc) and creates a single set of non-flattened translations, injected into the DOM. This is then compatible out-of-the-box with Is it a limitation of the GraphQL schema? |
No, we can return in any format from GraphQL we want but the flattened structure is simpler and eliminate the need to do recursions as the translate function only need to take in a single key. It also match more closely with how the extension is expected to use the translation cc @kumar303 who might have more to add |
Is it? Does the format even matter as we're going to provide the API to use the translation anyway, right? 🤔
I'm still not sure I get it. If the i18n's translate function already takes a single key without any recursion needed with nested JSON, then which function are you referring to?
It doesn't match how the translations are defined though, which seems odd. 😕 Note that I don't really care if it's flattened or not, as long as the final translate implementation can handle both (maybe check for the whole key first, then split it and see if it's available in any nested object). I'm just wondering why we're going through this flattening process as it's unclear right now. |
Reading the different documents and PRs related to this, I've noticed something that might explained the confusion:
I understand the merging process and its benefits, it's the flattening process that seems unnecessary, or I was not able to find the requirement explaining the need to flatten the keys. I won't be linking to the sources since this issue is public while all sources are private. |
Note that this merging won't work for things like pluralization, since in order to choose which pluralization key to use, you need to know the locale of the keys you're picking from. e.g., If you've fallen back to |
The flattening / unnesting was done for a few reasons (maybe more 🤔):
FWIW, these are all implementation details. One could implement
Checkout flattens / unnests the pluralization key, too: {
"main.colorPickerPrompt": "Please select a colour",
"main.colorPickerSelection.one": "You have selected {{count}} colour",
"main.colorPickerSelection.other": "You have selected {{count}} colours"
} The |
Thanks for the answers! As an extensibility dev on customer-accounts, a common i18n API meant for extension sandboxes sounds great. That said, I still don't get why we would change the format? It sounds like we're mixing the merging and flattening process into the same argument, but from my point of view, they're different processes, each with their own concerns and caveats. So let me share my understanding below. Flattening (unnesting)
This could be an argument for merging multiple locales into a single dataset. Though it's a counter-argument for flattening as repeated key prefixes makes the dataset actually heavier (memory and parsing).
This also seems to be related to merging data sources into a single one. Flattened or not, fallbacks could be used similarly in the same dataset.
This is the first argument I read for flattening yet. Was there any noticeable impact on performance that could explain the new format? Isn't the flattening process impacting performance in some ways as well? Merging multiple locales
@movermeyer was referring to merging different locales pluralization keys. It would not work unless we're aware of the locale used for the fallback, which is lost during the merging process. Flattening isn't really a concern here. I'm onboard with merging different sources into a single dataset, as long as it's all under the same language tag (e.g.
However, there are some caveats with merging anything else for fallbacks:
Alternative?Based on these 2 example files: // locales/en-GB.json
{
"main": {
"colorPickerPrompt": "Please select a colour"
}
} // locales/en.json
{
"main": {
"colorPickerPrompt": "Please select a color"
},
"footer": {
"privacyPolicyLabel": "Privacy Policy"
}
} The Query response could be defined like this: {
"data": {
"uiExtensions": [
{
"locale": "en-GB",
"translations": {
"main": {
"colorPickerPrompt": "Please select a colour"
}
},
"fallbackLocale": "en",
"fallbackTranslations": {
"footer": {
"privacyPolicyLabel": "Privacy Policy"
}
}
}
]
}
} This has all the informations to be able to properly translate using the |
Ah, yeah. Good catch ✨ I see that @movermeyer explained the problem here, too: https://github.com/Shopify/shopify/pull/339144
This seems fine to me. As mentioned earlier, these are all implementation details (i.e. partners won't have to change code). Implementors wouldn't need to follow the exact same approach but, of course, it would be nice to only inflict one set of bugs on our partners 😅. |
Creating this issue as a follow up on the Segmentation project migrating templates to a 1P app. This migration also adds i18n to the new Web sandbox runtime to allow apps to run:
A custom implementation of i18n was added in the
sandbox
to be able to pass a synchronous function to handle translation replacements.Reasons we could not use the exact implementation of react i18n:
Limitations of the current checkout-web implementation (the Admin implementation for UI Extensions is almost the same)
{{}}
instead of{}
as placehoders. It could be more flexible and accept both. Thereact-i18n
translate function can take ainterpolate
function to support any of them.scope
parameterThe text was updated successfully, but these errors were encountered: