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: upgrade to latest icu formatter #13834

Merged
merged 15 commits into from
Oct 5, 2023
4 changes: 3 additions & 1 deletion core/lib/i18n/i18n.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import url from 'url';
import lookupClosestLocale from 'lookup-closest-locale';
import log from 'lighthouse-logger';

import {getAvailableLocales} from '../../../shared/localization/format.js';
import {escapeIcuMessage, getAvailableLocales} from '../../../shared/localization/format.js';
import {LH_ROOT} from '../../../shared/root.js';
import {isIcuMessage, formatMessage, DEFAULT_LOCALE} from '../../../shared/localization/format.js';
import {getModulePath} from '../../../shared/esm-utils.js';
Expand Down Expand Up @@ -202,6 +202,8 @@ function createIcuMessageFn(filename, fileStrings) {
const unixStyleFilename = filenameToLookup.replace(/\\/g, '/');
const i18nId = `${unixStyleFilename} | ${keyname}`;

message = escapeIcuMessage(message);
connorjclark marked this conversation as resolved.
Show resolved Hide resolved

return {
i18nId,
values,
Expand Down
52 changes: 26 additions & 26 deletions core/scripts/i18n/collect-strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {pathToFileURL} from 'url';
import glob from 'glob';
import {expect} from 'expect';
import tsc from 'typescript';
import MessageParser from 'intl-messageformat-parser';
import MessageParser from '@formatjs/icu-messageformat-parser';
import esMain from 'es-main';
import isDeepEqual from 'lodash/isEqual.js';

Expand All @@ -24,6 +24,7 @@ import {pruneObsoleteLhlMessages} from './prune-obsolete-lhl-messages.js';
import {countTranslatedMessages} from './count-translated.js';
import {LH_ROOT} from '../../../shared/root.js';
import {resolveModulePath} from '../../../shared/esm-utils.js';
import {escapeIcuMessage} from '../../../shared/localization/format.js';

// Match declarations of UIStrings, terminating in either a `};\n` (very likely to always be right)
// or `}\n\n` (allowing semicolon to be optional, but insisting on a double newline so that an
Expand Down Expand Up @@ -160,7 +161,7 @@ function parseExampleJsDoc(rawExample) {
* @return {IncrementalCtc}
*/
function convertMessageToCtc(lhlMessage, examples = {}) {
_lhlValidityChecks(lhlMessage);
_lhlValidityChecks(escapeIcuMessage(lhlMessage));

/** @type {IncrementalCtc} */
const ctc = {
Expand Down Expand Up @@ -189,36 +190,35 @@ function convertMessageToCtc(lhlMessage, examples = {}) {
* @param {string} lhlMessage
*/
function _lhlValidityChecks(lhlMessage) {
let parsedMessage;
let parsedMessageElements;
try {
parsedMessage = MessageParser.parse(lhlMessage);
parsedMessageElements = MessageParser.parse(lhlMessage);
} catch (err) {
if (err.name !== 'SyntaxError') throw err;
// Improve the intl-messageformat-parser syntax error output.
/** @type {Array<{text: string}>} */
const expected = err.expected;
const expectedStr = expected.map(exp => `'${exp.text}'`).join(', ');
throw new Error(`Did not find the expected syntax (one of ${expectedStr}) in message "${lhlMessage}"`);
throw new Error(`[${err.message}] Did not find the expected syntax in message: ${err.originalMessage}`);
}

for (const element of parsedMessage.elements) {
if (element.type !== 'argumentElement' || !element.format) continue;

if (element.format.type === 'pluralFormat' || element.format.type === 'selectFormat') {
// `plural`/`select` arguments can't have content before or after them.
// See http://userguide.icu-project.org/formatparse/messages#TOC-Complex-Argument-Types
// e.g. https://github.com/GoogleChrome/lighthouse/pull/11068#discussion_r451682796
if (parsedMessage.elements.length > 1) {
throw new Error(`Content cannot appear outside plural or select ICU messages. Instead, repeat that content in each option (message: '${lhlMessage}')`);
}

// Each option value must also be a valid lhlMessage.
for (const option of element.format.options) {
const optionStr = lhlMessage.slice(option.value.location.start.offset, option.value.location.end.offset);
_lhlValidityChecks(optionStr);
/**
* @param {MessageParser.MessageFormatElement[]} elements
*/
function validate(elements) {
for (const element of elements) {
if (element.type === MessageParser.TYPE.plural || element.type === MessageParser.TYPE.select) {
// `plural`/`select` arguments can't have content before or after them.
// See http://userguide.icu-project.org/formatparse/messages#TOC-Complex-Argument-Types
// e.g. https://github.com/GoogleChrome/lighthouse/pull/11068#discussion_r451682796
if (elements.length > 1) {
throw new Error(`Content cannot appear outside plural or select ICU messages. Instead, repeat that content in each option (message: '${lhlMessage}')`);
}

for (const option of Object.values(element.options)) {
validate(option.value);
}
}
}
}

validate(parsedMessageElements);
}

/**
Expand Down Expand Up @@ -389,7 +389,7 @@ function _processPlaceholderDirectIcu(icu, examples) {
for (const [key, value] of Object.entries(examples)) {
// Make sure all examples have ICU vars
if (!icu.message.includes(`{${key}}`)) {
throw Error(`Example '${key}' provided, but has not corresponding ICU replacement in message "${icu.message}"`);
throw Error(`Example '${key}' provided, but has no corresponding ICU replacement in message "${icu.message}"`);
}
const eName = `ICU_${idx++}`;
tempMessage = tempMessage.replace(`{${key}}`, `$${eName}$`);
Expand Down Expand Up @@ -517,7 +517,7 @@ function parseUIStrings(sourceStr, liveUIStrings) {
const key = getIdentifier(property);

// Use live message to avoid having to e.g. concat strings broken into parts.
const message = liveUIStrings[key];
const message = (liveUIStrings[key]);

// @ts-expect-error - Not part of the public tsc interface yet.
const jsDocComments = tsc.getJSDocCommentsAndTags(property);
Expand Down
12 changes: 6 additions & 6 deletions core/scripts/i18n/prune-obsolete-lhl-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import fs from 'fs';
import path from 'path';

import glob from 'glob';
import MessageParser from 'intl-messageformat-parser';
import MessageParser from '@formatjs/icu-messageformat-parser';

import {collectAllCustomElementsFromICU} from '../../../shared/localization/format.js';
import {collectAllCustomElementsFromICU, escapeIcuMessage} from '../../../shared/localization/format.js';
import {LH_ROOT} from '../../../shared/root.js';
import {readJson} from '../../test/test-utils.js';

Expand All @@ -24,8 +24,8 @@ import {readJson} from '../../test/test-utils.js';
* @return {boolean}
*/
function equalArguments(goldenArgumentIds, lhlMessage) {
const parsedMessage = MessageParser.parse(lhlMessage);
const lhlArgumentElements = collectAllCustomElementsFromICU(parsedMessage.elements);
const parsedMessageElements = MessageParser.parse(escapeIcuMessage(lhlMessage));
const lhlArgumentElements = collectAllCustomElementsFromICU(parsedMessageElements);
const lhlArgumentIds = [...lhlArgumentElements.keys()];

if (goldenArgumentIds.length !== lhlArgumentIds.length) return false;
Expand Down Expand Up @@ -96,8 +96,8 @@ function getGoldenLocaleArgumentIds(goldenLhl) {
const goldenLocaleArgumentIds = {};

for (const [messageId, {message}] of Object.entries(goldenLhl)) {
const parsedMessage = MessageParser.parse(message);
const goldenArgumentElements = collectAllCustomElementsFromICU(parsedMessage.elements);
const parsedMessageElements = MessageParser.parse(escapeIcuMessage(message));
const goldenArgumentElements = collectAllCustomElementsFromICU(parsedMessageElements);
const goldenArgumentIds = [...goldenArgumentElements.keys()].sort();

goldenLocaleArgumentIds[messageId] = goldenArgumentIds;
Expand Down
8 changes: 4 additions & 4 deletions core/test/scripts/i18n/collect-strings-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ describe('#_lhlValidityChecks', () => {
it('errors when using non-supported custom-formatted ICU format', () => {
const message = 'Hello World took {var, badFormat, milliseconds}.';
expect(() => collect.convertMessageToCtc(message)).toThrow(
/Did not find the expected syntax \(one of 'number', 'date', 'time', 'plural', 'selectordinal', 'select'\) in message "Hello World took {var, badFormat, milliseconds}."$/);
/\[INVALID_ARGUMENT_TYPE\] Did not find the expected syntax in message: Hello World took {var, badFormat, milliseconds}.$/);
});

it('errors when there is content outside of a plural argument', () => {
Expand Down Expand Up @@ -370,14 +370,14 @@ describe('#_lhlValidityChecks', () => {
/Content cannot appear outside plural or select ICU messages.*=1 {1 request} other {# requests}}'\)$/);
});

it('errors when there is content outside of nested plural aguments', () => {
it('errors when there is content outside of nested plural arguments', () => {
const message = `{user_gender, select,
female {Ms. {name} received {count, plural, =1 {one award.} other {# awards.}}}
male {Mr. {name} received {count, plural, =1 {one award.} other {# awards.}}}
other {{name} received {count, plural, =1 {one award.} other {# awards.}}}
}`;
expect(() => collect.convertMessageToCtc(message, {name: 'Elbert'})).toThrow(
/Content cannot appear outside plural or select ICU messages.*\(message: 'Ms. {name} received {count, plural, =1 {one award.} other {# awards.}}'\)$/);
/Content cannot appear outside plural or select ICU messages.*\(message: '{user_gender, select/);
});
/* eslint-enable max-len */
});
Expand Down Expand Up @@ -562,7 +562,7 @@ describe('Convert Message to Placeholder', () => {
const message = 'Hello name.';
expect(() => collect.convertMessageToCtc(message, {name: 'Mary'}))
// eslint-disable-next-line max-len
.toThrow(/Example 'name' provided, but has not corresponding ICU replacement in message "Hello name."/);
.toThrow(/Example 'name' provided, but has no corresponding ICU replacement in message "Hello name."/);
});

it('errors when direct ICU has no examples', () => {
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
},
"devDependencies": {
"@build-tracker/cli": "^1.0.0-beta.15",
"@formatjs/icu-messageformat-parser": "^2.6.2",
"@esbuild-kit/esm-loader": "^2.1.1",
"@esbuild-plugins/node-modules-polyfill": "^0.1.4",
"@jest/fake-timers": "^28.1.0",
Expand Down Expand Up @@ -152,7 +153,6 @@
"gh-pages": "^2.0.1",
"glob": "^7.1.3",
"idb-keyval": "2.2.0",
"intl-messageformat-parser": "^1.8.1",
"jest-mock": "^27.3.0",
"jest-snapshot": "^28.1.0",
"jsdom": "^12.2.0",
Expand Down Expand Up @@ -188,7 +188,7 @@
"devtools-protocol": "0.0.1200039",
"enquirer": "^2.3.6",
"http-link-header": "^1.1.1",
"intl-messageformat": "^4.4.0",
"intl-messageformat": "^10.5.3",
"jpeg-js": "^0.4.4",
"js-library-detector": "^6.7.0",
"lighthouse-logger": "^2.0.1",
Expand Down
Loading
Loading