Wrong or missing key is not highlighted when there is a fallback

[SaraRavasio]

🐛 Bug Report

Wrong or missing key is not highlighted when there is a fallback.

To Reproduce

Expected behavior

In both cases I'm referencing a key that doesn't exist, but in the first case I have a fallback, and the wrong key is not highlighted. In the second case, where i don't have a fallback, I get it highlighted.

Your Environment

• "i18next": "^23.5.1"
• "react-i18next": "^13.2.2"
• "i18next-resources-for-ts": "^1.3.3"

i18next.d.ts

import Resources from './resources';

declare module 'i18next' {
  interface CustomTypeOptions {
    defaultNS: 'myfile';
    resources: Resources;
  }
}

Resources generated following this step

[adrai]

Yes, that's exactly how it should behave.
For the first example: since you defined a default value, it's all ok, because at runtime you will see the default translation value.
For the second example: you did not define a default value, that's not ok, because at runtime you will see the i18n key and no translation

[adrai]

It is also tested to behave exactly like that: https://github.com/i18next/i18next/blob/master/test/typescript/custom-types/t.test.ts#L78-L82

[SaraRavasio]

but then what's the point of having a validation, if i'm not alerted that a key doesn't exist?

fallbacks should be more for cases when you can't load the translations due to internet connection perhaps?

[adrai]

It was a requirement to have TS not complain when defining a defaultValue...
If you want you can suggest a new PR that introduces a new option that would change that behaviour.

[SaraRavasio]

Yeah I think it would just make sense to have an option to highlight wrong/missing keys even though there is a default value to fall back to.
These default values are used when translations can't be loaded in general, not only if the key is missing, so I really don't see how one thing should exclude the other 😄

[adrai]

Feel free to create a pull request.

[SaraRavasio]

I've spent a few hours on this topic now, and I have to say, I'm not really sure what to do, i'm missing a lot of context of what other functionalities are doing and I wouldn't really feel confident touching this code too much.

But while investigating, I've also discovered another problem with the change that was introduced here.

I think the change of having "just string" when there's a default value was rushed a bit, would love to have more insights on how to improve this tho :)

[adrai]

Sorry, but I'm not a TS expert. :-(

Maybe @pedrodurek @janpaepke @marcalexiei @hussainahmad @jtbandes @henrikvolmer @JounQin @nicegamer7 can help?

[janpaepke]

Hi Sara,
I think we might have a different understanding as to what the error highlight means. To me it makes sense to report an error if there is no actual meaningful string value that would be displayed.

If you supply a default value this should be considered valid by i18n-next and thus not report an error. As far as i18n-next is concerned, the code is fine.

If you are looking for vars which are missing from your translation files even though you supplied a default value, you might want to have a look at the available parsers (see docs) or maybe there's an eslint plugin which could report a warning. If there's not, you could consider creating one, if your usecase is important enough...

[nicegamer7]

It's possible to make TS throw an error for extraneous options, I just never opened a PR for this because I wasn't sure if it was useful to anyone.

I can open a PR that adds a new option to change the behaviour. Is that preferred, or should we leave this as-is?

[adrai]

It's possible to make TS throw an error for extraneous options, I just never opened a PR for this because I wasn't sure if it was useful to anyone.

I can open a PR that adds a new option to change the behaviour. Is that preferred, or should we leave this as-is?

Extraneous options should still be possible. There might be other i18next plugins or postprocessors that needs this.

But an option to toggle the TS validation when defaultValue is used, would be a nice addition.

[SaraRavasio]

@janpaepke the default value is not only used when there's no key, but also when translations can't be loaded: instead of showing the translation keys, the default value is being displayed.
That being said, I don't see the point of having a key validation to show us if a key is missing, if we just don't care about it if we have a default value, the two things are not really connected with each other. You have a very specific workflow in which you start by not having the keys and then adding them, but that's not usually the case.
Also, what if someone by mistake removes a character from the key, because there's a default value i shouldn't be alerted that translations are using a wrong/missing key?

