-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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: introduce script to swap in new locale to LHR #8755
Changes from 4 commits
a8ff2a0
81b0dc4
e12f714
30bb475
059bc0f
54c4833
bdd29b4
fdf96e8
5255aee
627f7be
3531318
5654af1
a225504
853bfd2
e3f2d3f
5efe7eb
cf0d3b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. empty fallback message concerns me a bit, maybe we should have not sure what to do there 🤷♂ |
||
return formattedString; | ||
} | ||
|
||
/** | ||
* @param {string} icuMessageInstanceId | ||
* @param {LH.Locale} locale | ||
|
@@ -331,6 +345,7 @@ module.exports = { | |
getRendererFormattedStrings, | ||
createMessageInstanceIdFn, | ||
getFormatted, | ||
formatMessageFromIdWithValues, | ||
replaceIcuMessageInstanceIds, | ||
isIcuMessage, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/** | ||
* @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 */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably shouldn't leave these on if this is for real now :) maybe we have a script version that does the |
||
|
||
const _set = require('lodash.set'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😍 |
||
|
||
const i18n = require('./i18n.js'); | ||
|
||
/** | ||
* Replaces all strings within an LHR with ones from a different locale | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this might be worth being the place where we explain how the
paulirish marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* @param {LH.Result} lhr | ||
* @param {LH.Locale} requestedLocale | ||
* @return {LH.Result} | ||
*/ | ||
function swapLocale(lhr, requestedLocale) { | ||
// copy LHR to avoid mutating provided LHR | ||
paulirish marked this conversation as resolved.
Show resolved
Hide resolved
|
||
lhr = JSON.parse(JSON.stringify(lhr)); | ||
|
||
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we |
||
const values = /** @type {LH.I18NMessageValuesEntry} */ (instance).values || undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably worth commenting that |
||
// Get new formatted strings in revised locale | ||
const formattedStr = i18n.formatMessageFromIdWithValues(locale, icuMessageId, values); | ||
// Write string back into the LHR | ||
_set(lhr, path, formattedStr); | ||
} | ||
}); | ||
|
||
// Tweak the config locale | ||
lhr.configSettings.locale = locale; | ||
return lhr; | ||
} | ||
|
||
module.exports = swapLocale; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/** | ||
* @license Copyright 2018 Google Inc. All Rights Reserved. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2019 :) |
||
* 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', () => { | ||
This comment was marked as resolved.
Sorry, something went wrong. |
||
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'); | ||
}); | ||
This comment was marked as outdated.
Sorry, something went wrong. |
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we should type values as |
||
export type I18NMessageEntry = string | I18NMessageValuesEntry; | ||
|
||
export interface I18NMessages { | ||
[icuMessageId: string]: I18NMessageEntry[]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about
to match
getFormatted
(param order forgetFormatted
vs_formatIcuMessage
is unfortunate but whatever)