Skip to content

Commit af0bb33

Browse files
authored
clients(devtools): only use locales that have locale files to download (#13214)
1 parent e039f77 commit af0bb33

File tree

4 files changed

+59
-27
lines changed

4 files changed

+59
-27
lines changed

build/build-bundle.js

+6-22
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,12 @@
1313
const fs = require('fs');
1414
const path = require('path');
1515
const assert = require('assert').strict;
16-
const stream = require('stream');
1716
const mkdir = fs.promises.mkdir;
1817
const LighthouseRunner = require('../lighthouse-core/runner.js');
1918
const exorcist = require('exorcist');
2019
const browserify = require('browserify');
2120
const terser = require('terser');
2221
const {minifyFileTransform} = require('./build-utils.js');
23-
const {LH_ROOT} = require('../root.js');
2422

2523
const COMMIT_HASH = require('child_process')
2624
.execSync('git rev-parse HEAD')
@@ -32,9 +30,6 @@ const audits = LighthouseRunner.getAuditList()
3230
const gatherers = LighthouseRunner.getGathererList()
3331
.map(f => './lighthouse-core/gather/gatherers/' + f.replace(/\.js$/, ''));
3432

35-
const locales = fs.readdirSync(LH_ROOT + '/shared/localization/locales/')
36-
.map(f => require.resolve(`../shared/localization/locales/${f}`));
37-
3833
// HACK: manually include the lighthouse-plugin-publisher-ads audits.
3934
/** @type {Array<string>} */
4035
// @ts-expect-error
@@ -67,23 +62,7 @@ async function browserifyFile(entryPath, distPath) {
6762
})
6863
// Transform the fs.readFile etc into inline strings.
6964
.transform('@wardpeet/brfs', {
70-
/** @param {string} file */
71-
readFileTransform: (file) => {
72-
// Don't include locales in DevTools.
73-
if (isDevtools(entryPath) && locales.includes(file)) {
74-
return new stream.Transform({
75-
transform(chunk, enc, next) {
76-
next();
77-
},
78-
final(next) {
79-
this.push('{}');
80-
next();
81-
},
82-
});
83-
}
84-
85-
return minifyFileTransform(file);
86-
},
65+
readFileTransform: minifyFileTransform,
8766
global: true,
8867
parserOpts: {ecmaVersion: 12},
8968
})
@@ -107,6 +86,11 @@ async function browserifyFile(entryPath, distPath) {
10786
bundle.ignore(require.resolve('../report/generator/report-assets.js'));
10887
}
10988

89+
// Don't include locales in DevTools.
90+
if (isDevtools(entryPath)) {
91+
bundle.ignore(require.resolve('../shared/localization/locales.js'));
92+
}
93+
11094
// Expose the audits, gatherers, and computed artifacts so they can be dynamically loaded.
11195
// Exposed path must be a relative path from lighthouse-core/config/config-helpers.js (where loading occurs).
11296
const corePath = './lighthouse-core/';

clients/devtools-entry.js

+14-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const lighthouse = require('../lighthouse-core/index.js');
99
const RawProtocol = require('../lighthouse-core/gather/connections/raw.js');
1010
const log = require('lighthouse-logger');
1111
const {lookupLocale} = require('../lighthouse-core/lib/i18n/i18n.js');
12-
const {registerLocaleData} = require('../shared/localization/format.js');
12+
const {registerLocaleData, getCanonicalLocales} = require('../shared/localization/format.js');
1313
const constants = require('../lighthouse-core/config/constants.js');
1414

1515
/** @typedef {import('../lighthouse-core/gather/connections/connection.js')} Connection */
@@ -60,6 +60,17 @@ function listenForStatus(listenCallback) {
6060
log.events.addListener('status', listenCallback);
6161
}
6262

