feat(v2): infer default i18n locale config from locale code #4449
Conversation
|
@longlho can you help review this please? |
| return new Intl.DisplayNames([locale], {type: 'language'}).of(locale); | ||
| } | ||
| return locale; | ||
| } |
There was a problem hiding this comment.
@longlho particularly not sure of this part.
It seems Intl.DisplayNames does not exist on Node 13. Can you help me figure out where to check the NodeJS support of Intl apis?
Also not sure how to make TS happy
There was a problem hiding this comment.
Intl.DisplayNames is ES2021 which is not yet published so it won't be in TS native type def for a while
There was a problem hiding this comment.
Please ignore the rest of the diff, this is mostly for this code that I pinged you 😆
Thanks, didn't know.
So is it safe to consider this API is available in Node14? Not sure where to check that exactly
There was a problem hiding this comment.
yeah as long as MDN says it is then you're good. I wouldn't check for node version but rather check for if (!!Intl.DisplayNames) or something like that
There was a problem hiding this comment.
Thanks, will do that.
It seems I can get this kind of infos on the v8 site directly: https://v8.dev/features/intl-displaynames
Seems supported since Node 14+, so I think we'd rather recommend using Node 14 and not just node >= 13 for Docusaurus i18n, as it's also a LTS
Thanks for the review
There was a problem hiding this comment.
Just for curiosity, why is this stage 4 in 2020 and not in ES2020?
Apparently, it's not even ES2021 but 2022?
https://tc39.es/ecma402/#intl-displaynames-objects
|
[V1] Deploy preview success Built with commit 3e693e2 |
|
Deploy preview for docusaurus-2 ready! Built with commit 3e693e2 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4449--docusaurus-2.netlify.app/classic/ |
|
Size Change: +195 B (0%) Total Size: 598 kB
ℹ️ View Unchanged
|
|
[V1] Deploy preview success Built with commit 2e1cc49 |
|
Deploy preview for docusaurus-2 ready! Built with commit 2e1cc49 |
|
@slorber can you explain the use case a bit more? default locale has a lot of implication in terms of negotiation (e.g if I support |
|
@longlho default locale is just the one we use in priority if no locale is provided by the cli - - locale option, we don't have a fallback system for now |
Motivation
In most situations, we shouldn't need to provide a
i18n.localeConfigsbecause the locale direction and locale labels can generally be inferred from the locale itself, considering it is compliant with BCP 47 locales and Node 14 Intl API is able to get language labels.Also made it possible to run
yarn start --locale faeven iffais not in the config: it will just print a warning instead of throwing.Have you read the Contributing Guidelines on pull requests?
yes
Test Plan
tests and v2 site