I think these behaviors should at least coexist, but i wouldn't completely remove the logic to check for the existence of the key and just bulk accept a random string.

Also, I'm not sure why we wouldn't want a check for the exact variables to be passed, I would just assume that being the best type safety, but maybe i'm missing something here 😅

Btw, I'm basing this behavior also having seen how flutter is integrated with i18next.

[janpaepke]

My point is essentially: If a default is provided, it's not an error if no value exists. That's the whole point of a default value: providing a fallback.

Just think of any function where you provide a default value for a parameter. You wouldn't expect typescript to complain if you don't provide one, when you use the function, right?

So to your point:

Also, what if someone by mistake removes a character from the key, because there's a default value i shouldn't be alerted that translations are using a wrong/missing key?

Yes, that's exactly what should happen, because it will show the default.

Remember: Key-checking isn't a miracle weapon that prevents you from doing stupid things. It only checks the default translation file. So for example if you only provide a key in the default lang and then forget to add it in other languages, only the closest fallback will be displayed. No error will be shown, even with strict key checking.

That being said, I agree that a point can be made to opt into strict key checking, which should be feasible with an option in i18next.d.ts, similar to how allowObjectInHTMLChildren works.

Wether or not this warrants the extra complexity in the code is for the maintainers to decide and likely depends on how many others also think that this is needed.

[SaraRavasio]

You wouldn't expect typescript to complain if you don't provide one, when you use the function, right?

Absolutely agree, but in this case, we're talking about a type safety check, so i agree that we cover the case when the key doesn't exist by showing the default value, but here we're talking about checking if that key exists from a "type" point of view.

That being said, I agree that a point can be made to opt into strict key checking, which should be feasible with an option in i18next.d.ts, similar to how allowObjectInHTMLChildren works.

that would be great! i think we should cover both cases, just becasue i think it's something that kinda makes sense anyway 😄 i'd love to help adding this, but i'm not feeling confident enough to do it myself right now. would be amazing if i could have some support here

[janpaepke]

Optional (or even required) parameters don't exist in vanilla javascript. So the compiler complaining (or not) when providing or missing function parameters is very much a "type safety check" and purely from the "type" point of view.

Regarding implementing this as an option I unfortunately lack the time to support.
You might want to have a look at the option(s) I mentioned and how they are implemented. That might give you some pointers. Best of luck!

[adrai]

Still waiting for someone who wants to try a PR without breaking the existing behaviour.

[SaraRavasio]

i haven't had any time to look into this unfortunately 😓

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[adrai]

@marcalexiei do you like to have a look at this?

[marcalexiei]

I'll try to wrap things up to see if I have understand the problem and the proposed solution:

the idea is to provide an option inside CustomTypeOptions to optionally enable type checking also on TOptions#defaultValue and TFunction 2nd parameter to be sure that the fallback key exists in available Resources?

[adrai]

Ok for you @SaraRavasio ?

[PookMook]

Just adding, as someone very new to this project, that @SaraRavasio point resonates with me. I would expect to be able to get flagged if a key is missing, even if fallback is provided.
If this is via an option or something I can setup in my project once to get that behavior, this is fine enough if documented properly. I guess you don't want to break existing workflow and I get that, but probably this should be a sane default for newer projects (have it in the documentation or in your examples)

To add my pinch of salt, feel free to tell me this is another issue that needs to be filed, but interpolated keys should also fail if the options object is not provided at all (first line of the "wrong").
We are implementing i18n + locize currently, and we currently have a rule to always include an object as second parameter, even if empty, to have peace of mind that TS would catch mistakes.

Interpolation in the default value (the 2&3 lines in the wrong) don't matter to us much, we have a rule to never get fallbacks (to keep TS checks). but if it could be correctly checked, we could get back to provide defaultValue.

And while on the subject, it also never consider count to be a missing key in option, even for plural keys, but that's a very nice to have compared to what's discussed here