diff --git a/README.md b/README.md index c2d0848..bc33c92 100644 --- a/README.md +++ b/README.md @@ -553,8 +553,28 @@ Key exported types: | `LabWriteOptions` | `LabAuditOptions` plus `clean` — what `runLabToDisk` takes | | `LabPlanControls` | Plan-level controls only: `continueOnError`, `reuseBrowser`, `repeats` | | `LabPlanOptions` | `LabPlanControls` plus the per-run options, minus `runNumber` and `port`, which `runLabPlan` owns | +| `LabProfile` | One `PROFILES` entry: `{ network, device, label }`, both keys `null` for the `native` profile | +| `NetworkPreset` | One `NETWORK_PRESETS` entry: nominal `rttMs`, `throughputKbps`, `uploadKbps`, `cpuSlowdownMultiplier`, `label` | +| `DevicePreset` | One `DEVICE_PRESETS` entry: `width`, `height`, `deviceScaleFactor`, `mobile`, `formFactor`, `label` | | `RunSummary` | Variance record for one repeated (URL x profile) pair: median, spread, `benchmarkIndex` range, per-metric arrays, stability warnings | +Every exported constant is frozen and published as `readonly`: the arrays `CHROME_FLAGS`, +`DEFAULT_SKIP_AUDITS`, `LAB_CATEGORIES`, `PSI_STRATEGIES`, `DEFAULT_PSI_STRATEGIES`, +`CRUX_FORM_FACTORS` and `DEFAULT_CRUX_FORM_FACTORS`, plus the three preset objects +`PROFILES`, `NETWORK_PRESETS` and `DEVICE_PRESETS` (from `web-perf-cli/profiles`). Mutating +any of them fails to compile, and at runtime throws under strict mode (ESM, or `'use strict'`) +rather than taking effect. + +The three objects are frozen at *every* level, because the damaging write is one step down: +`resolveProfileSettings` reads the presets on every audit, so `PROFILES.low.network = 'wifi'` +would silently change what later runs measure while the report still named the original +profile. Build a variant by spreading instead: + +```js +const custom = { ...NETWORK_PRESETS['3g'], rttMs: 250 }; +const onWifi = { ...PROFILES.low, network: 'wifi' }; +``` + ## Development ```bash diff --git a/lib/index.js b/lib/index.js index 3f84ee1..965c4a9 100644 --- a/lib/index.js +++ b/lib/index.js @@ -13,6 +13,9 @@ * @typedef {import('./crux').CruxFormFactor} CruxFormFactor * @typedef {import('./crux-history').CruxHistoryReport} CruxHistoryReport * @typedef {import('./crux-history').CruxHistoryBatchResult} CruxHistoryBatchResult + * @typedef {import('./profiles').LabProfile} LabProfile + * @typedef {import('./profiles').NetworkPreset} NetworkPreset + * @typedef {import('./profiles').DevicePreset} DevicePreset * @typedef {import('./variance').RunSummary} RunSummary */ diff --git a/lib/profiles.js b/lib/profiles.js index 90925b2..185bc7e 100644 --- a/lib/profiles.js +++ b/lib/profiles.js @@ -10,12 +10,53 @@ const LAB_CATEGORIES = Object.freeze(['performance', 'accessibility', 'best-prac const DEVTOOLS_RTT_ADJUSTMENT_FACTOR = 3.75; const DEVTOOLS_THROUGHPUT_ADJUSTMENT_FACTOR = 0.9; +/** + * Recursively freezes an object and every nested object it owns. + * + * `Object.freeze` on its own is shallow, and shallow is the wrong depth for the presets + * below: it blocks `PROFILES.low = {...}` but leaves `PROFILES.low.network = 'wifi'` + * writable, which is the mutation that matters. Every later `--profile=low` audit would + * then run on WiFi while the report still reports the profile as "low" — a wrong number + * with nothing signalling it, which is worse than a failed run. + * + * The parent is frozen BEFORE recursing, which is what makes the `Object.isFrozen` guard + * terminate on a self-referential object: freezing afterwards would leave the parent + * unfrozen when the recursion re-entered it, and the guard would never fire. The guard + * also skips subtrees that are already frozen. + * + * @template {object} T + * @param {T} obj + * @returns {T} the same object, now frozen at every level + */ +function deepFreeze(obj) { + Object.freeze(obj); + for (const value of Object.values(obj)) { + if (value !== null && typeof value === 'object' && !Object.isFrozen(value)) { + deepFreeze(value); + } + } + return obj; +} + // INVARIANT: `throughputKbps` and `uploadKbps` are always the NOMINAL (pre-adjustment) // link speeds, matching how DevTools names its presets. buildThrottling() owns the // 3.75 / 0.9 factors and is the only place they may be applied. Storing an already // adjusted value here double-applies the factor — that bug shipped in '4g', whose // uploadKbps held 675 (= 750 * 0.9) and so emitted 607.5 instead of 675. -const NETWORK_PRESETS = { +/** + * One entry in NETWORK_PRESETS. Named rather than inline so the published declarations + * describe a preset once instead of repeating its structure per key. + * + * @typedef {Object} NetworkPreset + * @property {number} rttMs - nominal round-trip time, in milliseconds + * @property {number} throughputKbps - nominal (pre-adjustment) download speed; `buildThrottling` applies the DevTools factors + * @property {number} uploadKbps - nominal (pre-adjustment) upload speed; `buildThrottling` applies the DevTools factors + * @property {number} cpuSlowdownMultiplier + * @property {string} label - the one-line summary printed by `list-profiles` + */ + +/** @type {Readonly>>} */ +const NETWORK_PRESETS = deepFreeze({ '3g-slow': { rttMs: 400, throughputKbps: 400, @@ -58,9 +99,22 @@ const NETWORK_PRESETS = { cpuSlowdownMultiplier: 1, label: 'No throttling', }, -}; +}); -const DEVICE_PRESETS = { +/** + * One entry in DEVICE_PRESETS: the screen emulation and form factor for a device. + * + * @typedef {Object} DevicePreset + * @property {number} width + * @property {number} height + * @property {number} deviceScaleFactor + * @property {boolean} mobile + * @property {'mobile'|'desktop'} formFactor + * @property {string} label - the one-line summary printed by `list-profiles` + */ + +/** @type {Readonly>>} */ +const DEVICE_PRESETS = deepFreeze({ 'moto-g-power': { width: 412, height: 823, @@ -109,9 +163,19 @@ const DEVICE_PRESETS = { formFactor: 'desktop', label: '1920x1080 @ 1x (desktop)', }, -}; +}); -const PROFILES = { +/** + * One entry in PROFILES: a named pairing of a network preset with a device preset. + * + * @typedef {Object} LabProfile + * @property {string|null} network - a NETWORK_PRESETS key, or null for the native profile + * @property {string|null} device - a DEVICE_PRESETS key, or null for the native profile + * @property {string} label - the one-line summary printed by `list-profiles` + */ + +/** @type {Readonly>>} */ +const PROFILES = deepFreeze({ low: { network: '3g', device: 'moto-g-power', @@ -132,7 +196,7 @@ const PROFILES = { device: null, label: 'Native device (no throttling, no emulation — actual hardware)', }, -}; +}); /** * Converts a nominal network preset into a Lighthouse `throttling` settings object, diff --git a/lib/profiles.test.js b/lib/profiles.test.js index 07c99b0..ad3b306 100644 --- a/lib/profiles.test.js +++ b/lib/profiles.test.js @@ -5,7 +5,9 @@ const { buildScreenEmulation, resolveProfileSettings, LAB_CATEGORIES, + PROFILES, NETWORK_PRESETS, + DEVICE_PRESETS, DEVTOOLS_RTT_ADJUSTMENT_FACTOR, DEVTOOLS_THROUGHPUT_ADJUSTMENT_FACTOR, } = require('./profiles'); @@ -220,3 +222,60 @@ describe('resolveProfileSettings', () => { expect(result.emulatedUserAgent).not.toContain('Mobile'); }); }); + +// This file is an ES module, so it runs in strict mode and a write to a frozen object throws +// instead of failing silently. A consumer in sloppy-mode CommonJS gets the silent no-op — +// still a mutation that never lands, which is the property these tests are about. +describe('exported preset objects are deeply frozen', () => { + const cases = [ + ['PROFILES', PROFILES, 'low', 'network', 'wifi'], + ['NETWORK_PRESETS', NETWORK_PRESETS, '3g', 'rttMs', 1], + ['DEVICE_PRESETS', DEVICE_PRESETS, 'iphone-12', 'width', 9999], + ]; + + it.each(cases)('%s is frozen at both levels', (_name, obj) => { + expect(Object.isFrozen(obj)).toBe(true); + for (const entry of Object.values(obj)) { + expect(Object.isFrozen(entry)).toBe(true); + } + }); + + // Object.freeze on its own would pass the test above for the container and fail this one: + // it leaves every nested preset writable, and the nested write is the damaging one. + it.each(cases)('%s rejects a nested write', (_name, obj, key, prop, value) => { + const before = obj[key][prop]; + expect(() => { + obj[key][prop] = value; + }).toThrow(TypeError); + expect(obj[key][prop]).toBe(before); + }); + + // The identity check is the assertion that carries this case for a sloppy-mode consumer, + // where the write is a silent no-op and never throws: `{}` is still an object, so only + // comparing against the entry held before the write proves the replacement did not land. + it.each(cases)('%s rejects replacing a whole entry', (_name, obj, key) => { + const before = obj[key]; + expect(() => { + obj[key] = {}; + }).toThrow(TypeError); + expect(obj[key]).toBe(before); + }); + + it.each(cases)('%s still reads nested values', (_name, obj, key, prop) => { + expect(obj[key][prop]).toBeDefined(); + }); + + it.each(cases)('%s can still be extended through a copy', (_name, obj, key, prop, value) => { + const variant = { ...obj[key], [prop]: value }; + expect(variant[prop]).toBe(value); + expect(obj[key][prop]).not.toBe(value); + }); + + // The freeze must not change what an audit resolves to — the presets are still the values + // resolveProfileSettings reads on every run. + it('leaves resolveProfileSettings reading the same values', () => { + const settings = resolveProfileSettings({ profile: 'low' }); + expect(settings.throttling.rttMs).toBe(NETWORK_PRESETS['3g'].rttMs); + expect(settings.screenEmulation.width).toBe(DEVICE_PRESETS['moto-g-power'].width); + }); +}); diff --git a/type-tests/frozen-constants.ts b/type-tests/frozen-constants.ts index fbdd4b1..ec94ae0 100644 --- a/type-tests/frozen-constants.ts +++ b/type-tests/frozen-constants.ts @@ -9,7 +9,8 @@ import { runPsi, runPsiBatch, PSI_STRATEGIES, DEFAULT_PSI_STRATEGIES } from '@hugoer/web-perf-cli/psi'; import { runCrux, DEFAULT_CRUX_FORM_FACTORS } from '@hugoer/web-perf-cli/crux'; import { buildLighthouseConfig, CHROME_FLAGS, DEFAULT_SKIP_AUDITS } from '@hugoer/web-perf-cli/lab'; -import { LAB_CATEGORIES } from '@hugoer/web-perf-cli/profiles'; +import { LAB_CATEGORIES, PROFILES, NETWORK_PRESETS, DEVICE_PRESETS } from '@hugoer/web-perf-cli/profiles'; +import type { LabProfile, NetworkPreset, DevicePreset } from '@hugoer/web-perf-cli/profiles'; // --- PSI strategies ------------------------------------------------------------------------- export const psiDefault = runPsi('https://example.com', 'KEY', undefined, { @@ -48,3 +49,35 @@ CHROME_FLAGS.push('--headless=new'); export const categories: string[] = [...LAB_CATEGORIES]; // @ts-expect-error LAB_CATEGORIES is frozen and published as readonly LAB_CATEGORIES.push('performance'); + +// --- profile, network and device presets ---------------------------------------------------- +// These are objects, not arrays, so the mutation that matters is one level down: freezing the +// container alone leaves PROFILES.low.network writable, and that write changes what every +// later audit measures while the report still names the original profile. +export const profile: LabProfile = PROFILES.low; +export const network: NetworkPreset = NETWORK_PRESETS['3g']; +export const device: DevicePreset = DEVICE_PRESETS['iphone-12']; + +export const rttMs: number = NETWORK_PRESETS['3g'].rttMs; +export const formFactor: 'mobile' | 'desktop' = DEVICE_PRESETS.desktop.formFactor; +export const profileNetwork: string | null = PROFILES.native.network; + +// The supported way to build a variant: spread, do not mutate. +export const slowerThan3g: NetworkPreset = { ...NETWORK_PRESETS['3g'], rttMs: 250 }; +export const taller: DevicePreset = { ...DEVICE_PRESETS['iphone-12'], height: 1000 }; +export const onWifi: LabProfile = { ...PROFILES.low, network: 'wifi' }; + +// @ts-expect-error PROFILES entries are frozen and published as readonly +PROFILES.low.network = 'wifi'; +// @ts-expect-error NETWORK_PRESETS entries are frozen and published as readonly +NETWORK_PRESETS['3g'].rttMs = 1; +// @ts-expect-error DEVICE_PRESETS entries are frozen and published as readonly +DEVICE_PRESETS['iphone-12'].width = 9999; + +// Replacing a whole entry is blocked too — the container itself is readonly. +// @ts-expect-error PROFILES is frozen and published as readonly +PROFILES.low = onWifi; +// @ts-expect-error NETWORK_PRESETS is frozen and published as readonly +NETWORK_PRESETS['3g'] = slowerThan3g; +// @ts-expect-error DEVICE_PRESETS is frozen and published as readonly +DEVICE_PRESETS['iphone-12'] = taller; diff --git a/type-tests/root.ts b/type-tests/root.ts index 6d00d61..4ff50b7 100644 --- a/type-tests/root.ts +++ b/type-tests/root.ts @@ -7,6 +7,7 @@ import type { LabReport, LabPlanResult, PsiReport, PsiBatchResult, CruxReport, CruxMetric, CruxBatchResult, CruxFormFactor, CruxHistoryReport, CruxHistoryBatchResult, RunSummary, + LabProfile, NetworkPreset, DevicePreset, } from '@hugoer/web-perf-cli'; export const fns = [runCruxAudit, runPsiAudit, runLabAudit, normalizeOrigin]; @@ -17,4 +18,5 @@ export type Rows = [ LabReport, LabPlanResult, PsiReport, PsiBatchResult, CruxReport, CruxMetric, CruxBatchResult, CruxFormFactor, CruxHistoryReport, CruxHistoryBatchResult, RunSummary, + LabProfile, NetworkPreset, DevicePreset, ]; diff --git a/types/lib/index.d.ts b/types/lib/index.d.ts index 8d58139..eab7e0b 100644 --- a/types/lib/index.d.ts +++ b/types/lib/index.d.ts @@ -1,5 +1,5 @@ declare namespace _exports { - export { LabReport, LabPlanResult, PsiReport, PsiBatchResult, CruxReport, CruxMetric, CruxBatchResult, CruxFormFactor, CruxHistoryReport, CruxHistoryBatchResult, RunSummary }; + export { LabReport, LabPlanResult, PsiReport, PsiBatchResult, CruxReport, CruxMetric, CruxBatchResult, CruxFormFactor, CruxHistoryReport, CruxHistoryBatchResult, LabProfile, NetworkPreset, DevicePreset, RunSummary }; } declare namespace _exports { const runLabAudit: typeof import("./lab").runLabAudit; @@ -87,6 +87,24 @@ type CruxHistoryReport = import("./crux-history").CruxHistoryReport; * documented `import type { CruxReport } from '@hugoer/web-perf-cli'` fails with TS2305. */ type CruxHistoryBatchResult = import("./crux-history").CruxHistoryBatchResult; +/** + * The types the README's "Key exported types" table promises from the package root. Re-declared + * here because lib/index.js is a value-only façade of lazy getters: without these, the + * documented `import type { CruxReport } from '@hugoer/web-perf-cli'` fails with TS2305. + */ +type LabProfile = import("./profiles").LabProfile; +/** + * The types the README's "Key exported types" table promises from the package root. Re-declared + * here because lib/index.js is a value-only façade of lazy getters: without these, the + * documented `import type { CruxReport } from '@hugoer/web-perf-cli'` fails with TS2305. + */ +type NetworkPreset = import("./profiles").NetworkPreset; +/** + * The types the README's "Key exported types" table promises from the package root. Re-declared + * here because lib/index.js is a value-only façade of lazy getters: without these, the + * documented `import type { CruxReport } from '@hugoer/web-perf-cli'` fails with TS2305. + */ +type DevicePreset = import("./profiles").DevicePreset; /** * The types the README's "Key exported types" table promises from the package root. Re-declared * here because lib/index.js is a value-only façade of lazy getters: without these, the diff --git a/types/lib/profiles.d.ts b/types/lib/profiles.d.ts index 7b44fec..5acbbc2 100644 --- a/types/lib/profiles.d.ts +++ b/types/lib/profiles.d.ts @@ -1,128 +1,93 @@ -export namespace PROFILES { - namespace low { - let network: string; - let device: string; - let label: string; - } - namespace medium { - let network_1: string; - export { network_1 as network }; - let device_1: string; - export { device_1 as device }; - let label_1: string; - export { label_1 as label }; - } - namespace high { - let network_2: string; - export { network_2 as network }; - let device_2: string; - export { device_2 as device }; - let label_2: string; - export { label_2 as label }; - } - namespace native { - let network_3: null; - export { network_3 as network }; - let device_3: null; - export { device_3 as device }; - let label_3: string; - export { label_3 as label }; - } -} -export const NETWORK_PRESETS: { - '3g-slow': { - rttMs: number; - throughputKbps: number; - uploadKbps: number; - cpuSlowdownMultiplier: number; - label: string; - }; - '3g': { - rttMs: number; - throughputKbps: number; - uploadKbps: number; - cpuSlowdownMultiplier: number; - label: string; - }; - '4g': { - rttMs: number; - throughputKbps: number; - uploadKbps: number; - cpuSlowdownMultiplier: number; - label: string; - }; - '4g-fast': { - rttMs: number; - throughputKbps: number; - uploadKbps: number; - cpuSlowdownMultiplier: number; - label: string; - }; - wifi: { - rttMs: number; - throughputKbps: number; - uploadKbps: number; - cpuSlowdownMultiplier: number; - label: string; - }; - none: { - rttMs: number; - throughputKbps: number; - uploadKbps: number; - cpuSlowdownMultiplier: number; - label: string; - }; +/** + * One entry in NETWORK_PRESETS. Named rather than inline so the published declarations + * describe a preset once instead of repeating its structure per key. + */ +export type NetworkPreset = { + /** + * - nominal round-trip time, in milliseconds + */ + rttMs: number; + /** + * - nominal (pre-adjustment) download speed; `buildThrottling` applies the DevTools factors + */ + throughputKbps: number; + /** + * - nominal (pre-adjustment) upload speed; `buildThrottling` applies the DevTools factors + */ + uploadKbps: number; + cpuSlowdownMultiplier: number; + /** + * - the one-line summary printed by `list-profiles` + */ + label: string; }; -export const DEVICE_PRESETS: { - 'moto-g-power': { - width: number; - height: number; - deviceScaleFactor: number; - mobile: boolean; - formFactor: string; - label: string; - }; - 'iphone-12': { - width: number; - height: number; - deviceScaleFactor: number; - mobile: boolean; - formFactor: string; - label: string; - }; - 'iphone-14': { - width: number; - height: number; - deviceScaleFactor: number; - mobile: boolean; - formFactor: string; - label: string; - }; - ipad: { - width: number; - height: number; - deviceScaleFactor: number; - mobile: boolean; - formFactor: string; - label: string; - }; - desktop: { - width: number; - height: number; - deviceScaleFactor: number; - mobile: boolean; - formFactor: string; - label: string; - }; - 'desktop-large': { - width: number; - height: number; - deviceScaleFactor: number; - mobile: boolean; - formFactor: string; - label: string; - }; +/** + * One entry in DEVICE_PRESETS: the screen emulation and form factor for a device. + */ +export type DevicePreset = { + width: number; + height: number; + deviceScaleFactor: number; + mobile: boolean; + formFactor: "mobile" | "desktop"; + /** + * - the one-line summary printed by `list-profiles` + */ + label: string; }; +/** + * One entry in PROFILES: a named pairing of a network preset with a device preset. + */ +export type LabProfile = { + /** + * - a NETWORK_PRESETS key, or null for the native profile + */ + network: string | null; + /** + * - a DEVICE_PRESETS key, or null for the native profile + */ + device: string | null; + /** + * - the one-line summary printed by `list-profiles` + */ + label: string; +}; +/** + * One entry in PROFILES: a named pairing of a network preset with a device preset. + * + * @typedef {Object} LabProfile + * @property {string|null} network - a NETWORK_PRESETS key, or null for the native profile + * @property {string|null} device - a DEVICE_PRESETS key, or null for the native profile + * @property {string} label - the one-line summary printed by `list-profiles` + */ +/** @type {Readonly>>} */ +export const PROFILES: Readonly>>; +/** + * One entry in NETWORK_PRESETS. Named rather than inline so the published declarations + * describe a preset once instead of repeating its structure per key. + * + * @typedef {Object} NetworkPreset + * @property {number} rttMs - nominal round-trip time, in milliseconds + * @property {number} throughputKbps - nominal (pre-adjustment) download speed; `buildThrottling` applies the DevTools factors + * @property {number} uploadKbps - nominal (pre-adjustment) upload speed; `buildThrottling` applies the DevTools factors + * @property {number} cpuSlowdownMultiplier + * @property {string} label - the one-line summary printed by `list-profiles` + */ +/** @type {Readonly>>} */ +export const NETWORK_PRESETS: Readonly>>; +/** + * One entry in DEVICE_PRESETS: the screen emulation and form factor for a device. + * + * @typedef {Object} DevicePreset + * @property {number} width + * @property {number} height + * @property {number} deviceScaleFactor + * @property {boolean} mobile + * @property {'mobile'|'desktop'} formFactor + * @property {string} label - the one-line summary printed by `list-profiles` + */ +/** @type {Readonly>>} */ +export const DEVICE_PRESETS: Readonly>>; export const LAB_CATEGORIES: readonly string[]; export const DEVTOOLS_RTT_ADJUSTMENT_FACTOR: 3.75; export const DEVTOOLS_THROUGHPUT_ADJUSTMENT_FACTOR: 0.9;