@s-ui/i18n: Allow monitorize missing translation keys

[oriolpuig]

Package

@s-ui/i18n@1

Description

A long time ago, we decided to create the @s-ui/i18n package by forking the node-polyglot npm library instead of wrapping it and getting the new library updates more easily than now. During this time, we have been iterating @s-ui/i18n with some little customizations that now make it more difficult to update the version with the new node-polyglot one.

One of the useful things the node-polyglot library has and we have not, is to allow @s-ui/i18n consumers to use observability tools to track which translation key does not have a translation and find translation issues on the fly. Our library only writes a console.warn to logo it into the client console browser.

Steps to Reproduce

1. Open your desired browser
2. Open the browser console
3. Go to a Fotocasa Ad detail
4. Filter the console logs by Missing and you will se something like the image bellow

Expected behavior: Allow @s-ui/i18n consumers to track when a translation key is missing. We have 3️⃣ approaches:

• 1️⃣ When onMissingKey callback is configured, prevent writing the console.warn, as is on node-polyglot at this line
• 2️⃣ Send the onMissingKey and write the console.warn at the same time.
• 3️⃣ Send the onMissingKey (in case of been configured) and add a flag logMissingKey to prevent writing the console.warn.

[!WARNING]
🙏🏻 Please, let me know which approach you like: 1️⃣ , 2️⃣ or 3️⃣ .

Current behavior: When a translation key is missing, a console.warn appears in the browser console.

Additional Information

We can disable the console.warn to avoid printing those warnings on the browser console, but in my opinion is better to empower teams to track and detect the translation issues.

[carlosvillu]

I would say the second approach. But Maybe we can avoid log warnings in production.

[marcbenito]

2️⃣ too

[jelowin]

@oriolpuig, great job, my KUDOS 😍

I agree with you and also with @carlosvillu. I think it's important for the teams to detect translation issues but maybe we could avoid show warnings on production to try to keep the production console cleaner. So I prefer 2️⃣

On the other hand, I understand that you want to avoid using @s-ui/i18n package and only make a wrapper of node-polyglot, am I right?

[oriolpuig]

@jelowin On the other hand, I understand that you want to avoid using @s-ui/i18n package and only make a wrapper of node-polyglot, am I right?

No, we can't override the current version of node-polyglot because our forked version was iterated on our side, si if we override now, we may cause breaking changes in how our tools are using this library.

What I'm proposing is picking this onMissingKey feature from node-polyglot and putting it in our library. But NOT override all.

Thanks for your Kudos!

[oriolpuig]

@carlosvillu I would say the second approach. But Maybe we can avoid log warnings in production.

Yes, actually it's not possible to pass a flag to let us skip this warning, but we can do it with a hack in our portals 👀 by doing this i18n.adapter.instance.allowMissing = true. It's not exactly what we want, but we can achieve it.

Reviewing your proposals, we can add a new prop avoidMissingKeyWarn: true|false to conditionally this from outside. What do you think?

cc/ @jelowin

[jordevo]

excellent contribution, @oriolpuig !

Not sure if I'm missing some context here and perhaps my comment doesn't make sense at all but... if the idea is to capture those error messages from i18n, couldn't we just set up our FE logger so that it also sends or collects console.warn messages? 🤔

[oriolpuig]

Not sure if I'm missing some context here and perhaps it doesn't make sense at all but... if the idea is to capture those error messages from i18n, couldn't we just set up our FE logger so that it also sends or collects console.warn messages? 🤔

Hello @jordevo, thanks for your time!

First of all, as a developer, I think warnings do not make sense to be displayed in production. To get it, we can add a specific flag (for example: avoidMissingKeyWarn: true|false), to skip in production, test CI, or wherever you desire.

But, why do I suggest allowing us to monitor those warnings? I have 2 scenarios in mind:

• Yes, we can keep those warnings in our lab environments (test or local), but for scenario 2, we may lose some cases.
• Sometimes we are building translation keys from production API responses, for example: CAR_MODEL_${car.typeId}. You are using a dynamic API response value to pass it to our translation dictionary and may not exist because we are passing an uncontrolled undefined or another value not translated.

For those 2 reasons, I suggest to enable this feature and empower teams to monitor uncontrolled translations.

Next Monday we can discuss it in a quick call with some specific demos if you need it, and add here some screenshots to enforce this proposal.

Have a nice weekend! ❤️

[jordevo]

gotcha, @oriolpuig !

I'd even go for a third approach as you kind of point out in your response...

3️⃣ call onMissingKey if configured and allow for console.warn to be configured so that it can be sent or not (ie to be logMissingKey: true on development but logMissingKey: false on production

does it make sense to you?