rxaviers / globalize-webpack-plugin

Globalize.js webpack plugin
Other
33 stars 27 forks source link

invalid messages should be skipped #25

Open alunny opened 8 years ago

alunny commented 8 years ago

This might need changes to globalize-compiler as well, but I wanted to get your thoughts on a possible approach.

For certain locales, we've been getting invalid ICU message strings back from our translators. For instance.

{
  "Hello": "Bon jour",
  "(name) and You": "{{name} et Vous"
}

When we do a production build, because {{name} et Vous is an invalid string, messageformat.js throws an error, which is propagated up to the top of the tree, and no bundle is created for the locale. We would much rather catch this error and show the default language's message to our users for this message than show the entire app in the default language.

compileExtracts in globalize-compiler doesn't have a reference to the project's default Globalize locale (only the current one, which is called defaultLocale in that file), so it would need some changes. We would want to change the generated formatters from (e.g):

function anonymous(Globalize
/**/) {
return [Globalize.messageFormatter('Hello'), Globalize.messageFormatter('(name) and You')];
}

to something like:

function anonymous(Globalize, _defaultGlobalize
/**/) {
var formatters = [];

try {
  formatters.push(Globalize.messageFormatter('Hello');
} catch {
  // log error
  formatters.push(_defaultGlobalize.messageFormatter('Hello');
}

try {
  formatters.push(Globalize.messageFormatter('(name) and You');
} catch {
  // log error
  formatters.push(_defaultGlobalize.messageFormatter('(name) and You');
}

return formatters;
}

or a more compact version of the same.

@rxaviers any ideas?

rxaviers commented 8 years ago

Oh, translators... :)

Your approach sounds sane to me, which is: let globalize-compiler handle the fallback: we try to create the message-formatter using locale X, otherwise we try to use the default locale (equivalent to Globalize(defaultLocale).messageFormatter(<message>)). Intriguingly, this is going to make more sense for react-globalize users than for raw globalize users, because the concept of default messages is present on react-globalize only (yet).

I'm still trying to reason "who" should be held responsible for this issue: webpack plugin, the compiler, or Globalize itself. At the compiler, lib/compiler will compile anything (any formatter, any locale, given the appropriate data are loaded), but lib/compile-extracts is currently made for a one-locale basis (although, pretty easy to extend/generalize).

Considering we are going to handle that at the compiler (as suggested), I think having the fallback as the defaultLocale is a good first step. I'm wondering if the chosen solution could later be extended to be made more generic. For example, it should cover the following scenario: let's suppose an application has several translations; a missing locale could be better replaced as fallback of another available translation instead of the default locale; for example, another Cyrillic script locale for a missing Russian, where the fallback logic could come from http://www.unicode.org/reports/tr35/#LanguageMatching.

Let's use fallbackLocale as the second argument? So we have something like function anonymous(Globalize, fallbackLocale..., where fallbackLocale is going to be defaultLocale now. Later we could extend it to become fallbackLocales, an Array of locales, where LanguageMatching could be used to define the order of the fallbacks.

Should we have that concept of fallbackLocales at Globalize itself?