messageformat icon indicating copy to clipboard operation
messageformat copied to clipboard

bug: incorrect locales used in in generated date functions

Open cdaringe opened this issue 4 years ago • 3 comments

Problem

locale es-MX gets en formatted functions

Demonstration

tiny runnable demo here: https://github.com/cdaringe/multi-locale-invalid-dates

it can be seen in just this code too:

const MessageFormat = require("@messageformat/core");
const compileMessageModule = require("@messageformat/core/lib/compile-module");

const messagePacks = {
  "en-US": {
    utilsDate: "{date, date, ::EEEMMMd}",
    utilsToday: "today",
    utilsTomorrow: "tomorrow",
  },
  "es-MX": {
    utilsDate: "{date, date, ::MMMMd}",
    utilsToday: "hoy",
    utilsTomorrow: "mañana",
  },
};

const mf = new MessageFormat("*");
console.log(compileMessageModule(mf, messagePacks));

which emits:

const date_en_MMMMd_meky2n = (function() { // should be es_mx
  var opt = {"month":"long","day":"numeric"}; // need to research if this is correct or not
  var dtf = new Intl.DateTimeFormat("en", opt); // should be "es-MX"
  return function(value) { return dtf.format(value); }
})();
export default {
  "en-US":   {/* snip */},
  "es-MX": {
    utilsDate: (d) => date_en_MMMMd_meky2n(d.date),
    utilsToday: () => "hoy",
    utilsTomorrow: () => "mañana"
  }
}

cdaringe avatar Feb 24 '22 19:02 cdaringe

looks to be that core wants en & es, not the locales 🤔

cdaringe avatar Feb 24 '22 20:02 cdaringe

Confirmed. This mostly affects the '*' special-case locale code. It's not just the date functions, but the locale isn't detected at all from the object path like it should be.

This works like it should:

const mf = new MessageFormat(['en-US', 'es-MX'])
console.log(compileModule(mf, { 'es-MX': { foo: '{x, plural, one{one} other{other}}' } }))
// import { es } from "@messageformat/runtime/lib/cardinals";
// import { plural } from "@messageformat/runtime";
// export default {
//   "es-MX": {
//     foo: (d) => plural(d.x, 0, es, { one: "one", other: "other" })
//   }
// }

But this sort of doesn't:

const mf = new MessageFormat(['en-US', 'es'])
console.log(compileModule(mf, { 'es-MX': { foo: '{x, plural, one{one} other{other}}' } }))
// import { en } from "@messageformat/runtime/lib/cardinals";
// import { plural } from "@messageformat/runtime";
// export default {
//   "es-MX": {
//     foo: (d) => plural(d.x, 0, en, { one: "one", other: "other" })
//   }
// }

On the other hand, this has somewhat surprising results:

const mf = new MessageFormat(['en-US', 'es-MX'])
console.log(compileModule(mf, { 'es': { foo: '{x, plural, one{one} other{other}}' } }))
// import { es } from "@messageformat/runtime/lib/cardinals";
// import { plural } from "@messageformat/runtime";
// export default {
//   es: {
//     foo: (d) => plural(d.x, 0, es, { one: "one", other: "other" })
//   }
// }

I need to look at this in more detail and review how exactly this auto-detection is documented, but that'll probably be sometime next week. In the interim, using an explicit and exactly matching list of locales should work.

eemeli avatar Feb 24 '22 20:02 eemeli

Thanks for the reply @eemeli :)

This works like it should

We actually tested this case, and believe it to not work per expectation. If you use my provided demonstration and replace '*' with [en-US, es-MX] and use date skeletons, you can see the unexpected output continue. @dpchamps found that because es-MX is not in the plurals language pack, core uses the en defaultLocale fallback.

We have the case which we want to support, using different translations for the same lang, but different locales:

compileModule(mf, { 'es-MX': { foo: /* ... */ }, 'es-ES': { foo: /* ... */ }), which is why we used the browser-style locale format, as opposed to the 2 or 3 char language code.

@dpchamps has a patch he'll propose to you. Maybe if you want to halt supporting us here, we can continue the conversation in the inbound pull request (landing shortly), that would be cool 😎 .

cdaringe avatar Feb 24 '22 20:02 cdaringe