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.

Sign up for GitHub

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

Jump to bottom

i18n: accept array of locales in lookupLocale #11349

Merged
merged 5 commits into from
Aug 31, 2020
Merged

i18n: accept array of locales in lookupLocale #11349

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c4d6ebe
i18n: set default locale outside lookupLocale
connorjclark Aug 31, 2020
532a85d
locales
connorjclark Aug 31, 2020
f133516
simpler
connorjclark Aug 31, 2020
c584f44
gravy
connorjclark Aug 31, 2020
0e12346
yaaaay
connorjclark Aug 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lighthouse-core/config/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ class Config {
// If a locale is requested in flags or settings, use it. A typical CLI run will not have one,
// however `lookupLocale` will always determine which of our supported locales to use (falling
// back if necessary).
const locale = i18n.lookupLocale((flags && flags.locale) || settingsJson.locale);
const locale = i18n.lookupLocale((flags && flags.locale) || settingsJson.locale) || 'en';
connorjclark marked this conversation as resolved.
Show resolved Hide resolved

// Fill in missing settings with defaults
const {defaultSettings} = constants;
Expand Down
7 changes: 3 additions & 4 deletions lighthouse-core/lib/i18n/i18n.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,18 +157,17 @@ const formats = {
* Look up the best available locale for the requested language through these fall backs:
* - exact match
* - progressively shorter prefixes (`de-CH-1996` -> `de-CH` -> `de`)
* - the default locale ('en') if no match is found
*
* If `locale` isn't provided, the default is used.
* If `locale` isn't provided or one could not be found, undefined is returned.
* @param {string=} locale
* @return {LH.Locale}
* @return {LH.Locale|undefined}
*/
function lookupLocale(locale) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

paul suggested perhaps we should accept an array of locales here and do the "try the next value" in LH core.

// TODO: could do more work to sniff out default locale
const canonicalLocale = Intl.getCanonicalLocales(locale)[0];

const closestLocale = lookupClosestLocale(canonicalLocale, LOCALES);
return closestLocale || 'en';
return closestLocale;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/lib/i18n/swap-locale.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function swapLocale(lhr, requestedLocale) {
// Copy LHR to avoid mutating provided LHR.
lhr = JSON.parse(JSON.stringify(lhr));

const locale = i18n.lookupLocale(requestedLocale);
const locale = i18n.lookupLocale(requestedLocale) || 'en';
const {icuMessagePaths} = lhr.i18n;
/** @type {string[]} */
const missingIcuMessageIds = [];
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/config/config-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ describe('Config', () => {
// Don't assert specific locale so it isn't tied to where tests are run, but
// check that it's valid and available.
assert.ok(config.settings.locale);
assert.strictEqual(config.settings.locale, i18n.lookupLocale(config.settings.locale));
assert.strictEqual(config.settings.locale, 'en');
});

it('uses config setting for locale if set', () => {
Expand Down