i18n: use better types for intl-messageformat (#9570)

build/build-bundle.js

@@ -62,6 +62,7 @@ async function browserifyFile(entryPath, distPath) {
   bundle.ignore('source-map')
     .ignore('debug/node')
     .ignore('intl')
+    .ignore('intl-pluralrules')
     .ignore('raven')
     .ignore('mkdirp')
     .ignore('rimraf')

lighthouse-core/lib/i18n/i18n.js

@@ -9,10 +9,12 @@ const path = require('path');
 const isDeepEqual = require('lodash.isequal');
 const log = require('lighthouse-logger');
 const MessageFormat = require('intl-messageformat').default;
-const MessageParser = require('intl-messageformat-parser');
 const lookupClosestLocale = require('lookup-closest-locale');
 const LOCALES = require('./locales.js');
 
+/** @typedef {import('intl-messageformat-parser').Element} MessageElement */
+/** @typedef {import('intl-messageformat-parser').ArgumentElement} ArgumentElement */
+
 const LH_ROOT = path.join(__dirname, '../../../');
 const MESSAGE_INSTANCE_ID_REGEX = /(.* \| .*) # (\d+)$/;
 // Above regex is very slow against large strings. Use QUICK_REGEX as a much quicker discriminator.
@@ -29,6 +31,10 @@ const MESSAGE_INSTANCE_ID_QUICK_REGEX = / # \d+$/;
 
     Intl.NumberFormat = IntlPolyfill.NumberFormat;
     Intl.DateTimeFormat = IntlPolyfill.DateTimeFormat;
+
+    // Intl.PluralRules polyfilled on require().
+    // @ts-ignore
+    require('intl-pluralrules');
   } catch (_) {
     log.warn('i18n', 'Failed to install `intl` polyfill');
   }
@@ -126,16 +132,18 @@ function lookupLocale(locale) {
 }
 
 /**
+ * Preformat values for the message based on how they're used, like KB or milliseconds.
  * @param {string} icuMessage
+ * @param {MessageFormat} messageFormatter
  * @param {Record<string, string | number>} [values]
+ * @return {Record<string, string | number>}
  */
-function _preprocessMessageValues(icuMessage, values = {}) {
+function _preformatValues(icuMessage, messageFormatter, values = {}) {
   const clonedValues = JSON.parse(JSON.stringify(values));
-  const parsed = MessageParser.parse(icuMessage);
 
-  const elements = _collectAllCustomElementsFromICU(parsed.elements);
+  const elements = _collectAllCustomElementsFromICU(messageFormatter.getAst().elements);
 
-  return _processParsedElements(Array.from(elements.values()), clonedValues);
+  return _processParsedElements(icuMessage, Array.from(elements.values()), clonedValues);
 }
 
 /**
@@ -152,23 +160,23 @@ function _preprocessMessageValues(icuMessage, values = {}) {
  *  be stored in a set because they are not equal since their locations are different,
  *  thus they are stored via a Map keyed on the "id" which is the ICU varName.
  *
- * @param {Array<import('intl-messageformat-parser').Element>} icuElements
- * @param {Map<string, import('intl-messageformat-parser').Element>} seenElementsById
+ * @param {Array<MessageElement>} icuElements
+ * @param {Map<string, ArgumentElement>} seenElementsById
+ * @return {Map<string, ArgumentElement>}
  */
 function _collectAllCustomElementsFromICU(icuElements, seenElementsById = new Map()) {
   for (const el of icuElements) {
     // We are only interested in elements that need ICU formatting (argumentElements)
     if (el.type !== 'argumentElement') continue;
-    // @ts-ignore - el.id is always defined when el.format is defined
+
     seenElementsById.set(el.id, el);
 
     // Plurals need to be inspected recursively
     if (!el.format || el.format.type !== 'pluralFormat') continue;
     // Look at all options of the plural (=1{} =other{}...)
     for (const option of el.format.options) {
       // Run collections on each option's elements
-      _collectAllCustomElementsFromICU(option.value.elements,
-        seenElementsById);
+      _collectAllCustomElementsFromICU(option.value.elements, seenElementsById);
     }
   }
 
@@ -180,36 +188,40 @@ function _collectAllCustomElementsFromICU(icuElements, seenElementsById = new Ma
  * will apply Lighthouse custom formatting to the values based on the argumentElement
  * format style.
  *
- * @param {Array<import('intl-messageformat-parser').Element>} argumentElements
+ * @param {string} icuMessage
+ * @param {Array<ArgumentElement>} argumentElements
  * @param {Record<string, string | number>} [values]
+ * @return {Record<string, string | number>}
  */
-function _processParsedElements(argumentElements, values = {}) {
-  // Throw an error if a message's value isn't provided
-  argumentElements
-    .filter(el => el.type === 'argumentElement')
-    .forEach(el => {
-      if (el.id && (el.id in values) === false) {
-        throw new Error(`ICU Message contains a value reference ("${el.id}") that wasn't provided`);
-      }
-    });
-
-  // Round all milliseconds to the nearest 10
-  argumentElements
-    .filter(el => el.format && el.format.style === 'milliseconds')
-    // @ts-ignore - el.id is always defined when el.format is defined
-    .forEach(el => (values[el.id] = Math.round(values[el.id] / 10) * 10));
-
-  // Convert all seconds to the correct unit
-  argumentElements
-    .filter(el => el.format && el.format.style === 'seconds' && el.id === 'timeInMs')
-    // @ts-ignore - el.id is always defined when el.format is defined
-    .forEach(el => (values[el.id] = Math.round(values[el.id] / 100) / 10));
-
-  // Replace all the bytes with KB
-  argumentElements
-    .filter(el => el.format && el.format.style === 'bytes')
-    // @ts-ignore - el.id is always defined when el.format is defined
-    .forEach(el => (values[el.id] = values[el.id] / 1024));
+function _processParsedElements(icuMessage, argumentElements, values = {}) {
+  for (const {id, format} of argumentElements) {
+    // Throw an error if a message's value isn't provided
+    if (id && (id in values) === false) {
+      throw new Error(`ICU Message "${icuMessage}" contains a value reference ("${id}") ` +
+        `that wasn't provided`);
+    }
+
+    // Direct `{id}` replacement and non-numeric values need no formatting.
+    if (!format || format.type !== 'numberFormat') continue;
+
+    const value = values[id];
+    if (typeof value !== 'number') {
+      throw new Error(`ICU Message "${icuMessage}" contains a numeric reference ("${id}") ` +
+        'but provided value was not a number');
+    }
+
+    // Format values for known styles.
+    if (format.style === 'milliseconds') {
+      // Round all milliseconds to the nearest 10.
+      values[id] = Math.round(value / 10) * 10;
+    } else if (format.style === 'seconds' && id === 'timeInMs') {
+      // Convert all seconds to the correct unit (currently only for `timeInMs`).
+      values[id] = Math.round(value / 100) / 10;
+    } else if (format.style === 'bytes') {
+      // Replace all the bytes with KB.
+      values[id] = value / 1024;
+    }
+  }
 
   return values;
 }
@@ -257,12 +269,13 @@ function _formatIcuMessage(locale, icuMessageId, uiStringMessage, values) {
 
   // when using accented english, force the use of a different locale for number formatting
   const localeForMessageFormat = (locale === 'en-XA' || locale === 'en-XL') ? 'de-DE' : locale;
-  // pre-process values for the message format like KB and milliseconds
-  const valuesForMessageFormat = _preprocessMessageValues(localeMessage, values);
 
   const formatter = new MessageFormat(localeMessage, localeForMessageFormat, formats);
-  const formattedString = formatter.format(valuesForMessageFormat);
 
+  // preformat values for the message format like KB and milliseconds
+  const valuesForMessageFormat = _preformatValues(localeMessage, formatter, values);
+
+  const formattedString = formatter.format(valuesForMessageFormat);
   return {formattedString, icuMessage: localeMessage};
 }
 

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

@@ -149,13 +149,15 @@ describe('i18n', () => {
 
     it('throws an error when values are needed but not provided', () => {
       expect(_ => i18n.getFormatted(str_(UIStrings.helloBytesWorld), 'en-US'))
-      .toThrow(`ICU Message contains a value reference ("in") that wasn't provided`);
+        // eslint-disable-next-line max-len
+        .toThrow(`ICU Message "Hello {in, number, bytes} World" contains a value reference ("in") that wasn't provided`);
     });
 
     it('throws an error when a value is missing', () => {
       expect(_ => i18n.getFormatted(str_(UIStrings.helloWorldMultiReplace,
         {hello: 'hello'}), 'en-US'))
-      .toThrow(`ICU Message contains a value reference ("world") that wasn't provided`);
+        // eslint-disable-next-line max-len
+        .toThrow(`ICU Message "{hello} {world}" contains a value reference ("world") that wasn't provided`);
     });
 
     it('formats a message with plurals', () => {
@@ -165,7 +167,8 @@ describe('i18n', () => {
 
     it('throws an error when a plural control value is missing', () => {
       expect(_ => i18n.getFormatted(str_(UIStrings.helloPlural), 'en-US'))
-      .toThrow(`ICU Message contains a value reference ("itemCount") that wasn't provided`);
+        // eslint-disable-next-line max-len
+        .toThrow(`ICU Message "{itemCount, plural, =1{1 hello} other{hellos}}" contains a value reference ("itemCount") that wasn't provided`);
     });
 
     it('formats a message with plurals and nested custom ICU', () => {
@@ -179,5 +182,14 @@ describe('i18n', () => {
         in: 1875});
       expect(helloStr).toBeDisplayString('hellos 1 goodbye 2');
     });
+
+    it('throws an error if a string value is used for a numeric placeholder', () => {
+      const helloStr = str_(UIStrings.helloTimeInMsWorld, {
+        timeInMs: 'string not a number',
+      });
+      expect(_ => i18n.getFormatted(helloStr, 'en-US'))
+        // eslint-disable-next-line max-len
+        .toThrow(`ICU Message "Hello {timeInMs, number, seconds} World" contains a numeric reference ("timeInMs") but provided value was not a number`);
+    });
   });
 });

package.json

@@ -79,7 +79,6 @@
     "@types/gh-pages": "^2.0.0",
     "@types/google.analytics": "0.0.39",
     "@types/inquirer": "^0.0.35",
-    "@types/intl-messageformat": "^1.3.0",
     "@types/jest": "^24.0.9",
     "@types/jpeg-js": "^0.3.0",
     "@types/lodash.isequal": "^4.5.2",
@@ -140,8 +139,8 @@
     "http-link-header": "^0.8.0",
     "inquirer": "^3.3.0",
     "intl": "^1.2.5",
-    "intl-messageformat": "^2.2.0",
-    "intl-messageformat-parser": "^1.4.0",
+    "intl-messageformat": "^4.4.0",
+    "intl-pluralrules": "^1.0.3",
     "jpeg-js": "0.1.2",
     "js-library-detector": "^5.4.0",
     "jsonld": "^1.5.0",

yarn.lock

@@ -493,11 +493,6 @@
   dependencies:
     "@types/node" "*"
 
-"@types/intl-messageformat@^1.3.0":
-  version "1.3.0"
-  resolved "https://registry.yarnpkg.com/@types/intl-messageformat/-/intl-messageformat-1.3.0.tgz#6af7144802b13d62ade9ad68f8420cdb33b75916"
-  integrity sha1-avcUSAKxPWKt6a1o+EIM2zO3WRY=
-
 "@types/istanbul-lib-coverage@^1.1.0":
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-1.1.0.tgz#2cc2ca41051498382b43157c8227fea60363f94a"
@@ -3941,17 +3936,24 @@ insert-module-globals@^7.0.0:
     undeclared-identifiers "^1.1.2"
     xtend "^4.0.0"
 
-intl-messageformat-parser@1.4.0, intl-messageformat-parser@^1.4.0:
-  version "1.4.0"
-  resolved "https://registry.yarnpkg.com/intl-messageformat-parser/-/intl-messageformat-parser-1.4.0.tgz#b43d45a97468cadbe44331d74bb1e8dea44fc075"
-  integrity sha1-tD1FqXRoytvkQzHXS7Ho3qRPwHU=
+intl-messageformat-parser@^1.8.1:
+  version "1.8.1"
+  resolved "https://registry.yarnpkg.com/intl-messageformat-parser/-/intl-messageformat-parser-1.8.1.tgz#0eb14c5618333be4c95c409457b66c8c33ddcc01"
+  integrity sha512-IMSCKVf0USrM/959vj3xac7s8f87sc+80Y/ipBzdKy4ifBv5Gsj2tZ41EAaURVg01QU71fYr77uA8Meh6kELbg==
 
-intl-messageformat@^2.2.0:
-  version "2.2.0"
-  resolved "https://registry.yarnpkg.com/intl-messageformat/-/intl-messageformat-2.2.0.tgz#345bcd46de630b7683330c2e52177ff5eab484fc"
-  integrity sha1-NFvNRt5jC3aDMwwuUhd/9eq0hPw=
+intl-messageformat@^4.4.0:
+  version "4.4.0"
+  resolved "https://registry.yarnpkg.com/intl-messageformat/-/intl-messageformat-4.4.0.tgz#aa196a4d04b573f4090bc417f982d81de4f74fad"
+  integrity sha512-z+Bj2rS3LZSYU4+sNitdHrwnBhr0wO80ZJSW8EzKDBowwUe3Q/UsvgCGjrwa+HPzoGCLEb9HAjfJgo4j2Sac8w==
   dependencies:
-    intl-messageformat-parser "1.4.0"
+    intl-messageformat-parser "^1.8.1"
+
+intl-pluralrules@^1.0.3:
+  version "1.0.3"
+  resolved "https://registry.yarnpkg.com/intl-pluralrules/-/intl-pluralrules-1.0.3.tgz#25438469f76fd13a4ac54a68a0ae4c0d30248a47"
+  integrity sha512-N+q+NmxJD5Pi+h7DoemDcNZN86e1dPSFUsWUtYtnSc/fKRen4vxCTcsmGveWOUgI9O9uSLTaiwJpC9f5xa2X4w==
+  dependencies:
+    make-plural "^4.3.0"
 
 intl@^1.2.5:
   version "1.2.5"
@@ -5215,6 +5217,13 @@ make-dir@^2.0.0:
     pify "^4.0.1"
     semver "^5.6.0"
 
+make-plural@^4.3.0:
+  version "4.3.0"
+  resolved "https://registry.yarnpkg.com/make-plural/-/make-plural-4.3.0.tgz#f23de08efdb0cac2e0c9ba9f315b0dff6b4c2735"
+  integrity sha512-xTYd4JVHpSCW+aqDof6w/MebaMVNTVYBZhbB/vi513xXdiPT92JMVCo0Jq8W2UZnzYRFeVbQiQ+I25l13JuKvA==
+  optionalDependencies:
+    minimist "^1.2.0"
+
 [email protected]:
   version "1.0.11"
   resolved "https://registry.yarnpkg.com/makeerror/-/makeerror-1.0.11.tgz#e01a5c9109f2af79660e4e8b9587790184f5a96c"