From a8ff2a08d1bc3b7da548d89b1488453319777573 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Fri, 19 Apr 2019 16:29:52 -0400 Subject: [PATCH 01/14] i18n: introduce script to swap in new locale to LHR --- .build-for-now.sh | 8 ++-- lighthouse-core/lib/i18n/i18n.js | 15 +++++++ lighthouse-core/runner.js | 1 + .../build-report-for-autodeployment.js | 22 +++++++--- .../scripts/i18n/swap-in-new-locale.js | 42 +++++++++++++++++++ package.json | 2 + types/lhr.d.ts | 3 +- yarn.lock | 12 ++++++ 8 files changed, 96 insertions(+), 9 deletions(-) create mode 100644 lighthouse-core/scripts/i18n/swap-in-new-locale.js diff --git a/.build-for-now.sh b/.build-for-now.sh index 07e7accd0791..54f40211073c 100644 --- a/.build-for-now.sh +++ b/.build-for-now.sh @@ -8,9 +8,11 @@ set -x -# Manually install this one report dependency. Avoid installing all deps (for speed) -rm package.json -yarn add details-element-polyfill@2.2.0 +# now doesn't support node 10 yet +cat package.json | sed 's|"node": .*|"node": "*"|' > package.json.fixed +mv package.json.fixed package.json + +yarn install # Create dist if it's not already there mkdir -p dist diff --git a/lighthouse-core/lib/i18n/i18n.js b/lighthouse-core/lib/i18n/i18n.js index e77bc4e50ef6..2a2139c314da 100644 --- a/lighthouse-core/lib/i18n/i18n.js +++ b/lighthouse-core/lib/i18n/i18n.js @@ -259,6 +259,20 @@ function getFormatted(icuMessageIdOrRawString, locale) { return icuMessageIdOrRawString; } +/** + * @param {LH.Locale} locale + * @param {string} icuMessageId + * @param {*} [values] + * @return {string} + */ +function formatMessageFromIdWithValues(locale, icuMessageId, values) { + const icuMessageIdRegex = /(.* \| .*)$/; + if (!icuMessageIdRegex.test(icuMessageId)) throw new Error('This is not an ICU message ID'); + + const {formattedString} = _formatIcuMessage(locale, icuMessageId, '', values); + return formattedString; +} + /** * @param {string} icuMessageInstanceId * @param {LH.Locale} locale @@ -331,6 +345,7 @@ module.exports = { getRendererFormattedStrings, createMessageInstanceIdFn, getFormatted, + formatMessageFromIdWithValues, replaceIcuMessageInstanceIds, isIcuMessage, }; diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index 1cbbf12ce1cd..b8744a4e472b 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -234,6 +234,7 @@ class Runner { gatherMode: undefined, auditMode: undefined, output: undefined, + channel: undefined, }; const normalizedGatherSettings = Object.assign({}, artifacts.settings, overrides); const normalizedAuditSettings = Object.assign({}, settings, overrides); diff --git a/lighthouse-core/scripts/build-report-for-autodeployment.js b/lighthouse-core/scripts/build-report-for-autodeployment.js index 65c2d7b05eaa..04f9862067b3 100644 --- a/lighthouse-core/scripts/build-report-for-autodeployment.js +++ b/lighthouse-core/scripts/build-report-for-autodeployment.js @@ -10,12 +10,24 @@ /* eslint-disable no-console */ const fs = require('fs'); const path = require('path'); +const replaceLHRLocale = require('./i18n/swap-in-new-locale.js'); const ReportGenerator = require('../../lighthouse-core/report/report-generator.js'); const lhr = /** @type {LH.Result} */ (require('../../lighthouse-core/test/results/sample_v2.json')); -console.log('đŸ•’ Generating report for sample_v2.json...'); -const html = ReportGenerator.generateReport(lhr, 'html'); -const filename = path.join(__dirname, '../../dist/index.html'); -fs.writeFileSync(filename, html, {encoding: 'utf-8'}); -console.log('✅', filename, 'written.'); +(async function() { + const filenameToLhr = { + 'index.html': lhr, + 'index.ar.html': replaceLHRLocale(JSON.parse(JSON.stringify(lhr)), 'ar'), + // Now serves the first alphabetical file as the root + 'index.aaaaaaaaaaaaaaaaaaaaa.html': lhr, + }; + + // Generate and write reports + Object.entries(filenameToLhr).forEach(([filename, lhr]) => { + const html = ReportGenerator.generateReport(lhr, 'html'); + const filepath = path.join(__dirname, `../../dist/${filename}`); + fs.writeFileSync(filepath, html, {encoding: 'utf-8'}); + console.log('✅', filepath, 'written.'); + }); +})(); diff --git a/lighthouse-core/scripts/i18n/swap-in-new-locale.js b/lighthouse-core/scripts/i18n/swap-in-new-locale.js new file mode 100644 index 000000000000..44bb58db94f3 --- /dev/null +++ b/lighthouse-core/scripts/i18n/swap-in-new-locale.js @@ -0,0 +1,42 @@ +#!/usr/bin/env node +/** + * @license Copyright 2019 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +/* eslint-disable no-console, max-len */ + +const fs = require('fs'); +const path = require('path'); +const _set = require('lodash.set'); + +const i18n = require('../../lib/i18n/i18n.js'); + +/** + * Replaces all strings within an LHR with ones from a different locale + * @param {LH.Result} lhr + * @param {LH.Locale} locale + */ +function replaceLHRLocale(lhr, locale) { + const resolvedLocale = i18n.lookupLocale(locale); + const {icuMessagePaths} = lhr.i18n; + + Object.entries(icuMessagePaths).forEach(([icuMessageId, messageInstancesInLHR]) => { + for (const instance of messageInstancesInLHR) { + const path = /** @type {LH.I18NMessageValuesEntry} */ (instance).path || /** @type {string} */ (instance); + const values = /** @type {LH.I18NMessageValuesEntry} */ (instance).values || undefined; + // Get new formatted strings in revised locale + const formattedStr = i18n.formatMessageFromIdWithValues(locale, icuMessageId, values); + // Write strings back into the LHR + _set(lhr, path, formattedStr); + } + }); + + // Tweak the config locale + lhr.configSettings.locale = resolvedLocale; + return lhr; +} + +module.exports = replaceLHRLocale; diff --git a/package.json b/package.json index f97637f73a37..bd6ec4729acb 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "@types/jest": "^24.0.9", "@types/jpeg-js": "^0.3.0", "@types/lodash.isequal": "^4.5.2", + "@types/lodash.set": "^4.3.6", "@types/make-dir": "^1.0.3", "@types/node": "*", "@types/opn": "^3.0.28", @@ -117,6 +118,7 @@ "isomorphic-fetch": "^2.2.1", "jest": "^24.3.0", "jsdom": "^12.2.0", + "lodash.set": "^4.3.2", "make-dir": "^1.3.0", "npm-run-posix-or-windows": "^2.0.2", "nyc": "^13.3.0", diff --git a/types/lhr.d.ts b/types/lhr.d.ts index f78312b5c292..2c08a3dbf795 100644 --- a/types/lhr.d.ts +++ b/types/lhr.d.ts @@ -8,7 +8,8 @@ import LHError = require('../lighthouse-core/lib/lh-error.js'); declare global { module LH { - export type I18NMessageEntry = string | {path: string, values: any}; + export type I18NMessageValuesEntry = {path: string, values: any}; + export type I18NMessageEntry = string | I18NMessageValuesEntry; export interface I18NMessages { [icuMessageId: string]: I18NMessageEntry[]; diff --git a/yarn.lock b/yarn.lock index 3bd581ac4752..b8cb6a9f8b6a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -533,6 +533,13 @@ dependencies: "@types/lodash" "*" +"@types/lodash.set@^4.3.6": + version "4.3.6" + resolved "https://registry.yarnpkg.com/@types/lodash.set/-/lodash.set-4.3.6.tgz#33e635c2323f855359225df6a5c8c6f1f1908264" + integrity sha512-ZeGDDlnRYTvS31Laij0RsSaguIUSBTYIlJFKL3vm3T2OAZAQj2YpSvVWJc0WiG4jqg9fGX6PAPGvDqBcHfSgFg== + dependencies: + "@types/lodash" "*" + "@types/lodash@*": version "4.14.106" resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.106.tgz#6093e9a02aa567ddecfe9afadca89e53e5dce4dd" @@ -5587,6 +5594,11 @@ lodash.memoize@~3.0.3: resolved "https://registry.yarnpkg.com/lodash.memoize/-/lodash.memoize-3.0.4.tgz#2dcbd2c287cbc0a55cc42328bd0c736150d53e3f" integrity sha1-LcvSwofLwKVcxCMovQxzYVDVPj8= +lodash.set@^4.3.2: + version "4.3.2" + resolved "https://registry.yarnpkg.com/lodash.set/-/lodash.set-4.3.2.tgz#d8757b1da807dde24816b0d6a84bea1a76230b23" + integrity sha1-2HV7HagH3eJIFrDWqEvqGnYjCyM= + lodash.sortby@^4.7.0: version "4.7.0" resolved "https://registry.yarnpkg.com/lodash.sortby/-/lodash.sortby-4.7.0.tgz#edd14c824e2cc9c1e0b0a1b42bb5210516a42438" From 81b0dc48c215c627ca72be1313fe523046b3c25b Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Mon, 22 Apr 2019 13:42:59 -0700 Subject: [PATCH 02/14] rename --- .../i18n/swap-locale.js} | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) rename lighthouse-core/{scripts/i18n/swap-in-new-locale.js => lib/i18n/swap-locale.js} (79%) diff --git a/lighthouse-core/scripts/i18n/swap-in-new-locale.js b/lighthouse-core/lib/i18n/swap-locale.js similarity index 79% rename from lighthouse-core/scripts/i18n/swap-in-new-locale.js rename to lighthouse-core/lib/i18n/swap-locale.js index 44bb58db94f3..c86532d44b99 100644 --- a/lighthouse-core/scripts/i18n/swap-in-new-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -1,4 +1,3 @@ -#!/usr/bin/env node /** * @license Copyright 2019 Google Inc. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 @@ -8,35 +7,34 @@ /* eslint-disable no-console, max-len */ -const fs = require('fs'); -const path = require('path'); const _set = require('lodash.set'); -const i18n = require('../../lib/i18n/i18n.js'); +const i18n = require('./i18n.js'); /** * Replaces all strings within an LHR with ones from a different locale * @param {LH.Result} lhr - * @param {LH.Locale} locale + * @param {LH.Locale} requestedLocale */ -function replaceLHRLocale(lhr, locale) { - const resolvedLocale = i18n.lookupLocale(locale); +function swapLocale(lhr, requestedLocale) { + const locale = i18n.lookupLocale(requestedLocale); const {icuMessagePaths} = lhr.i18n; Object.entries(icuMessagePaths).forEach(([icuMessageId, messageInstancesInLHR]) => { for (const instance of messageInstancesInLHR) { + // The path that _formatPathAsString() generated const path = /** @type {LH.I18NMessageValuesEntry} */ (instance).path || /** @type {string} */ (instance); const values = /** @type {LH.I18NMessageValuesEntry} */ (instance).values || undefined; // Get new formatted strings in revised locale const formattedStr = i18n.formatMessageFromIdWithValues(locale, icuMessageId, values); - // Write strings back into the LHR + // Write string back into the LHR _set(lhr, path, formattedStr); } }); // Tweak the config locale - lhr.configSettings.locale = resolvedLocale; + lhr.configSettings.locale = locale; return lhr; } -module.exports = replaceLHRLocale; +module.exports = swapLocale; From e12f7144ab31fe9b31ba2a7cfc27d22f2af165d8 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Wed, 24 Apr 2019 13:07:00 -0700 Subject: [PATCH 03/14] add start of a test. --- lighthouse-core/lib/i18n/swap-locale.js | 4 +++ .../test/lib/i18n/swap-locale-test.js | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 lighthouse-core/test/lib/i18n/swap-locale-test.js diff --git a/lighthouse-core/lib/i18n/swap-locale.js b/lighthouse-core/lib/i18n/swap-locale.js index c86532d44b99..cdaaf076a61d 100644 --- a/lighthouse-core/lib/i18n/swap-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -15,8 +15,12 @@ const i18n = require('./i18n.js'); * Replaces all strings within an LHR with ones from a different locale * @param {LH.Result} lhr * @param {LH.Locale} requestedLocale + * @return {LH.Result} */ function swapLocale(lhr, requestedLocale) { + // copy LHR to avoid mutating provided LHR + lhr = JSON.parse(JSON.stringify(lhr)); + const locale = i18n.lookupLocale(requestedLocale); const {icuMessagePaths} = lhr.i18n; diff --git a/lighthouse-core/test/lib/i18n/swap-locale-test.js b/lighthouse-core/test/lib/i18n/swap-locale-test.js new file mode 100644 index 000000000000..d7ac923aa849 --- /dev/null +++ b/lighthouse-core/test/lib/i18n/swap-locale-test.js @@ -0,0 +1,28 @@ +/** + * @license Copyright 2018 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +const swapLocale = require('../../../lib/i18n/swap-locale.js'); + +const lhr = require('../../results/sample_v2.json'); + +/* eslint-env jest */ + +describe('swap-locale', () => { + it('can change golden LHR english strings into spanish', () => { + const lhrEn = /** @type {LH.Result} */ (JSON.parse(JSON.stringify(lhr))); + + expect(lhrEn.audits.plugins.title).toEqual('Document avoids plugins'); + expect(lhrEn.audits['dom-size'].displayValue).toEqual('31 elements'); + + const lhrEs = swapLocale(lhrEn, 'es'); + + // Basic replacement + expect(lhrEs.audits.plugins.title).toEqual('El documento no usa complementos'); + // With ICU string argument values + expect(lhrEs.audits['dom-size'].displayValue).toEqual('31 nodos'); + }); +}); From 30bb4753a27a0c596e24b696c9538fe17f758c27 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 30 Apr 2019 16:02:07 -0700 Subject: [PATCH 04/14] revert now build changes. --- .build-for-now.sh | 8 +++---- lighthouse-core/runner.js | 1 - .../build-report-for-autodeployment.js | 22 +++++-------------- 3 files changed, 8 insertions(+), 23 deletions(-) diff --git a/.build-for-now.sh b/.build-for-now.sh index 54f40211073c..07e7accd0791 100644 --- a/.build-for-now.sh +++ b/.build-for-now.sh @@ -8,11 +8,9 @@ set -x -# now doesn't support node 10 yet -cat package.json | sed 's|"node": .*|"node": "*"|' > package.json.fixed -mv package.json.fixed package.json - -yarn install +# Manually install this one report dependency. Avoid installing all deps (for speed) +rm package.json +yarn add details-element-polyfill@2.2.0 # Create dist if it's not already there mkdir -p dist diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index b8744a4e472b..1cbbf12ce1cd 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -234,7 +234,6 @@ class Runner { gatherMode: undefined, auditMode: undefined, output: undefined, - channel: undefined, }; const normalizedGatherSettings = Object.assign({}, artifacts.settings, overrides); const normalizedAuditSettings = Object.assign({}, settings, overrides); diff --git a/lighthouse-core/scripts/build-report-for-autodeployment.js b/lighthouse-core/scripts/build-report-for-autodeployment.js index 04f9862067b3..65c2d7b05eaa 100644 --- a/lighthouse-core/scripts/build-report-for-autodeployment.js +++ b/lighthouse-core/scripts/build-report-for-autodeployment.js @@ -10,24 +10,12 @@ /* eslint-disable no-console */ const fs = require('fs'); const path = require('path'); -const replaceLHRLocale = require('./i18n/swap-in-new-locale.js'); const ReportGenerator = require('../../lighthouse-core/report/report-generator.js'); const lhr = /** @type {LH.Result} */ (require('../../lighthouse-core/test/results/sample_v2.json')); -(async function() { - const filenameToLhr = { - 'index.html': lhr, - 'index.ar.html': replaceLHRLocale(JSON.parse(JSON.stringify(lhr)), 'ar'), - // Now serves the first alphabetical file as the root - 'index.aaaaaaaaaaaaaaaaaaaaa.html': lhr, - }; - - // Generate and write reports - Object.entries(filenameToLhr).forEach(([filename, lhr]) => { - const html = ReportGenerator.generateReport(lhr, 'html'); - const filepath = path.join(__dirname, `../../dist/${filename}`); - fs.writeFileSync(filepath, html, {encoding: 'utf-8'}); - console.log('✅', filepath, 'written.'); - }); -})(); +console.log('đŸ•’ Generating report for sample_v2.json...'); +const html = ReportGenerator.generateReport(lhr, 'html'); +const filename = path.join(__dirname, '../../dist/index.html'); +fs.writeFileSync(filename, html, {encoding: 'utf-8'}); +console.log('✅', filename, 'written.'); From 059bc0fb460583b4904da6665dbfe266bbe3f2b5 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Thu, 2 May 2019 12:47:55 -0700 Subject: [PATCH 05/14] feedback --- lighthouse-core/lib/i18n/i18n.js | 7 +-- lighthouse-core/lib/i18n/swap-locale.js | 52 ++++++++++++++++--- .../test/lib/i18n/swap-locale-test.js | 13 +++-- 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/lighthouse-core/lib/i18n/i18n.js b/lighthouse-core/lib/i18n/i18n.js index 2a2139c314da..d3f1328546a9 100644 --- a/lighthouse-core/lib/i18n/i18n.js +++ b/lighthouse-core/lib/i18n/i18n.js @@ -150,7 +150,7 @@ const _icuMessageInstanceMap = new Map(); * * @param {LH.Locale} locale * @param {string} icuMessageId - * @param {string} icuMessage + * @param {string=} icuMessage * @param {*} [values] * @return {{formattedString: string, icuMessage: string}} */ @@ -160,10 +160,11 @@ function _formatIcuMessage(locale, icuMessageId, icuMessage, values) { // fallback to the original english message if we couldn't find a message in the specified locale // better to have an english message than no message at all, in some number cases it won't even matter const messageForMessageFormat = localeMessage || icuMessage; + if (messageForMessageFormat === undefined) throw new Error('No ICU message string to format'); // when using accented english, force the use of a different locale for number formatting const localeForMessageFormat = locale === 'en-XA' ? 'de-DE' : locale; // pre-process values for the message format like KB and milliseconds - const valuesForMessageFormat = _preprocessMessageValues(icuMessage, values); + const valuesForMessageFormat = _preprocessMessageValues(messageForMessageFormat, values); const formatter = new MessageFormat(messageForMessageFormat, localeForMessageFormat, formats); const formattedString = formatter.format(valuesForMessageFormat); @@ -269,7 +270,7 @@ function formatMessageFromIdWithValues(locale, icuMessageId, values) { const icuMessageIdRegex = /(.* \| .*)$/; if (!icuMessageIdRegex.test(icuMessageId)) throw new Error('This is not an ICU message ID'); - const {formattedString} = _formatIcuMessage(locale, icuMessageId, '', values); + const {formattedString} = _formatIcuMessage(locale, icuMessageId, undefined, values); return formattedString; } diff --git a/lighthouse-core/lib/i18n/swap-locale.js b/lighthouse-core/lib/i18n/swap-locale.js index cdaaf076a61d..c20e5ba65987 100644 --- a/lighthouse-core/lib/i18n/swap-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -11,6 +11,35 @@ const _set = require('lodash.set'); const i18n = require('./i18n.js'); +/** + * @fileoverview Use the lhr.i18n.icuMessagePaths object to change locales + * + * `icuMessagePaths` is an object keyed by `icuMessageId`s. Within each is either + * 1) an array of strings, which are just object paths to where that message is used in the LHR + * 2) an array of `LH.I18NMessageValuesEntry`s which include both a `path` and a `values` object + * which will be used in the replacement within `i18n._formatIcuMessage()` + * + * An example: + * "icuMessagePaths": { + "lighthouse-core/audits/metrics/first-contentful-paint.js | title": [ + "audits[first-contentful-paint].title" + ], + "lighthouse-core/audits/time-to-first-byte.js | displayValue": [ + { + "values": { + "timeInMs": 570.5630000000001 + }, + "path": "audits[time-to-first-byte].displayValue" + } + ], + "lighthouse-core/lib/i18n/i18n.js | columnTimeSpent": [ + "audits[mainthread-work-breakdown].details.headings[1].text", + "audits[network-rtt].details.headings[1].text", + "audits[network-server-latency].details.headings[1].text" + ], + ... + */ + /** * Replaces all strings within an LHR with ones from a different locale * @param {LH.Result} lhr @@ -27,15 +56,26 @@ function swapLocale(lhr, requestedLocale) { Object.entries(icuMessagePaths).forEach(([icuMessageId, messageInstancesInLHR]) => { for (const instance of messageInstancesInLHR) { // The path that _formatPathAsString() generated - const path = /** @type {LH.I18NMessageValuesEntry} */ (instance).path || /** @type {string} */ (instance); - const values = /** @type {LH.I18NMessageValuesEntry} */ (instance).values || undefined; - // Get new formatted strings in revised locale - const formattedStr = i18n.formatMessageFromIdWithValues(locale, icuMessageId, values); - // Write string back into the LHR - _set(lhr, path, formattedStr); + let path; + let values; + if (typeof instance === 'string') { + path = instance; + } else { + path = /** @type {LH.I18NMessageValuesEntry} */ (instance).path; + // `values` are the string template values to be used. eg. `values: {wastedBytes: 9028}` + values = /** @type {LH.I18NMessageValuesEntry} */ (instance).values; + } + // If we couldn't find the new replacement message, keep things as is. + try { + // Get new formatted strings in revised locale + const formattedStr = i18n.formatMessageFromIdWithValues(locale, icuMessageId, values); + // Write string back into the LHR + _set(lhr, path, formattedStr); + } catch (e) {} } }); + lhr.i18n.rendererFormattedStrings = i18n.getRendererFormattedStrings(locale); // Tweak the config locale lhr.configSettings.locale = locale; return lhr; diff --git a/lighthouse-core/test/lib/i18n/swap-locale-test.js b/lighthouse-core/test/lib/i18n/swap-locale-test.js index d7ac923aa849..31efdb6c7cf3 100644 --- a/lighthouse-core/test/lib/i18n/swap-locale-test.js +++ b/lighthouse-core/test/lib/i18n/swap-locale-test.js @@ -1,5 +1,5 @@ /** - * @license Copyright 2018 Google Inc. All Rights Reserved. + * @license Copyright 2019 Google Inc. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ @@ -15,14 +15,19 @@ describe('swap-locale', () => { it('can change golden LHR english strings into spanish', () => { const lhrEn = /** @type {LH.Result} */ (JSON.parse(JSON.stringify(lhr))); - expect(lhrEn.audits.plugins.title).toEqual('Document avoids plugins'); - expect(lhrEn.audits['dom-size'].displayValue).toEqual('31 elements'); - const lhrEs = swapLocale(lhrEn, 'es'); // Basic replacement + expect(lhrEn.audits.plugins.title).toEqual('Document avoids plugins'); expect(lhrEs.audits.plugins.title).toEqual('El documento no usa complementos'); + // With ICU string argument values + expect(lhrEn.audits['dom-size'].displayValue).toEqual('31 elements'); expect(lhrEs.audits['dom-size'].displayValue).toEqual('31 nodos'); + + /* eslint-disable max-len */ + // Renderer formatted strings + expect(lhrEn.i18n.rendererFormattedStrings.notApplicableAuditsGroupTitle).toEqual('Not applicable'); + expect(lhrEs.i18n.rendererFormattedStrings.notApplicableAuditsGroupTitle).toEqual('No aplicable'); }); }); From 54c48337c3e80bb503ec419e0b5dd257c1a6b2db Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Thu, 2 May 2019 12:50:05 -0700 Subject: [PATCH 06/14] rename to fallbackMessage --- lighthouse-core/lib/i18n/i18n.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/lib/i18n/i18n.js b/lighthouse-core/lib/i18n/i18n.js index d3f1328546a9..a9473b311e41 100644 --- a/lighthouse-core/lib/i18n/i18n.js +++ b/lighthouse-core/lib/i18n/i18n.js @@ -150,16 +150,16 @@ const _icuMessageInstanceMap = new Map(); * * @param {LH.Locale} locale * @param {string} icuMessageId - * @param {string=} icuMessage + * @param {string=} fallbackMessage * @param {*} [values] * @return {{formattedString: string, icuMessage: string}} */ -function _formatIcuMessage(locale, icuMessageId, icuMessage, values) { +function _formatIcuMessage(locale, icuMessageId, fallbackMessage, values) { const localeMessages = LOCALES[locale]; const localeMessage = localeMessages[icuMessageId] && localeMessages[icuMessageId].message; // fallback to the original english message if we couldn't find a message in the specified locale // better to have an english message than no message at all, in some number cases it won't even matter - const messageForMessageFormat = localeMessage || icuMessage; + const messageForMessageFormat = localeMessage || fallbackMessage; if (messageForMessageFormat === undefined) throw new Error('No ICU message string to format'); // when using accented english, force the use of a different locale for number formatting const localeForMessageFormat = locale === 'en-XA' ? 'de-DE' : locale; From bdd29b406ed68243e82ad5c0d16f0ceb4f7d225b Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Fri, 3 May 2019 13:41:59 -0700 Subject: [PATCH 07/14] fix espanol string --- lighthouse-core/test/lib/i18n/swap-locale-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/lib/i18n/swap-locale-test.js b/lighthouse-core/test/lib/i18n/swap-locale-test.js index 31efdb6c7cf3..acbd73f8431a 100644 --- a/lighthouse-core/test/lib/i18n/swap-locale-test.js +++ b/lighthouse-core/test/lib/i18n/swap-locale-test.js @@ -23,7 +23,7 @@ describe('swap-locale', () => { // With ICU string argument values expect(lhrEn.audits['dom-size'].displayValue).toEqual('31 elements'); - expect(lhrEs.audits['dom-size'].displayValue).toEqual('31 nodos'); + expect(lhrEs.audits['dom-size'].displayValue).toEqual('31 elementos'); /* eslint-disable max-len */ // Renderer formatted strings From 5255aee40da3713515344a2795c7f3a4b3ac132e Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 28 May 2019 14:06:47 -0700 Subject: [PATCH 08/14] Update lighthouse-core/lib/i18n/swap-locale.js Co-Authored-By: Brendan Kenny --- lighthouse-core/lib/i18n/swap-locale.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/lib/i18n/swap-locale.js b/lighthouse-core/lib/i18n/swap-locale.js index c20e5ba65987..ee7b12952066 100644 --- a/lighthouse-core/lib/i18n/swap-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -41,7 +41,7 @@ const i18n = require('./i18n.js'); */ /** - * Replaces all strings within an LHR with ones from a different locale + * Returns a new LHR with all strings changed to the new `requestedLocale`. * @param {LH.Result} lhr * @param {LH.Locale} requestedLocale * @return {LH.Result} From 627f7be12c4ecd2c074ba4ea9ebecabfc1186226 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 28 May 2019 14:06:56 -0700 Subject: [PATCH 09/14] Update lighthouse-core/lib/i18n/swap-locale.js Co-Authored-By: Brendan Kenny --- lighthouse-core/lib/i18n/swap-locale.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/lib/i18n/swap-locale.js b/lighthouse-core/lib/i18n/swap-locale.js index ee7b12952066..1dec2966c0e2 100644 --- a/lighthouse-core/lib/i18n/swap-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -47,7 +47,7 @@ const i18n = require('./i18n.js'); * @return {LH.Result} */ function swapLocale(lhr, requestedLocale) { - // copy LHR to avoid mutating provided LHR + // Copy LHR to avoid mutating provided LHR. lhr = JSON.parse(JSON.stringify(lhr)); const locale = i18n.lookupLocale(requestedLocale); From 3531318c5c6900301d296a21ca5fd3e6d10efb27 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 28 May 2019 14:26:38 -0700 Subject: [PATCH 10/14] feedback --- lighthouse-core/lib/i18n/i18n.js | 4 +- lighthouse-core/lib/i18n/swap-locale.js | 46 +++++++++++-------- .../test/lib/i18n/swap-locale-test.js | 18 ++++++-- types/lhr.d.ts | 2 +- 4 files changed, 43 insertions(+), 27 deletions(-) diff --git a/lighthouse-core/lib/i18n/i18n.js b/lighthouse-core/lib/i18n/i18n.js index 3e4cb9174112..628d492a996a 100644 --- a/lighthouse-core/lib/i18n/i18n.js +++ b/lighthouse-core/lib/i18n/i18n.js @@ -284,7 +284,7 @@ function getFormatted(icuMessageIdOrRawString, locale) { * @param {*} [values] * @return {string} */ -function formatMessageFromIdWithValues(locale, icuMessageId, values) { +function getFormattedFromIdAndValues(locale, icuMessageId, values) { const icuMessageIdRegex = /(.* \| .*)$/; if (!icuMessageIdRegex.test(icuMessageId)) throw new Error('This is not an ICU message ID'); @@ -364,7 +364,7 @@ module.exports = { getRendererFormattedStrings, createMessageInstanceIdFn, getFormatted, - formatMessageFromIdWithValues, + getFormattedFromIdAndValues, replaceIcuMessageInstanceIds, isIcuMessage, }; diff --git a/lighthouse-core/lib/i18n/swap-locale.js b/lighthouse-core/lib/i18n/swap-locale.js index 1dec2966c0e2..de028b864d9b 100644 --- a/lighthouse-core/lib/i18n/swap-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -20,24 +20,24 @@ const i18n = require('./i18n.js'); * which will be used in the replacement within `i18n._formatIcuMessage()` * * An example: - * "icuMessagePaths": { - "lighthouse-core/audits/metrics/first-contentful-paint.js | title": [ - "audits[first-contentful-paint].title" - ], - "lighthouse-core/audits/time-to-first-byte.js | displayValue": [ - { - "values": { - "timeInMs": 570.5630000000001 - }, - "path": "audits[time-to-first-byte].displayValue" - } - ], - "lighthouse-core/lib/i18n/i18n.js | columnTimeSpent": [ - "audits[mainthread-work-breakdown].details.headings[1].text", - "audits[network-rtt].details.headings[1].text", - "audits[network-server-latency].details.headings[1].text" - ], - ... + "icuMessagePaths": { + "lighthouse-core/audits/metrics/first-contentful-paint.js | title": [ + "audits[first-contentful-paint].title" + ], + "lighthouse-core/audits/time-to-first-byte.js | displayValue": [ + { + "values": { + "timeInMs": 570.5630000000001 + }, + "path": "audits[time-to-first-byte].displayValue" + } + ], + "lighthouse-core/lib/i18n/i18n.js | columnTimeSpent": [ + "audits[mainthread-work-breakdown].details.headings[1].text", + "audits[network-rtt].details.headings[1].text", + "audits[network-server-latency].details.headings[1].text" + ], + ... */ /** @@ -68,10 +68,16 @@ function swapLocale(lhr, requestedLocale) { // If we couldn't find the new replacement message, keep things as is. try { // Get new formatted strings in revised locale - const formattedStr = i18n.formatMessageFromIdWithValues(locale, icuMessageId, values); + const formattedStr = i18n.getFormattedFromIdAndValues(locale, icuMessageId, values); // Write string back into the LHR _set(lhr, path, formattedStr); - } catch (e) {} + } catch (err) { + if (err.message === 'No ICU message string to format') { + console.error('No message found for ', {locale, icuMessageId}); + } else { + throw err; + } + } } }); diff --git a/lighthouse-core/test/lib/i18n/swap-locale-test.js b/lighthouse-core/test/lib/i18n/swap-locale-test.js index acbd73f8431a..e63540daea75 100644 --- a/lighthouse-core/test/lib/i18n/swap-locale-test.js +++ b/lighthouse-core/test/lib/i18n/swap-locale-test.js @@ -14,7 +14,6 @@ const lhr = require('../../results/sample_v2.json'); describe('swap-locale', () => { it('can change golden LHR english strings into spanish', () => { const lhrEn = /** @type {LH.Result} */ (JSON.parse(JSON.stringify(lhr))); - const lhrEs = swapLocale(lhrEn, 'es'); // Basic replacement @@ -25,9 +24,20 @@ describe('swap-locale', () => { expect(lhrEn.audits['dom-size'].displayValue).toEqual('31 elements'); expect(lhrEs.audits['dom-size'].displayValue).toEqual('31 elementos'); - /* eslint-disable max-len */ // Renderer formatted strings - expect(lhrEn.i18n.rendererFormattedStrings.notApplicableAuditsGroupTitle).toEqual('Not applicable'); - expect(lhrEs.i18n.rendererFormattedStrings.notApplicableAuditsGroupTitle).toEqual('No aplicable'); + expect(lhrEn.i18n.rendererFormattedStrings.labDataTitle).toEqual('Lab Data'); + expect(lhrEs.i18n.rendererFormattedStrings.labDataTitle).toEqual('Datos de prueba'); + }); + + it('can roundtrip back to english correctly', () => { + const lhrEn = /** @type {LH.Result} */ (JSON.parse(JSON.stringify(lhr))); + + // via Spanish + const lhrEnEsRT = swapLocale(swapLocale(lhrEn, 'es'), 'en-US'); + expect(lhrEnEsRT).toMatchObject(lhrEn); + + // via Arabic + const lhrEnArRT = swapLocale(swapLocale(lhrEn, 'ar'), 'en-US'); + expect(lhrEnArRT).toMatchObject(lhrEn); }); }); diff --git a/types/lhr.d.ts b/types/lhr.d.ts index 3d5230a1c4d4..79d6e8aa60e6 100644 --- a/types/lhr.d.ts +++ b/types/lhr.d.ts @@ -8,7 +8,7 @@ import LHError = require('../lighthouse-core/lib/lh-error.js'); declare global { module LH { - export type I18NMessageValuesEntry = {path: string, values: any}; + export type I18NMessageValuesEntry = {path: string, values: Record}; export type I18NMessageEntry = string | I18NMessageValuesEntry; export interface I18NMessages { From 5654af17936a29dffebab65323ee21d0f869f251 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 28 May 2019 15:31:10 -0700 Subject: [PATCH 11/14] console time --- lighthouse-core/lib/i18n/swap-locale.js | 7 ++++++- lighthouse-core/test/lib/i18n/swap-locale-test.js | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/lib/i18n/swap-locale.js b/lighthouse-core/lib/i18n/swap-locale.js index de028b864d9b..852f227ba8b1 100644 --- a/lighthouse-core/lib/i18n/swap-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -52,6 +52,7 @@ function swapLocale(lhr, requestedLocale) { const locale = i18n.lookupLocale(requestedLocale); const {icuMessagePaths} = lhr.i18n; + const missingIcuMessageIds = /** @type {string[]} */([]); Object.entries(icuMessagePaths).forEach(([icuMessageId, messageInstancesInLHR]) => { for (const instance of messageInstancesInLHR) { @@ -73,7 +74,7 @@ function swapLocale(lhr, requestedLocale) { _set(lhr, path, formattedStr); } catch (err) { if (err.message === 'No ICU message string to format') { - console.error('No message found for ', {locale, icuMessageId}); + missingIcuMessageIds.push(icuMessageId); } else { throw err; } @@ -81,6 +82,10 @@ function swapLocale(lhr, requestedLocale) { } }); + if (missingIcuMessageIds.length) { + console.error(`No message in locale (${locale}) found for:\n`, missingIcuMessageIds); + } + lhr.i18n.rendererFormattedStrings = i18n.getRendererFormattedStrings(locale); // Tweak the config locale lhr.configSettings.locale = locale; diff --git a/lighthouse-core/test/lib/i18n/swap-locale-test.js b/lighthouse-core/test/lib/i18n/swap-locale-test.js index e63540daea75..1db1351b19e8 100644 --- a/lighthouse-core/test/lib/i18n/swap-locale-test.js +++ b/lighthouse-core/test/lib/i18n/swap-locale-test.js @@ -10,6 +10,11 @@ const swapLocale = require('../../../lib/i18n/swap-locale.js'); const lhr = require('../../results/sample_v2.json'); /* eslint-env jest */ +beforeEach(() => { + // silence console.error spam about messages not found + // eslint-disable-next-line no-console + console.error = jest.fn(); +}); describe('swap-locale', () => { it('can change golden LHR english strings into spanish', () => { From 853bfd2d99aa53440a2addb3c076f5545c4a142f Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 28 May 2019 15:39:51 -0700 Subject: [PATCH 12/14] drop casts --- lighthouse-core/lib/i18n/swap-locale.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lighthouse-core/lib/i18n/swap-locale.js b/lighthouse-core/lib/i18n/swap-locale.js index 852f227ba8b1..f89f0cd78420 100644 --- a/lighthouse-core/lib/i18n/swap-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -62,9 +62,9 @@ function swapLocale(lhr, requestedLocale) { if (typeof instance === 'string') { path = instance; } else { - path = /** @type {LH.I18NMessageValuesEntry} */ (instance).path; + path = instance.path; // `values` are the string template values to be used. eg. `values: {wastedBytes: 9028}` - values = /** @type {LH.I18NMessageValuesEntry} */ (instance).values; + values = instance.values; } // If we couldn't find the new replacement message, keep things as is. try { From e3f2d3feb2a8e9d639bf526c990eb6d482c2aac1 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Wed, 5 Jun 2019 14:25:08 +0200 Subject: [PATCH 13/14] return warnings. feedback --- lighthouse-core/lib/i18n/i18n.js | 4 +- lighthouse-core/lib/i18n/swap-locale.js | 16 +++--- .../test/lib/i18n/swap-locale-test.js | 50 +++++++++++++++---- 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/lighthouse-core/lib/i18n/i18n.js b/lighthouse-core/lib/i18n/i18n.js index 628d492a996a..2d55457044f2 100644 --- a/lighthouse-core/lib/i18n/i18n.js +++ b/lighthouse-core/lib/i18n/i18n.js @@ -164,6 +164,7 @@ function _preprocessMessageValues(icuMessage, values) { /** @type {Map} */ const _icuMessageInstanceMap = new Map(); +const _ICUMsgNotFoundMsg = 'ICU message not found in destination locale'; /** * * @param {LH.Locale} locale @@ -178,7 +179,7 @@ function _formatIcuMessage(locale, icuMessageId, fallbackMessage, values) { // fallback to the original english message if we couldn't find a message in the specified locale // better to have an english message than no message at all, in some number cases it won't even matter const messageForMessageFormat = localeMessage || fallbackMessage; - if (messageForMessageFormat === undefined) throw new Error('No ICU message string to format'); + if (messageForMessageFormat === undefined) throw new Error(_ICUMsgNotFoundMsg); // when using accented english, force the use of a different locale for number formatting const localeForMessageFormat = locale === 'en-XA' ? 'de-DE' : locale; // pre-process values for the message format like KB and milliseconds @@ -359,6 +360,7 @@ function replaceIcuMessageInstanceIds(inputObject, locale) { module.exports = { _formatPathAsString, + _ICUMsgNotFoundMsg, UIStrings, lookupLocale, getRendererFormattedStrings, diff --git a/lighthouse-core/lib/i18n/swap-locale.js b/lighthouse-core/lib/i18n/swap-locale.js index f89f0cd78420..11b44bfb6215 100644 --- a/lighthouse-core/lib/i18n/swap-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -5,11 +5,10 @@ */ 'use strict'; -/* eslint-disable no-console, max-len */ - const _set = require('lodash.set'); const i18n = require('./i18n.js'); +const LHError = require('../lh-error.js'); /** * @fileoverview Use the lhr.i18n.icuMessagePaths object to change locales @@ -44,7 +43,7 @@ const i18n = require('./i18n.js'); * Returns a new LHR with all strings changed to the new `requestedLocale`. * @param {LH.Result} lhr * @param {LH.Locale} requestedLocale - * @return {LH.Result} + * @return {{lhr: LH.Result, missingIcuMessageIds: string[]}} */ function swapLocale(lhr, requestedLocale) { // Copy LHR to avoid mutating provided LHR. @@ -73,7 +72,7 @@ function swapLocale(lhr, requestedLocale) { // Write string back into the LHR _set(lhr, path, formattedStr); } catch (err) { - if (err.message === 'No ICU message string to format') { + if (err.message === i18n._ICUMsgNotFoundMsg) { missingIcuMessageIds.push(icuMessageId); } else { throw err; @@ -82,14 +81,13 @@ function swapLocale(lhr, requestedLocale) { } }); - if (missingIcuMessageIds.length) { - console.error(`No message in locale (${locale}) found for:\n`, missingIcuMessageIds); - } - lhr.i18n.rendererFormattedStrings = i18n.getRendererFormattedStrings(locale); // Tweak the config locale lhr.configSettings.locale = locale; - return lhr; + return { + lhr, + missingIcuMessageIds, + }; } module.exports = swapLocale; diff --git a/lighthouse-core/test/lib/i18n/swap-locale-test.js b/lighthouse-core/test/lib/i18n/swap-locale-test.js index 1db1351b19e8..9a73a7e8198a 100644 --- a/lighthouse-core/test/lib/i18n/swap-locale-test.js +++ b/lighthouse-core/test/lib/i18n/swap-locale-test.js @@ -10,16 +10,10 @@ const swapLocale = require('../../../lib/i18n/swap-locale.js'); const lhr = require('../../results/sample_v2.json'); /* eslint-env jest */ -beforeEach(() => { - // silence console.error spam about messages not found - // eslint-disable-next-line no-console - console.error = jest.fn(); -}); - describe('swap-locale', () => { it('can change golden LHR english strings into spanish', () => { const lhrEn = /** @type {LH.Result} */ (JSON.parse(JSON.stringify(lhr))); - const lhrEs = swapLocale(lhrEn, 'es'); + const lhrEs = swapLocale(lhrEn, 'es').lhr; // Basic replacement expect(lhrEn.audits.plugins.title).toEqual('Document avoids plugins'); @@ -38,11 +32,45 @@ describe('swap-locale', () => { const lhrEn = /** @type {LH.Result} */ (JSON.parse(JSON.stringify(lhr))); // via Spanish - const lhrEnEsRT = swapLocale(swapLocale(lhrEn, 'es'), 'en-US'); - expect(lhrEnEsRT).toMatchObject(lhrEn); + const lhrEnEsRT = swapLocale(swapLocale(lhrEn, 'es').lhr, 'en-US').lhr; + expect(lhrEnEsRT).toEqual(lhrEn); // via Arabic - const lhrEnArRT = swapLocale(swapLocale(lhrEn, 'ar'), 'en-US'); - expect(lhrEnArRT).toMatchObject(lhrEn); + const lhrEnArRT = swapLocale(swapLocale(lhrEn, 'ar').lhr, 'en-US').lhr; + expect(lhrEnArRT).toEqual(lhrEn); + }); + + it('leaves alone messages where there is no translation available', () => { + const miniLHR = { + audits: { + redirects: { + id: 'redirects', + title: 'Avoid multiple page redirects', + }, + fakeaudit: { + id: 'fakeaudit', + title: 'An audit without translations', + }, + }, + configSettings: { + locale: 'en-US', + }, + i18n: { + icuMessagePaths: { + 'lighthouse-core/audits/redirects.js | title': ['audits.redirects.title'], + 'lighthouse-core/audits/redirects.js | doesntExist': ['audits.redirects.doesntExist'], + 'lighthouse-core/audits/fakeaudit.js | title': ['audits.fakeaudit.title'], + }, + }, + }; + const {missingIcuMessageIds} = swapLocale(miniLHR, 'es'); + + // Updated strings are not found, so these remain in the original language + expect(missingIcuMessageIds).toMatchInlineSnapshot(` +Array [ + "lighthouse-core/audits/redirects.js | doesntExist", + "lighthouse-core/audits/fakeaudit.js | title", +] +`); }); }); From cf0d3b783669db2f51db12cb32e44ab14bcd5905 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 11 Jun 2019 18:04:22 -0700 Subject: [PATCH 14/14] remove lherror dep --- lighthouse-core/lib/i18n/swap-locale.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lighthouse-core/lib/i18n/swap-locale.js b/lighthouse-core/lib/i18n/swap-locale.js index 11b44bfb6215..0624f6fca339 100644 --- a/lighthouse-core/lib/i18n/swap-locale.js +++ b/lighthouse-core/lib/i18n/swap-locale.js @@ -8,7 +8,6 @@ const _set = require('lodash.set'); const i18n = require('./i18n.js'); -const LHError = require('../lh-error.js'); /** * @fileoverview Use the lhr.i18n.icuMessagePaths object to change locales