Skip to content

Add localeCodeFromKey option#386

Merged
eemeli merged 3 commits intomasterfrom
plural-from-key
Mar 11, 2023
Merged

Add localeCodeFromKey option#386
eemeli merged 3 commits intomasterfrom
plural-from-key

Conversation

@eemeli
Copy link
Member

@eemeli eemeli commented Feb 10, 2023

Fixes #359
See also #360

Ping @cdaringe, @dpchamps, would this approach work for you?

@eemeli
Copy link
Member Author

eemeli commented Feb 10, 2023

Ping @jantimon, I think this would also fix #328?

// input
const pluralCodeFromKey = key => key === '0' ? 'en' : key === '1' ? 'de' : null
const mf = new MessageFormat(['en', 'de'], { pluralCodeFromKey })
compileModule(mf, ['{count, plural, other{EN}}', '{count, plural, other{DE}}'])
// output
import { de, en } from "@messageformat/runtime/lib/cardinals";
import { plural } from "@messageformat/runtime";
export default {
  "0": (d) => plural(d.count, 0, en, { other: "EN" }),
  "1": (d) => plural(d.count, 0, de, { other: "DE" })
}

@eemeli eemeli linked an issue Feb 10, 2023 that may be closed by this pull request
@dpchamps
Copy link
Contributor

Changes look good! I like this more explicit arbitrary mapping strategy as opposed to assuming normalization. Thanks!

Copy link
Contributor

@cdaringe cdaringe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @eemeli! Technically I agree this would fix our issue! 🎉 I do have a suggestion on the interface name/semantics.

I'd be happy to even send the patch. I think your solution--putting the user in control--is good and elegant.

@eemeli
Copy link
Member Author

eemeli commented Mar 11, 2023

Been punting this until "later" for a while, but ultimately I agree with the option rename suggested by @cdaringe. It wasn't so much the shape of the change that was tricky here, but the scope of what it doesn't do.

Specifically, this PR doesn't actually fix the exact issue that was originally reported in #359, which used the '*' locale and locale codes that include a region specifier. For usage like that, the best that's available is to set something like

const localeCodeFromKey = (key) => key === 'es-MX' ? 'es' : null

and then you'd get es rather than es-MX formatting.

However, the issue is fixed for language+region locale codes if the input list of locales is explicit, i.e. something like ['en-us','es-MX'].

If someone's interested in improving the former without breaking the API, then I'd be willing to consider a PR on it. But for now, I'm going to add the option as localeCodeFromKey, and package a new release with it.

@eemeli eemeli changed the title Add pluralCodeFromKey option Add localeCodeFromKey option Mar 11, 2023
@eemeli eemeli merged commit 9d3c5e6 into master Mar 11, 2023
@eemeli eemeli deleted the plural-from-key branch March 11, 2023 11:37
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 this pull request may close these issues.

bug: incorrect locales used in in generated date functions Compile only a single translation

3 participants