Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(i18n): incorporate 52 languages into messages pipeline #5781

Merged
merged 8 commits into from
Aug 7, 2018

Conversation

paulirish
Copy link
Member

@paulirish paulirish commented Aug 4, 2018

picking up on #5780...

Overview

  1. All language messages files are added now. The fallback story is indicated in locales/index.js
    • I'm deleting the message files that are duplicated because fallback. Later, we can exclude them from the BUILD/export.
  2. The default locale is changed to 'en'.
    • AFAICT this is equivalent to 'en-US'. (This seems crazy to me, since all en-* (that's not en-US) do things Dates/Times differently.) But anyway. I like having the fallback be less specific.
  3. various en-* locales are called out and mapped to en-IE, which we're treating like a en-GB (eg. "Minimise" instead of "Minimize"). This makes sure these locales get localized number formatting and not american style.
  4. CLI won't validate your chosen locale anymore. We have a fallback story, so we won't block someone trying with 'es-MX'.
  5. I don't know how important it is that the LH.Locale extern is updated, but.. it is.

FWIW: Our bundle size for lighthouse-background was 1.5MB before we started i18n. Once we had en-us/en-XA/ar-XB, it was 1.6MB. With all these locales, it's now 2.5MB. There's a couple optimizations we could make to improve that.

Oops those were sizes before our 'minification' step. dist/scripts/lighthouse-background.js went from 349KB to 504KB.

@paulirish paulirish changed the title core(i18n): incorporate 52 languages into messages pipeline core(i18n): incorporate 52 languages into messages pipeline Aug 4, 2018
Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

I don't know how important it is that the LH.Locale extern is updated, but.. it is.

that's good

Oops those were sizes before our 'minification' step. dist/scripts/lighthouse-background.js went from 349KB to 504KB.

you can't change the locale in the extension or devtools yet. Should we exclude from the bundle for now?

'vi': require('./vi.json'),
'zh-CN': require('./zh-CN.json'),
'zh-HK': require('./zh-HK.json'),
'zh-TW': require('./zh-TW.json'),
Copy link
Member

Choose a reason for hiding this comment

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

on looking more, it seems like we should have a 'zh' that uses 'zh-CN', but we can investigate more

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

wooooo!

@@ -64,6 +64,7 @@ function getFlags(manualArgv) {
],
'Configuration:')
.describe({
// We don't allowlist specific locales. Why? So we can support the user who requests 'es-MX' (unsupported) and we'll fall back to 'es' (supported)
Copy link
Member

Choose a reason for hiding this comment

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

we might want a command for listing available locales, but something for another day :)

@brendankenny brendankenny merged commit e40f77e into master Aug 7, 2018
@brendankenny brendankenny deleted the tcroll-integrate branch August 7, 2018 18:54
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.

2 participants