From cf0ad6a47b44c0313334de099bcb59c390e31e86 Mon Sep 17 00:00:00 2001 From: Joan Leon Date: Sat, 14 Nov 2026 18:21:35 +0100 Subject: [PATCH 1/2] fix(visualizer): export an audit with a rating and no value as an audit Copy as Markdown treated every result with a rating as a metric, so an audit such as Script-Loading came out as a heading with "undefined" and none of its issues or items, and a result whose unit is a score threw before copying anything. The export picks its format with detectRenderer, as the page picks its view. detectRenderer moves to its own module so that the export can import it. A result the page shows as JSON is exported as JSON instead of "No issues found". The tests export the 114 result fixtures and fail on any "undefined" or "NaN". --- CHANGELOG.md | 3 ++ .../SnippetVisualizer/AuditRenderer.test.jsx | 3 +- .../SnippetVisualizer/detectRenderer.js | 7 ++++ .../SnippetVisualizer/exportMarkdown.js | 13 +++++-- .../SnippetVisualizer/exportMarkdown.test.js | 38 +++++++++++++++++++ components/SnippetVisualizer/index.jsx | 9 +---- 6 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 components/SnippetVisualizer/detectRenderer.js create mode 100644 components/SnippetVisualizer/exportMarkdown.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 855a342f..dad661d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ Format based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed - The prompt that the Copy prompt button of a snippet page copies installs and runs Playwright 1.65, the version the CLI is tested with, which brings Chromium 159. That Chromium does not run `unload` handlers unless the page asks for them with `Permissions-Policy: unload=(self)`, so on a page that does not, Back-Forward-Cache no longer finds an `unload` handler to report. +### Fixed +- Copy as Markdown of the [visualizer](https://webperf-snippets.net/visualizer) exported an audit that has a rating and no value, such as Script-Loading, as a metric: a heading with `undefined` and none of its issues or items, and nothing at all when its unit is a score. The export picks its format as the page picks its view, so the audit comes with its issues and its items, and a result the page shows as JSON is exported as JSON instead of "No issues found". + --- ## [2.6.3] — 2026-11-08 diff --git a/components/SnippetVisualizer/AuditRenderer.test.jsx b/components/SnippetVisualizer/AuditRenderer.test.jsx index 10567a33..2109358a 100644 --- a/components/SnippetVisualizer/AuditRenderer.test.jsx +++ b/components/SnippetVisualizer/AuditRenderer.test.jsx @@ -2,7 +2,8 @@ import { describe, it, expect } from "vitest"; import { readFileSync } from "node:fs"; import { renderToStaticMarkup } from "react-dom/server"; import { AuditRenderer } from "./AuditRenderer"; -import { SnippetVisualizer, detectRenderer } from "./index"; +import { SnippetVisualizer } from "./index"; +import { detectRenderer } from "./detectRenderer"; const fixture = (name) => JSON.parse(readFileSync(new URL(`../../scripts/fixtures/results/seeded-${name}.json`, import.meta.url), "utf8")); diff --git a/components/SnippetVisualizer/detectRenderer.js b/components/SnippetVisualizer/detectRenderer.js new file mode 100644 index 00000000..315a10cc --- /dev/null +++ b/components/SnippetVisualizer/detectRenderer.js @@ -0,0 +1,7 @@ +export function detectRenderer(result) { + if (result.script === "Fonts-Preloaded-Loaded-and-used-above-the-fold") return "fonts"; + // A metric has a value to rate; an audit such as Script-Loading has a rating and no value + if (result.rating != null && typeof result.value === "number") return "cwv"; + if (Array.isArray(result.issues) || Array.isArray(result.items)) return "audit"; + return "raw"; +} diff --git a/components/SnippetVisualizer/exportMarkdown.js b/components/SnippetVisualizer/exportMarkdown.js index ee3d6321..6836668d 100644 --- a/components/SnippetVisualizer/exportMarkdown.js +++ b/components/SnippetVisualizer/exportMarkdown.js @@ -1,3 +1,5 @@ +import { detectRenderer } from "./detectRenderer"; + const RATING_ICON = { good: "✅", "needs-improvement": "⚠️", @@ -100,8 +102,13 @@ function auditMarkdown(result) { return lines.join("\n"); } +function rawMarkdown(result) { + return [`## ${result.script ?? "Result"}`, "", "```json", JSON.stringify(result, null, 2), "```"].join("\n"); +} + +const MARKDOWN = { fonts: fontsMarkdown, cwv: cwvMarkdown, audit: auditMarkdown, raw: rawMarkdown }; + +// The same view the page renders, so the export never reads a field the result does not have export function exportMarkdown(result) { - if (result.script === "Fonts-Preloaded-Loaded-and-used-above-the-fold") return fontsMarkdown(result); - if (result.rating != null) return cwvMarkdown(result); - return auditMarkdown(result); + return MARKDOWN[detectRenderer(result)](result); } diff --git a/components/SnippetVisualizer/exportMarkdown.test.js b/components/SnippetVisualizer/exportMarkdown.test.js new file mode 100644 index 00000000..6a115d33 --- /dev/null +++ b/components/SnippetVisualizer/exportMarkdown.test.js @@ -0,0 +1,38 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync, readdirSync } from "node:fs"; +import { exportMarkdown } from "./exportMarkdown"; + +const dir = new URL("../../scripts/fixtures/results/", import.meta.url); +const read = (file) => JSON.parse(readFileSync(new URL(file, dir), "utf8")); +const fixture = (name) => read(`seeded-${name}.json`); + +describe("exportMarkdown", () => { + it("exports an audit that has a rating and no value as an audit", () => { + const md = exportMarkdown(fixture("Script-Loading")); + expect(md).not.toContain("undefined"); + expect(md).toContain("### Items (3)"); + expect(md).toContain("third.js"); + }); + + it("exports a score that has a rating and no value without failing", () => { + const md = exportMarkdown({ script: "Demo", status: "ok", unit: "score", rating: "good", issues: [] }); + expect(md).toContain("## Demo"); + expect(md).not.toContain("undefined"); + }); + + it("exports a metric with its value and its rating", () => { + // The icon of the rating sits between the value and the rating + expect(exportMarkdown(fixture("CLS"))).toMatch(/^## CLS — 0\.0638 \S+ good$/); + }); + + it("exports a result with neither issues nor items as its JSON, as the page shows it", () => { + const result = { script: "Demo", status: "ok", details: { count: 2 } }; + const md = exportMarkdown(result); + expect(md).toContain("```json\n" + JSON.stringify(result, null, 2) + "\n```"); + expect(md).not.toContain("No issues found"); + }); + + it.each(readdirSync(dir).filter((file) => file.endsWith(".json")))("exports %s with every value defined", (file) => { + expect(exportMarkdown(read(file))).not.toMatch(/\bundefined\b|\bNaN\b/); + }); +}); diff --git a/components/SnippetVisualizer/index.jsx b/components/SnippetVisualizer/index.jsx index 57464c2a..3f0d2193 100644 --- a/components/SnippetVisualizer/index.jsx +++ b/components/SnippetVisualizer/index.jsx @@ -6,6 +6,7 @@ import { FontsRenderer } from "./FontsRenderer"; import { AuditRenderer } from "./AuditRenderer"; import { Icon } from "../Icon"; import { exportMarkdown } from "./exportMarkdown"; +import { detectRenderer } from "./detectRenderer"; const PLACEHOLDER = `Paste the return value of any snippet here. @@ -14,14 +15,6 @@ and choose "Copy object", then paste it here. Or run: JSON.stringify(result) and paste the output.`; -export function detectRenderer(result) { - if (result.script === "Fonts-Preloaded-Loaded-and-used-above-the-fold") return "fonts"; - // A metric has a value to rate; an audit such as Script-Loading has a rating and no value - if (result.rating != null && typeof result.value === "number") return "cwv"; - if (Array.isArray(result.issues) || Array.isArray(result.items)) return "audit"; - return "raw"; -} - function parseInput(text) { if (!text.trim()) return null; try { From ba1e9be1e5c621c96b72deefb34fd9eb6882d7da Mon Sep 17 00:00:00 2001 From: Joan Leon Date: Sat, 14 Nov 2026 18:28:24 +0100 Subject: [PATCH 2/2] fix(extension): open the panel with the defaults when a saved setting is not one it knows The panel looked for the radio of the saved theme and of the saved side, and with a value that has no radio, such as one synced from another version of the extension, the module threw and the panel stayed empty. The settings check each value they read: the theme, the side, the state of the list and the filter of the audit against the values the panel has, and the rest by type. A value that fails gives way to its default. --- CHANGELOG.md | 1 + extension/lib/settings.js | 17 ++++++++++++++++- extension/lib/settings.test.js | 12 ++++++++++++ extension/tests/panel.e2e.test.js | 19 +++++++++++++++++++ 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dad661d9..bb3b7a4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Format based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed - Copy as Markdown of the [visualizer](https://webperf-snippets.net/visualizer) exported an audit that has a rating and no value, such as Script-Loading, as a metric: a heading with `undefined` and none of its issues or items, and nothing at all when its unit is a score. The export picks its format as the page picks its view, so the audit comes with its issues and its items, and a result the page shows as JSON is exported as JSON instead of "No issues found". +- The panel of the extension stayed empty when the storage brought a theme or a side of the list that it does not know, such as a value synced from another version: it looked for the option of that value in the settings and stopped there. A saved value the panel does not know gives way to its default. --- diff --git a/extension/lib/settings.js b/extension/lib/settings.js index bd05ab34..468645bb 100644 --- a/extension/lib/settings.js +++ b/extension/lib/settings.js @@ -9,6 +9,20 @@ export const DEFAULTS = { theme: 'devtools', side: 'left', sidebar: 'expanded', snippet: null, audits: [], reloadBeforeAudit: true, auditFilter: 'issues' } const SYNCED = new Set(['theme', 'side', 'reloadBeforeAudit', 'auditFilter']) +// What each setting may hold. The storage can bring anything: a value of another version of the +// extension, synced from another Chrome, or one written by hand. A value that fails gives way to +// the default, so the panel never looks for a control that does not exist +const oneOf = (...values) => (value) => values.includes(value) +const VALID = { + theme: oneOf('devtools', 'light', 'dark'), + side: oneOf('left', 'right'), + sidebar: oneOf('expanded', 'collapsed'), + snippet: (value) => value === null || typeof value === 'string', + audits: Array.isArray, + reloadBeforeAudit: (value) => typeof value === 'boolean', + auditFilter: oneOf('errors', 'issues', 'all'), +} + export function createSettings(storage) { const areaOf = (key) => (SYNCED.has(key) ? storage?.sync : storage?.local) const read = async (area, keys) => { @@ -25,7 +39,8 @@ export function createSettings(storage) { read(storage?.sync, keys.filter((k) => SYNCED.has(k))), read(storage?.local, keys.filter((k) => !SYNCED.has(k))), ]) - return { ...DEFAULTS, ...synced, ...local } + const stored = { ...synced, ...local } + return Object.fromEntries(keys.map((key) => [key, key in stored && VALID[key](stored[key]) ? stored[key] : DEFAULTS[key]])) }, async save(key, value) { try { diff --git a/extension/lib/settings.test.js b/extension/lib/settings.test.js index 0f245f43..0af96197 100644 --- a/extension/lib/settings.test.js +++ b/extension/lib/settings.test.js @@ -49,6 +49,18 @@ test('a value saved in the old place of the setting is ignored', async () => { assert.deepEqual(await settings.load(), DEFAULTS) }) +test('a saved value the panel does not know gives way to the default', async () => { + const sync = area({ theme: 'sepia', side: 'top', auditFilter: 'none', reloadBeforeAudit: 'yes' }) + const local = area({ sidebar: 'floating', snippet: 42, audits: 'none' }) + assert.deepEqual(await createSettings({ sync, local }).load(), DEFAULTS) +}) + +test('each value the panel knows is kept', async () => { + const sync = area({ theme: 'light', side: 'right', auditFilter: 'errors', reloadBeforeAudit: false }) + const local = area({ sidebar: 'collapsed', snippet: 'LCP', audits: [] }) + assert.deepEqual(await createSettings({ sync, local }).load(), { ...sync.data, ...local.data }) +}) + test('without chrome.storage, or when it fails, the panel keeps the defaults and saving does nothing', async () => { assert.deepEqual(await createSettings(undefined).load(), DEFAULTS) await createSettings(undefined).save('theme', 'dark') diff --git a/extension/tests/panel.e2e.test.js b/extension/tests/panel.e2e.test.js index 9f1e8680..a245255c 100644 --- a/extension/tests/panel.e2e.test.js +++ b/extension/tests/panel.e2e.test.js @@ -302,6 +302,25 @@ test('the settings choose the theme and the side of the list, and the panel reme assert.equal(await side(), 'left') }) +test('the panel opens with its defaults when the storage brings a theme or a side it does not know', async () => { + await panel.evaluate(() => { + localStorage.setItem('sync:theme', JSON.stringify('sepia')) + localStorage.setItem('sync:side', JSON.stringify('top')) + }) + try { + await panel.reload() + await panel.waitForSelector('#list button', { timeout: 5000 }) + await panel.click('#open-settings') + assert.ok(await panel.isChecked('input[name="theme"][value="devtools"]')) + assert.ok(await panel.isChecked('input[name="side"][value="left"]')) + await panel.click('#settings .close') + } finally { + // The page error of the panel that did not open is what the test reports, not the ones after it + await panel.evaluate(() => ['sync:theme', 'sync:side'].forEach((key) => localStorage.removeItem(key))) + await reopenPanel() + } +}) + test('Clear empties the result, and only shows when there is one', async () => { await target.goto(`${servers.base}/seeded`, { waitUntil: 'load' }) await panel.click('#list button[data-id="Input-Latency-Breakdown"]')