PolymerElements / app-localize-behavior

Polymer behaviour to help internationalize your application
48 stars 55 forks source link

localize() function returns empty string if key not found #22

Closed a-katsarov closed 8 years ago

a-katsarov commented 8 years ago

Description

The localize() function does not return nothing if the key cannot be matched in the resources object.

Expected outcome

It is better to return the original key in the original language. It is always better to have a string in English (if it is the original language) than no string.

Actual outcome

Returns empty string or null (not sure).

Steps to reproduce

Just pass some random string to localize().

Browsers Affected

Tested on:

a-katsarov commented 8 years ago

Here is the error when a string is not localized:

Uncaught TypeError: A message must be provided as a String or AST.

Which is normal for an empty string. And of course the error "breaks" the whole component.

a-katsarov commented 8 years ago

That seams to work :-)

msg = new IntlMessageFormat(resources[language][key] || key, language, formats);
addyosmani commented 8 years ago

At two minds about this as you can either consider empty string as a fail state and handle yourself, but agree supplying back the original key than no string at all.

felixzapata commented 8 years ago

hi, I think another option can be return the value in a default language. For example, if you set that the default language is "en" and you try to find a key in French.. if the key in French does not exist, the component will return the value in English. Only if the key does not exist in the default language, you return the original key or an empty string.

a-katsarov commented 8 years ago

I am used to gettext, which is used in the most free software programs. It just returns the untranslated key. In that manner if the translation is for an app, it can be usable, even not fully translated.

a-katsarov commented 8 years ago

Think of the case of failure fetching the translation .json. The component will throw an error and most likely not work. But if we return the original key, we will get non translated, but working component.

fbndsprz commented 8 years ago

Hi, Temporary fixed on my side this way (inspired by @webfacebg) :

__computeLocalize: function(language, resources, formats) {
    var proto = this.constructor.prototype;

    // Everytime any of the parameters change, invalidate the strings cache.
    proto.__localizationCache.messages = {};

    return function() {
      var key = arguments[0];
      var defaultValue = arguments[1];
      if (!key || !resources || !language)
        return;

      // Cache the key/value pairs for the same language, so that we don't
      // do extra work if we're just reusing strings across an application.
      var messageKey = key + resources[language][key];
      var msg = proto.__localizationCache.messages[messageKey];

      if (!msg) {
        msg = new IntlMessageFormat(resources[language][key] || defaultValue, language, formats);
        proto.__localizationCache.messages[messageKey] = msg;
      }

      var args = {};
      for (var i = 2; i < arguments.length; i+=2) {
        args[arguments[i]] = arguments[i+1];
      }

      return msg.format(args);
    };
  },

and I call localize this way:

localize(stringKey, defaultValue, param1Name, param1Value, param2Name, param2Value)

I find it visually cleaner than displaying the key.

a-katsarov commented 8 years ago

Hm. I would prefer the key itself. This means I should put the key twice. In most cases it is easier to use the defaultString for a key. And we weould get something like this:

localize("Hello, {name}!", "Hello, {name}!", "name", "Batman")

If you intent to use dafaultValue, then I would prefer:

      if (!msg) {
        msg = new IntlMessageFormat(resources[language][key] || defaultValue || key, language, formats);
        proto.__localizationCache.messages[messageKey] = msg;
      }

And then:

localize("Hello, {name}!",  null, "name", "Batman")

Or even simpler when I do not need params:

localize("Hello, World!")
mvolkmann commented 8 years ago

Here is a new pull request for this where my multiple commits are squashed to one. https://github.com/PolymerElements/app-localize-behavior/pull/43

notwaldorf commented 8 years ago

Fixed in #43: there is a new attribute that can return the key itself. We can make this the default behaviour in the future if people like this.