63+
/**
64+
* Does a locale lookup but limits the result to the *canonical* Lighthouse
65+
* locales, which are only the locales with a messages locale file that can
66+
* be downloaded and then used via `registerLocaleData`.
67+
* @param {string|string[]=} locales
68+
* @return {LH.Locale}
69+
*/
70+
function lookupCanonicalLocale(locales) {
71+
return lookupLocale(locales, getCanonicalLocales());
72+
}
73+
6374
// For the bundle smoke test.
6475
if (typeof module !== 'undefined' && module.exports) {
6576
// Ideally this could be exposed via browserify's `standalone`, but it doesn't
@@ -85,6 +96,7 @@ if (typeof self !== 'undefined') {
8596
self.listenForStatus = listenForStatus;
8697
// @ts-expect-error
8798
self.registerLocaleData = registerLocaleData;
99+
// TODO: expose as lookupCanonicalLocale in LighthouseService.ts?
88100
// @ts-expect-error
89-
self.lookupLocale = lookupLocale;
101+
self.lookupLocale = lookupCanonicalLocale;
90102
}

lighthouse-core/lib/i18n/i18n.js

+13-3
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,22 @@ const UIStrings = {
120120
* - supported locales in Intl formatters
121121
*
122122
* If `locale` isn't provided or one could not be found, DEFAULT_LOCALE is returned.
123+
*
124+
* By default any of the locales Lighthouse has strings for can be returned, but this
125+
* can be overriden with `possibleLocales`, useful e.g. when Lighthouse is bundled and
126+
* only DEFAULT_LOCALE is available, but `possibleLocales` can be used to select a
127+
* locale available to be downloaded on demand.
123128
* @param {string|string[]=} locales
129+
* @param {Array<string>=} possibleLocales
124130
* @return {LH.Locale}
125131
*/
126-
function lookupLocale(locales) {
127-
// If Node was built with `--with-intl=none`, `Intl` won't exist.
132+
function lookupLocale(locales, possibleLocales) {
133+
// TODO: lookupLocale may need to be split into two functions, one that canonicalizes
134+
// locales and one that looks up the best locale filename for a given locale.
135+
// e.g. `en-IE` is canonical, but uses `en-GB.json`. See TODO in locales.js
136+
128137
if (typeof Intl !== 'object') {
138+
// If Node was built with `--with-intl=none`, `Intl` won't exist.
129139
throw new Error('Lighthouse must be run in Node with `Intl` support. See https://nodejs.org/api/intl.html for help');
130140
}
131141

@@ -135,7 +145,7 @@ function lookupLocale(locales) {
135145
const availableLocales = Intl.NumberFormat.supportedLocalesOf(canonicalLocales);
136146

137147
// Get available locales and transform into object to match `lookupClosestLocale`'s API.
138-
const localesWithMessages = getAvailableLocales();
148+
const localesWithMessages = possibleLocales || getAvailableLocales();
139149
const localesWithmessagesObj = /** @type {Record<LH.Locale, LhlMessages>} */ (
140150
Object.fromEntries(localesWithMessages.map(l => [l, {}])));
141151

lighthouse-core/test/lib/i18n/i18n-test.js

+26
Original file line numberDiff line numberDiff line change
@@ -80,5 +80,31 @@ describe('i18n', () => {
8080
it('falls back to en-US if no match is available', () => {
8181
expect(i18n.lookupLocale(invalidLocale)).toEqual('en-US');
8282
});
83+
84+
describe('possibleLocales option', () => {
85+
it('canonicalizes from the possible locales', () => {
86+
expect(i18n.lookupLocale('en-xa', ['ar', 'en-XA'])).toEqual('en-XA');
87+
});
88+
89+
it('takes multiple locale strings and returns a possible, canonicalized one', () => {
90+
// COMPAT: Node 12 only has 'en-US' by default.
91+
if (isNode12SmallIcu()) {
92+
expect(i18n.lookupLocale([invalidLocale, 'eN-uS', 'en-xa'], ['ar', 'es', 'en-US']))
93+
.toEqual('en-US');
94+
return;
95+
}
96+
97+
expect(i18n.lookupLocale([invalidLocale, 'eS', 'en-xa'], ['ar', 'es']))
98+
.toEqual('es');
99+
});
100+
101+
it('falls back to en-US if no possible match is available', () => {
102+
expect(i18n.lookupLocale('es', ['en-US', 'ru', 'zh'])).toEqual('en-US');
103+
});
104+
105+
it('falls back to en-US if no possible matchs are available at all', () => {
106+
expect(i18n.lookupLocale('ru', [])).toEqual('en-US');
107+
});
108+
});
83109
});
84110
});

0 commit comments

Comments
 (0)