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

i18n: add better support for the default locale in bundles #13211

Merged
merged 3 commits into from
Oct 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions lighthouse-core/config/config-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ function resolveSettings(settingsJson = {}, overrides = undefined) {
// 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).
// TODO: could do more work to sniff out the user's locale
Copy link
Member Author

Choose a reason for hiding this comment

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

longtime TODO in i18n.js but seemed better here where the locale is actually chosen

const locale = i18n.lookupLocale((overrides && overrides.locale) || settingsJson.locale);

// Fill in missing settings with defaults
Expand Down
9 changes: 5 additions & 4 deletions lighthouse-core/lib/i18n/i18n.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ const lookupClosestLocale = require('lookup-closest-locale');
const {getAvailableLocales} = require('../../../shared/localization/format.js');
const log = require('lighthouse-logger');
const {LH_ROOT} = require('../../../root.js');
const {isIcuMessage, _formatMessage} = require('../../../shared/localization/format.js');

const DEFAULT_LOCALE = 'en';
const {
isIcuMessage,
_formatMessage,
DEFAULT_LOCALE,
} = require('../../../shared/localization/format.js');

const UIStrings = {
/** Used to show the duration in milliseconds that something lasted. The `{timeInMs}` placeholder will be replaced with the time duration, shown in milliseconds (e.g. 63 ms) */
Expand Down Expand Up @@ -127,7 +129,6 @@ function lookupLocale(locales) {
throw new Error('Lighthouse must be run in Node with `Intl` support. See https://nodejs.org/api/intl.html for help');
}

// TODO: could do more work to sniff out the user's locale
const canonicalLocales = Intl.getCanonicalLocales(locales);

// Filter by what's available in this runtime.
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/config/config-helpers-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,9 @@ describe('.mergePlugins', () => {
describe('.resolveSettings', () => {
it('resolves the locale', () => {
const settings = resolveSettings({locale: 'zh-CN'});
// COMPAT: Node 12 only has 'en' by default.
// COMPAT: Node 12 only has 'en-US' by default.
if (isNode12SmallIcu()) {
expect(settings.locale).toEqual('en');
expect(settings.locale).toEqual('en-US');
return;
}
expect(settings.locale).toEqual('zh');
Expand Down
20 changes: 10 additions & 10 deletions lighthouse-core/test/lib/i18n/i18n-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,46 +39,46 @@ describe('i18n', () => {
});

it('falls back to default if locale not provided or cant be found', () => {
expect(i18n.lookupLocale(undefined)).toEqual('en');
expect(i18n.lookupLocale(invalidLocale)).toEqual('en');
expect(i18n.lookupLocale([invalidLocale, invalidLocale])).toEqual('en');
expect(i18n.lookupLocale(undefined)).toEqual('en-US');
expect(i18n.lookupLocale(invalidLocale)).toEqual('en-US');
expect(i18n.lookupLocale([invalidLocale, invalidLocale])).toEqual('en-US');
});

it('logs a warning if locale is not available and the default is used', () => {
const logListener = jest.fn();
log.events.on('warning', logListener);

expect(i18n.lookupLocale(invalidLocale)).toEqual('en');
expect(i18n.lookupLocale(invalidLocale)).toEqual('en-US');

// COMPAT: Node 12 logs an extra warning that full-icu is not available.
if (isNode12SmallIcu()) {
expect(logListener).toBeCalledTimes(2);
expect(logListener).toHaveBeenNthCalledWith(1, ['i18n',
expect.stringMatching(/Requested locale not available in this version of node/)]);
expect(logListener).toHaveBeenNthCalledWith(2, ['i18n',
`locale(s) '${invalidLocale}' not available. Falling back to default 'en'`]);
`locale(s) '${invalidLocale}' not available. Falling back to default 'en-US'`]);
return;
}

expect(logListener).toBeCalledTimes(1);
expect(logListener).toBeCalledWith(['i18n',
`locale(s) '${invalidLocale}' not available. Falling back to default 'en'`]);
`locale(s) '${invalidLocale}' not available. Falling back to default 'en-US'`]);

log.events.off('warning', logListener);
});

it('falls back to root tag prefix if specific locale not available', () => {
// COMPAT: Node 12 only has 'en' by default.
// COMPAT: Node 12 only has 'en-US' by default.
if (isNode12SmallIcu()) {
expect(i18n.lookupLocale('es-JKJK')).toEqual('en');
expect(i18n.lookupLocale('es-JKJK')).toEqual('en-US');
return;
}

expect(i18n.lookupLocale('es-JKJK')).toEqual('es');
});

it('falls back to en if no match is available', () => {
expect(i18n.lookupLocale(invalidLocale)).toEqual('en');
it('falls back to en-US if no match is available', () => {
expect(i18n.lookupLocale(invalidLocale)).toEqual('en-US');
});
});
});
61 changes: 47 additions & 14 deletions shared/localization/format.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const {isObjectOfUnknownValues, isObjectOrArrayOfUnknownValues} = require('../ty
/** Contains available locales with messages. May be an empty object if bundled. */
const LOCALE_MESSAGES = require('./locales.js');

const DEFAULT_LOCALE = 'en-US';

/**
* The locale tags for the localized messages available to Lighthouse on disk.
* When bundled, these will be inlined by brfs.
Expand Down Expand Up @@ -189,14 +191,16 @@ function _formatMessage(message, values = {}, locale) {
* @return {string}
*/
function _localizeIcuMessage(icuMessage, locale) {
const localeMessages = LOCALE_MESSAGES[locale];
if (!localeMessages) throw new Error(`Unsupported locale '${locale}'`);
const localeMessages = _getLocaleMessages(locale);
const localeMessage = localeMessages[icuMessage.i18nId];

// Fall back to the default (usually the original english message) if we couldn't find a
// message in the specified locale. This could be because of string drift between
// Lighthouse versions or because new strings haven't been updated yet. Better to have
// an english message than no message at all; in some cases it won't even matter.
// Use the DEFAULT_LOCALE fallback (usually the original english message) if we couldn't
// find a message in the specified locale. Possible reasons:
// - string drift between Lighthouse versions
// - in a bundle stripped of locale files but running in the DEFAULT_LOCALE
// - new strings haven't been updated yet in a local dev run
// Better to have an english message than no message at all; in some cases it
// won't even matter.
if (!localeMessage) {
return icuMessage.formattedDefault;
}
Expand All @@ -209,9 +213,10 @@ function _localizeIcuMessage(icuMessage, locale) {
* @return {Record<string, string>}
*/
function getRendererFormattedStrings(locale) {
const localeMessages = LOCALE_MESSAGES[locale];
if (!localeMessages) throw new Error(`Unsupported locale '${locale}'`);
const localeMessages = _getLocaleMessages(locale);

// If `localeMessages` is empty in the bundled and DEFAULT_LOCALE case, this
// will be empty and the report will fall back to the util UIStrings for these.
const icuMessageIds = Object.keys(localeMessages).filter(f => f.startsWith('report/'));
/** @type {Record<string, string>} */
const strings = {};
Expand Down Expand Up @@ -349,21 +354,48 @@ function replaceIcuMessages(inputObject, locale) {
return icuMessagePaths;
}

/**
* Returns the locale messages for the given `locale`, if they exist.
* Throws if an unsupported locale.
*
* NOTE: If DEFAULT_LOCALE is requested and this is inside a bundle with locale
* messages stripped, an empty object will be returned. Default fallbacks will need to handle that case.
* @param {LH.Locale} locale
* @return {import('./locales').LhlMessages}
*/
function _getLocaleMessages(locale) {
const localeMessages = LOCALE_MESSAGES[locale];
if (!localeMessages) {
if (locale === DEFAULT_LOCALE) {
// If the default locale isn't in LOCALE_MESSAGES, this is likely executing
// in a bundle. Let the caller use the fallbacks available.
return {};
}
throw new Error(`Unsupported locale '${locale}'`);
}

return localeMessages;
}

/**
* Returns whether the `requestedLocale` can be used.
* @param {LH.Locale} requestedLocale
* @return {boolean}
*/
function hasLocale(requestedLocale) {
// The default locale is always supported through `IcuMessage.formattedDefault`.
if (requestedLocale === DEFAULT_LOCALE) return true;

const hasIntlSupport = Intl.NumberFormat.supportedLocalesOf([requestedLocale]).length > 0;
const hasMessages = Boolean(LOCALE_MESSAGES[requestedLocale]);

return hasIntlSupport && hasMessages;
}

/**
* Returns a list of canonical locales (each of which may have aliases, but those would
* only show in getAvailableLocales)
* Returns a list of canonical locales, as defined by the existent message files.
* In practice, each of these may have aliases in the full list returned by
* `getAvailableLocales()`.
* TODO: create a CanonicalLocale type
* @return {Array<string>}
*/
Expand All @@ -375,13 +407,13 @@ function getCanonicalLocales() {
* Returns a list of available locales.
* - if full build, this includes all canonical locales, aliases, and any locale added
* via `registerLocaleData`.
* - if bundled and locale messages have been stripped (locales.js shimmed), this includes no
* locales (perhaps available in a separate bundle), and perhaps any locales
* from `registerLocaleData`.
* - if bundled and locale messages have been stripped (locales.js shimmed), this includes
* only DEFAULT_LOCALE and any locales from `registerLocaleData`.
* @return {Array<LH.Locale>}
*/
function getAvailableLocales() {
return /** @type {Array<LH.Locale>} */ (Object.keys(LOCALE_MESSAGES).sort());
const localesWithMessages = new Set([...Object.keys(LOCALE_MESSAGES), DEFAULT_LOCALE]);
return /** @type {Array<LH.Locale>} */ ([...localesWithMessages].sort());
}

/**
Expand All @@ -407,6 +439,7 @@ function getIcuMessageIdParts(i18nMessageId) {
}

module.exports = {
DEFAULT_LOCALE,
_formatPathAsString,
collectAllCustomElementsFromICU,
isIcuMessage,
Expand Down
6 changes: 6 additions & 0 deletions shared/localization/locales.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
* Google locale inheritance rules: https://goto.google.com/ccssm
* CLDR language aliases: https://www.unicode.org/cldr/charts/latest/supplemental/aliases.html
* CLDR locale inheritance: https://github.com/unicode-cldr/cldr-core/blob/master/supplemental/parentLocales.json
*
* For Lighthouse bundles that shouldn't include locale data, the recommended pattern
* is to replace the default export of this file with `{}` so that no locale messages
* are included. Strings will work normally through the IcuMessage.formattedDefault
* fallback, and locale messages can be added on demand (e.g. dynamically fetched)
* through `format.registerLocaleData()`.
Copy link
Member Author

Choose a reason for hiding this comment

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

not sure who will ever look in here, but worth documenting somewhere :)

*/

// TODO(paulirish): Centralize locale inheritance (combining this & i18n.lookupLocale()), adopt cldr parentLocale rules.
Expand Down
44 changes: 44 additions & 0 deletions shared/test/localization/format-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,54 @@ const path = require('path');

const format = require('../../localization/format.js');
const i18n = require('../../../lighthouse-core/lib/i18n/i18n.js');
const constants = require('../../../lighthouse-core/config/constants.js');
const locales = require('../../localization/locales.js');

/* eslint-env jest */

describe('format', () => {
describe('DEFAULT_LOCALE', () => {
it('is the same as the default config locale', () => {
expect(format.DEFAULT_LOCALE).toBe(constants.defaultSettings.locale);
});
});

describe('#getAvailableLocales', () => {
it('has all the available locales', () => {
const availableLocales = format.getAvailableLocales();
for (const locale of ['en', 'es', 'ru', 'zh']) {
expect(availableLocales).toContain(locale);
}

const rawLocales = Object.keys(locales).sort();
expect(availableLocales.sort()).toEqual(rawLocales);
});

it('contains the default locale', () => {
expect(format.getAvailableLocales()).toContain(format.DEFAULT_LOCALE);
});
});

describe('#getCanonicalLocales', () => {
it('contains some canonical locales', () => {
const canonicalLocales = format.getCanonicalLocales();
for (const locale of ['en-US', 'es', 'ru', 'zh']) {
expect(canonicalLocales).toContain(locale);
}
});

it('is a subset of the available locales', () => {
const canonicalLocales = format.getCanonicalLocales();
const availableLocales = format.getAvailableLocales();

for (const canonicalLocale of canonicalLocales) {
expect(availableLocales).toContain(canonicalLocale);
}

expect(canonicalLocales.length).toBeLessThan(availableLocales.length);
});
});

describe('#_formatPathAsString', () => {
it('handles simple paths', () => {
expect(format._formatPathAsString(['foo'])).toBe('foo');
Expand Down