feat: introduce i18n to error page #669
Conversation
|
Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖 Please select which version do you want to release:
And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.
|
|
Beep boop 🤖 I noticed you didn't make any changes at the
In order to keep track, I'll create an issue if you decide now is not a good time
|
| @@ -0,0 +1 @@ | |||
| {} | |||
There was a problem hiding this comment.
shouldn't we need those translations before merging the PR?
There was a problem hiding this comment.
For now it just falls back to english so nothings is changed
There was a problem hiding this comment.
Folks, I'm concerned with one aspect of these changes: a lot of times Render breaks is when it's going through its lifecycle that, among other things, injects the locale in the context.
This means that on real-world scenarios, we might just render the title IDs on the page with this approach in case the language is not available. This is why most of these strings are hardcoded and do not rely on react-intl.
There is a function that uses the english strings as fallback, the function could also handle an undefined locale and fall back to english. Isn't that enough? |
anitavincent
force-pushed
the
errorpage-i18n
branch
from
50649da
to
53cbe5b
Compare
If we do have a fallback mechanism in place, yes, we're good 😉 |
There was a problem hiding this comment.
@anitavincent , why aren't we using the messages builder instead?
|
|
||
| export function getMessages(locale: string) { | ||
| const localeMessages = (locale && messages[locale]) || {} | ||
| const fallbackMessages = messages['en-US'] |
There was a problem hiding this comment.
This is key and it answers @iago1501's question ❤️
There was a problem hiding this comment.
@marcelovicentegc If it's because of the fallback we could simply use the available languages in vtex.store as an example of languages that we should take care of in stores, for example https://github.com/vtex-apps/store/tree/master/messages
or talking about admin, the ones that we have in vtex.admin-pages: https://github.com/vtex-apps/admin-pages/tree/master/messages
My main concern is that using the message's builder is one of our patterns when talking about translations, we have the messages folder and all translations are inside it, with this approach, talking about scaling, if any other component needs translation, we would have to add a messages folder for the component instead of centering in a single place and this logic would be replicated 🤔
There was a problem hiding this comment.
I see your point @iago1501, but given that this application didn't have translations until this day while being around for many years, I wouldn't be concerned about adopting a pattern that we might not need by the end of the day, and that we know it's the entry point for one of the most frequent errors we experience in the Admin given that the messages service itself didn't scale as we expected.
For this same reason, it occurs to me that we should rather not rely on the messages service nor its builder, specially on a mission critical component such as this application, when we could otherwise organize translations where needed (the error component) and skip all complexity that using the messages builder involves (triggering the messages service lifecycle just to fetch some strings stored elsewhere while it could just to be packaged with the application itself?).
It occurs to me that using the messages service could break the error component itself quite frequently.
What does this PR do? *
introduces i18n to error page
How to test it? *
To check full page error: https://anita--teamadmin.myvtex.com/admin
If your locale is pt-br some strings should be in portuguese (but not all)
for client side error:
https://anita1--teamadmin.myvtex.com/admin/io-manager/explode/?forceLogs=true