Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception on not existing key or value - is this the wanted behavior? #6

Closed
ilanbm opened this issue Nov 29, 2020 · 10 comments · Fixed by #7
Closed

Exception on not existing key or value - is this the wanted behavior? #6

ilanbm opened this issue Nov 29, 2020 · 10 comments · Fixed by #7

Comments

@ilanbm
Copy link

ilanbm commented Nov 29, 2020

If a key does not exist in the namespace, or if the value is empty, t function raises an exception.

The key can be debatable, but why would an empty value raise an exception?

Is there a default value to set if there is no key/value?

@amannn
Copy link
Owner

amannn commented Nov 30, 2020

Thanks for the report!

If a key does not exist in the namespace

Do you mean a scenario like this?

function Component() {
  const t = useTranslations('Component');

  return <Text>{t('nonExistingKey')}</Text>
}
// en.json
{
  "Component": {
    "existingKey": "Hello"
  }
}

Currently next-intl would throw here and I think that's helpful to fix the error. What would be your expected behaviour?

if the value is empty

That's true, I guess we could avoid the error here. However I'm wondering when you'd practically want to use an empty label? Why would you want a translation for that? Isn't this a situation where the developer might has forgot to set up a message?

Is there a default value to set if there is no key/value?

What would you want to put there? Something like "Sorry, no translation available currently."? I'm wondering if that's really helpful to the user. If you have a locale where you haven't completed all translations yet, you could deep merge messages from the default locale to have a fallback (if this is desired – you wouldn't get errors for missing translations then).

@ilanbm
Copy link
Author

ilanbm commented Dec 2, 2020

Currently next-intl would throw here and I think that's helpful to fix the error. What would be your expected behaviour?

Well I think it is wrong to consider that an error, not having a key is legit. It is not code, it can come from various sources and in different languages.
Besides that, its ok not to have a key and expect a predefined default value (that's the common case in most of the other libraries)

That's true, I guess we could avoid the error here. However I'm wondering when you'd practically want to use an empty label? Why would you want a translation for that? Isn't this a situation where the developer might has forgot to set up a message?

Nope, you can't narrow it to a case where a developer/translator forgot to set a value to a key, there are various scenarios of having a wanted empty value. It caused me to look for workarounds here, ended using a transparent char... it my case some keys in some languages don't have a known translation so if there is no value I use the English translation. So having an empty value is a wanted scenario.

What would you want to put there? Something like "Sorry, no translation available currently."? I'm wondering if that's really helpful to the user. If you have a locale where you haven't completed all translations yet, you could deep merge messages from the default locale to have a fallback (if this is desired – you wouldn't get errors for missing translations then).

Keep it simple and predictable, return exactly what it is: an empty string.

@ilanbm
Copy link
Author

ilanbm commented Dec 2, 2020

I have also lots of cases where the empty value is there until translated in the future (where the English texts are not final, but are live. and I expect showing nothing at that paragraph in another language until it will be translated in the future.

@ilanbm
Copy link
Author

ilanbm commented Dec 2, 2020

(empty is not a bug, its a feature) :)

@amannn
Copy link
Owner

amannn commented Dec 2, 2020

Thanks for explaining! Well I guess it's ok to accept empty messages, since as you explained it's quite explicit. I'll add a PR for this.

Besides that, its ok not to have a key and expect a predefined default value (that's the common case in most of the other libraries)

Do you have a link that describes this feature in other libraries? For the missing key I feel stronger about keeping the error.

it my case some keys in some languages don't have a known translation so if there is no value I use the English translation. So having an empty value is a wanted scenario.

You can use fallback messages from a different locale like this:

const messages = deepMerge(defaultMessages, userLocaleMessages);

@amannn
Copy link
Owner

amannn commented Dec 2, 2020

This is fixed in next-intl@0.1.1.

Please let me know though about the question from above how other libraries handle missing keys with default values.

Thank you by the way for giving the library a shot and providing feedback! This is really helpful to better understand use cases of others.

@ilanbm
Copy link
Author

ilanbm commented Dec 2, 2020

thanks for the fix!

about missing key, I think they all do raise an exception on missing key, but I think most of them offer a default fallback value at the translate function/component itself.

I didn't check but I think i18next shows the key itself when it doesn't find it

Anyway I do think it should raise an exception on missing key, but should have a way to have a default value, and also a flag 'raise exception on missing keys"

@amannn amannn mentioned this issue Dec 3, 2020
4 tasks
@amannn
Copy link
Owner

amannn commented Dec 3, 2020

Thanks again for your feedback! I did some research and I agree that we should be able to keep the app running in more instances, like when messages are missing.

I've wrote up a detailed plan on how I think errors could be handled: #8.

Would you like to have a look and let me know if this sounds reasonable from your perspective?

@ilanbm
Copy link
Author

ilanbm commented Dec 3, 2020

sure, checking

@amannn
Copy link
Owner

amannn commented Mar 31, 2023

@eug-vs has shared his solution for merging messages and adding a prefix in #231.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants