Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 9 additions & 29 deletions core/audits/insights/image-delivery-insight.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,22 @@
* SPDX-License-Identifier: Apache-2.0
*/

import {UIStrings, ImageOptimizationType} from '@paulirish/trace_engine/models/trace/insights/ImageDelivery.js';
import * as ImageDeliveryInsightModule from '@paulirish/trace_engine/models/trace/insights/ImageDelivery.js';
import {UIStrings} from '@paulirish/trace_engine/models/trace/insights/ImageDelivery.js';

import {Audit} from '../audit.js';
import * as i18n from '../../lib/i18n/i18n.js';
import {adaptInsightToAuditProduct} from './insight-audit.js';
import {TraceEngineResult} from '../../computed/trace-engine-result.js';

// eslint-disable-next-line max-len
const str_ = i18n.createIcuMessageFn('node_modules/@paulirish/trace_engine/models/trace/insights/ImageDelivery.js', UIStrings);

const getOptimizationMessage =
// @ts-expect-error TODO(cjamcl): update trace engine lib to modify types accordingly.
// right now return type of getOptimizationMessage is wrongly `string`.
TraceEngineResult.localizeFunction(str_, ImageDeliveryInsightModule.getOptimizationMessage);

class ImageDeliveryInsight extends Audit {
/**
* @return {LH.Audit.Meta}
Expand All @@ -34,33 +41,6 @@ class ImageDeliveryInsight extends Audit {
};
}

/**
* Note: This function is a copy of the `getOptimizationMessage` function found in the imported
* module. We could re-use the output of that function but it's output is shimmed to a {i18nId, values} object
* which is not consistent with the TS return type.
*
* We also can't change the function to output the untranslated strings because the responsive
* size string has placeholders that need to be resolved here.
*
* @param {import('@paulirish/trace_engine/models/trace/insights/ImageDelivery.js').ImageOptimization} optimization
* @returns
*/
static getOptimizationMessage(optimization) {
switch (optimization.type) {
case ImageOptimizationType.ADJUST_COMPRESSION:
return str_(UIStrings.useCompression);
case ImageOptimizationType.MODERN_FORMAT_OR_COMPRESSION:
return str_(UIStrings.useModernFormat);
case ImageOptimizationType.VIDEO_FORMAT:
return str_(UIStrings.useVideoFormat);
case ImageOptimizationType.RESPONSIVE_SIZE:
return str_(UIStrings.useResponsiveSize, {
PH1: `${optimization.fileDimensions.width}x${optimization.fileDimensions.height}`,
PH2: `${optimization.displayDimensions.width}x${optimization.displayDimensions.height}`,
});
}
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
Expand Down Expand Up @@ -90,7 +70,7 @@ class ImageDeliveryInsight extends Audit {
subItems: {
type: /** @type {const} */ ('subitems'),
items: image.optimizations.map(optimization => ({
reason: this.getOptimizationMessage(optimization),
reason: getOptimizationMessage(optimization),
wastedBytes: optimization.byteSavings,
})),
},
Expand Down
102 changes: 78 additions & 24 deletions core/computed/trace-engine-result.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,57 @@ class TraceEngineResult {
}

/**
* @param {import('@paulirish/trace_engine/models/trace/insights/types.js').TraceInsightSets} insightSets
* Adapts the given DevTools function that returns a localized string to one
* that returns a LH.IcuMessage.
*
* @template {any[]} Args
* @template {import('../lib/trace-engine.js').DevToolsIcuMessage} Ret
* @param {ReturnType<i18n.createIcuMessageFn>} str_
* @param {(...args: Args) => Ret} fn
* @return {(...args: Args) => LH.IcuMessage}
*/
static localizeInsights(insightSets) {
static localizeFunction(str_, fn) {
return (...args) => this.localize(str_, fn(...args));
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

localizeFunction is pretty much the only addition to this file. Everything else is shuffling code around.


/**
* Converts the input parameters given to `i18nString` usages in DevTools to a
* LH.IcuMessage.
*
* @param {ReturnType<i18n.createIcuMessageFn>} str_
* @param {import('../lib/trace-engine.js').DevToolsIcuMessage} traceEngineI18nObject
* @return {LH.IcuMessage}
*/
static localize(str_, traceEngineI18nObject) {
/** @type {Record<string, string|number>|undefined} */
let values;
if (traceEngineI18nObject.values) {
values = {};
for (const [key, value] of Object.entries(traceEngineI18nObject.values)) {
if (value && typeof value === 'object' && '__i18nBytes' in value) {
values[key] = value.__i18nBytes;
// TODO: use an actual byte formatter. Right now, this shows the exact number of bytes.
} else if (value && typeof value === 'object' && 'i18nId' in value) {
// TODO: add support for str_ values to be IcuMessage. For now, we translate it here.
// This means that locale swapping won't work for this portion of the IcuMessage.
// @ts-expect-error
values[key] = str_(value.i18nId, value.values).formattedDefault;
} else {
values[key] = value;
}
}
}

return str_(traceEngineI18nObject.i18nId, values);
}

/**
* Recursively finds all DevToolsIcuMessage objects and replaces them with LH.IcuMessage.
*
* @param {ReturnType<i18n.createIcuMessageFn>} str_
* @param {object} object
*/
static localizeObject(str_, object) {
/**
* Execute `cb(traceEngineI18nObject)` on every i18n object, recursively. The cb return
* value replaces traceEngineI18nObject.
Expand Down Expand Up @@ -79,6 +127,33 @@ class TraceEngineResult {
}
}

// Pass `{i18nId: string, values?: {}}` through Lighthouse's i18n pipeline.
// This is equivalent to if we directly did `str_(UIStrings.whatever, ...)`
recursiveReplaceLocalizableStrings(object, (traceEngineI18nObject) => {
let values = traceEngineI18nObject.values;
if (values) {
values = structuredClone(values);
for (const [key, value] of Object.entries(values)) {
if (value && typeof value === 'object' && '__i18nBytes' in value) {
// @ts-expect-error
values[key] = value.__i18nBytes;
// TODO: use an actual byte formatter. Right now, this shows the exact number of bytes.
} else if (value && typeof value === 'object' && 'i18nId' in value) {
// TODO: add support for str_ values to be IcuMessage.
// @ts-expect-error
values[key] = str_(value.i18nId, value.values).formattedDefault;
}
}
}

return str_(traceEngineI18nObject.i18nId, values);
}, new Set());
}

/**
* @param {import('@paulirish/trace_engine/models/trace/insights/types.js').TraceInsightSets} insightSets
*/
static localizeInsights(insightSets) {
for (const insightSet of insightSets.values()) {
for (const [name, model] of Object.entries(insightSet.model)) {
if (model instanceof Error) {
Expand All @@ -96,28 +171,7 @@ class TraceEngineResult {

const key = `node_modules/@paulirish/trace_engine/models/trace/insights/${name}.js`;
const str_ = i18n.createIcuMessageFn(key, traceEngineUIStrings);

// Pass `{i18nId: string, values?: {}}` through Lighthouse's i18n pipeline.
// This is equivalent to if we directly did `str_(UIStrings.whatever, ...)`
recursiveReplaceLocalizableStrings(model, (traceEngineI18nObject) => {
let values = traceEngineI18nObject.values;
if (values) {
values = structuredClone(values);
for (const [key, value] of Object.entries(values)) {
if (value && typeof value === 'object' && '__i18nBytes' in value) {
// @ts-expect-error
values[key] = value.__i18nBytes;
// TODO: use an actual byte formatter. Right now, this shows the exact number of bytes.
} else if (value && typeof value === 'object' && 'i18nId' in value) {
// TODO: add support for str_ values to be IcuMessage.
// @ts-expect-error
values[key] = str_(value.i18nId, value.values).formattedDefault;
}
}
}

return str_(traceEngineI18nObject.i18nId, values);
}, new Set());
this.localizeObject(str_, model);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions core/lib/trace-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {polyfillDOMRect} from './polyfill-dom-rect.js';

/** @typedef {import('@paulirish/trace_engine').Types.Events.SyntheticLayoutShift} SyntheticLayoutShift */
/** @typedef {SyntheticLayoutShift & {args: {data: NonNullable<SyntheticLayoutShift['args']['data']>}}} SaneSyntheticLayoutShift */
/** @typedef {{i18nId: string, values: Record<string, string|number|{__i18nBytes: number}>}} DevToolsIcuMessage */

polyfillDOMRect();

Expand Down