i18n: update test for new canonical locale codes #6360
Conversation
| // Map of deprecated codes to their canonical version. Depending on the ICU | ||
| // version used to run Lighthouse/this test, these *may* come back as their | ||
| // substitute, not themselves. | ||
| const deprecatedCodes = { |
There was a problem hiding this comment.
Seems like this won't scale if we don't keep up on our deprecation notices. Do we have a transition plan for the future where we won't need to map these? Or will this be necessary for the foreseeable future as we encounter old Node versions and support explicit aliases?
There was a problem hiding this comment.
Alternatively, should we be offering deprecated locales before we even have anyone relying on our locales, or should we just update to canonical now? :)
There was a problem hiding this comment.
Seems like this won't scale if we don't keep up on our deprecation notices.
Ideally it won't have to scale. There's been a few major ICU updates between Node 8 and latest 10, and these are the only locales that have had their getCanonicalLocale response changed. Looking at
https://www.unicode.org/cldr/charts/latest/supplemental/aliases.html
which has all the aliases in the next version of ICU, the only other deprecated one I spot from our list is mo, so we may have to eventually add that here.
Do we have a transition plan for the future where we won't need to map these?
When we move to Node 10 as the minimum required version, our i18n lib will return id even if the user explicitly requests in, so we can safely remove it from locales.js, at which point it can be removed from here.
So, yeah, basically support both until all our users only get one or the other.
There was a problem hiding this comment.
Alternatively, should we be offering deprecated locales before we even have anyone relying on our locales, or should we just update to canonical now? :)
In theory it's not necessary since LR is presumably running on recent Chrome (which has been doing the deprecated mapping for a while now), so, yeah, no actual users can use the deprecated codes until we launch support in the CLI.
@paulirish wanted to keep support in because it's trivial (just a mapping in locales.js) and tbf this test was always just a sanity check that we weren't trying to support locales that a user couldn't ever get. It makes sense to update it to recognize we're still providing the deprecated codes to the subset of users (CLI on Node < 10.7) that can still request them (until they can't anymore).
|
lol, whoops, should have pushed this before #6361, as the Node 10 build on master is now broken 😳 |
Some recent ICU update (maybe in Node 10.7?) now acts upon the recommended deprecation of some locale codes (namely,
in->idandiw->he), and so we have a failure on a test checking that all our supported locales use canonical codes in more recent versions of Node.For now we still support the deprecated codes with explicit aliases in
locales.jsbecause existing versions of Node won't do that extra canonicalization step, so this PR updates the test to recognize both the old and new behavior.