From 196cc4a8cae7a7a3a18c2345ead0193c8d90f04c Mon Sep 17 00:00:00 2001 From: Ivo Date: Thu, 15 Oct 2026 11:20:48 +0200 Subject: [PATCH 01/10] feat(convert-core): let a converter declare differences it makes on purpose Some content changes are known and accepted, such as the editor's Social block writing an icon's alt text as "Facebook icon", or Beefree's phone hamburger menu having no equivalent. Without a way to say so, the content check reported them as a converter bug. applyDeclared(originalHtml, declarations) applies only the named substitutions (an alt or src on an exact image, a glyph inside a named class, an element shown or hidden) to the reference before the unchanged content check runs, and describeDeclared lists each one as a difference. Anything undeclared still fails; --strict still fails declared ones. Co-Authored-By: Claude Code (Claude Opus 5.5 xhigh) --- packages/convert-core/src/declared.ts | 111 ++++++++++++++++++++ packages/convert-core/src/index.ts | 1 + packages/convert-core/test/declared.test.ts | 83 +++++++++++++++ 3 files changed, 195 insertions(+) create mode 100644 packages/convert-core/src/declared.ts create mode 100644 packages/convert-core/test/declared.test.ts diff --git a/packages/convert-core/src/declared.ts b/packages/convert-core/src/declared.ts new file mode 100644 index 00000000..b6a31df2 --- /dev/null +++ b/packages/convert-core/src/declared.ts @@ -0,0 +1,111 @@ +/** + * Declared differences: content a converter knows it changes, said up front so the check expects it instead of + * failing on it. Each declaration names exactly what it replaces (this image's alt text, this glyph in this element), + * so it can't hide anything else: the original's HTML gets the declared changes, then the check compares as usual, and + * every change made is reported. A declaration that matches nothing changes nothing. + */ + +import { parse, type DefaultTreeAdapterMap } from "parse5"; + +type Node = DefaultTreeAdapterMap["node"]; +type Element = DefaultTreeAdapterMap["element"]; +type TextNode = DefaultTreeAdapterMap["textNode"]; + +export type DeclaredDifference = + /** On images with exactly this `src` and exactly this alt text (`from`, "" for none), the migration writes `to`. */ + | { kind: "alt"; src: string; from: string; to: string; reason: string } + /** The migration shows this image (`to`) where the original shows `from`, exactly: another picture of the same thing. */ + | { kind: "src"; from: string; to: string; reason: string } + /** The migration leaves out text that is exactly `text` (trimmed) inside an element with the class `parentClass`. */ + | { kind: "text"; text: string; parentClass: string; reason: string } + /** + * The migration shows, at every width, the elements with the class `className` that the original hides at some + * (a menu folded into a hamburger button on phones): they're shown in the original too (`display: block`). + */ + | { kind: "shown"; className: string; reason: string } + /** The migration leaves out the elements with the class `className` (a hamburger button): hidden in the original too. */ + | { kind: "hidden"; className: string; reason: string }; + +export interface AppliedDifference { + declaration: DeclaredDifference; + /** How many places in the original it changed. */ + count: number; +} + +const attribute = (element: Element, name: string): string | undefined => element.attrs.find((a) => a.name === name)?.value; +const escapeAttribute = (value: string) => value.replace(/&/g, "&").replace(/"/g, """).replace(/ (element && "attrs" in element ? attribute(element, "class") ?? "" : "").split(/\s+/).filter(Boolean); + +/** + * The original's HTML as the migration is expected to show it, with what each declaration changed. Only the attribute + * values and text the declarations name are rewritten; the rest of the document is kept byte for byte. + */ +export function applyDeclared(html: string, declared: readonly DeclaredDifference[]): { html: string; applied: AppliedDifference[] } { + if (!declared.length) return { html, applied: [] }; + const document = parse(html, { sourceCodeLocationInfo: true }); + const edits: Array<{ start: number; end: number; text: string }> = []; + const counts = declared.map(() => 0); + + const replaceAttribute = (element: Element, name: string, value: string) => { + const location = element.sourceCodeLocation; + const at = location?.attrs?.[name]; + if (at) edits.push({ start: at.startOffset, end: at.endOffset, text: `${name}="${escapeAttribute(value)}"` }); + else if (location?.startTag) { + // ` { + if ("tagName" in node && node.tagName === "img") { + const src = attribute(node, "src") ?? ""; + const alt = attribute(node, "alt") ?? ""; + // One declaration per attribute of an image: the first that names it. + const altAt = declared.findIndex((d) => d.kind === "alt" && d.src === src && d.from === alt); + if (altAt >= 0 && replaceAttribute(node, "alt", (declared[altAt] as { to: string }).to)) counts[altAt]++; + const srcAt = declared.findIndex((d) => d.kind === "src" && d.from === src); + if (srcAt >= 0 && replaceAttribute(node, "src", (declared[srcAt] as { to: string }).to)) counts[srcAt]++; + } + if ("tagName" in node) { + const at = declared.findIndex((d) => (d.kind === "shown" || d.kind === "hidden") && classes(node).includes(d.className)); + if (at >= 0) { + const style = attribute(node, "style"); + const display = `${style ? `${style.replace(/;?\s*$/, ";")}` : ""}display:${declared[at].kind === "shown" ? "block" : "none"} !important`; + if (replaceAttribute(node, "style", display)) counts[at]++; + } + } + if (node.nodeName === "#text") { + const text = node as TextNode; + const at = declared.findIndex((d) => d.kind === "text" && text.value.trim() === d.text && classes(text.parentNode as Element).includes(d.parentClass)); + const location = text.sourceCodeLocation; + if (at >= 0 && location) { + edits.push({ start: location.startOffset, end: location.endOffset, text: "" }); + counts[at]++; + } + } + for (const child of "childNodes" in node ? node.childNodes : []) visit(child); + if ("content" in node && node.content) visit(node.content as unknown as Node); + }; + visit(document as unknown as Node); + + let out = html; + let last = Infinity; + for (const edit of edits.sort((a, b) => b.start - a.start)) { + if (edit.end > last) continue; // overlapping: the first one kept + out = out.slice(0, edit.start) + edit.text + out.slice(edit.end); + last = edit.start; + } + return { html: out, applied: declared.map((declaration, i) => ({ declaration, count: counts[i] })).filter((a) => a.count > 0) }; +} + +/** A declared difference as a report lists it: its reason, and what it changed. */ +export function describeDeclared(applied: AppliedDifference): { reason: string; detail: string } { + const { declaration: d, count } = applied; + const times = count > 1 ? ` (×${count})` : ""; + if (d.kind === "alt") return { reason: d.reason, detail: `alt "${d.from}" → "${d.to}"${times}` }; + if (d.kind === "src") return { reason: d.reason, detail: `${d.from} → ${d.to}${times}` }; + if (d.kind === "shown" || d.kind === "hidden") return { reason: d.reason, detail: `.${d.className}${times}` }; + return { reason: d.reason, detail: `"${d.text}"${times}` }; +} diff --git a/packages/convert-core/src/index.ts b/packages/convert-core/src/index.ts index cf920a75..95d38bb3 100644 --- a/packages/convert-core/src/index.ts +++ b/packages/convert-core/src/index.ts @@ -24,3 +24,4 @@ export { compareStyles, DESKTOP_WIDTH, normalizeWord, StyledDocument, type Style export { decodeHtmlEntities } from "./entities"; export { autoTableWidths, htmlText, unbreakableWords, wordWidth, type TableColumn } from "./text-width"; export { editorFonts, shareEditorFonts, type EditorFont } from "./fonts"; +export { applyDeclared, describeDeclared, type AppliedDifference, type DeclaredDifference } from "./declared"; diff --git a/packages/convert-core/test/declared.test.ts b/packages/convert-core/test/declared.test.ts new file mode 100644 index 00000000..71458f69 --- /dev/null +++ b/packages/convert-core/test/declared.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "vitest"; +import { applyDeclared, describeDeclared, type DeclaredDifference } from "../src/declared"; +import { compareText } from "../src/verify"; + +const ICON = "https://cdn.example.com/icons/facebook.png"; +const ALT: DeclaredDifference = { kind: "alt", src: ICON, from: "Facebook", to: "Facebook icon", reason: "social icon alt text is the editor's" }; +const GLYPH: DeclaredDifference = { kind: "text", text: "☰", parentClass: "menu-open", reason: "the menu toggle's glyph is left out" }; + +const page = (body: string) => `t${body}`; +const icon = (alt: string, src = ICON) => `

Follow us

${alt}`; + +describe("declared differences", () => { + it("make the original show what the migration is expected to: the check passes, and each change is reported", () => { + const original = page(icon("Facebook")); + const migrated = page(icon("Facebook icon")); + expect(compareText(original, migrated).missingAttributes.length).toBeGreaterThan(0); + const expected = applyDeclared(original, [ALT]); + const check = compareText(expected.html, migrated); + expect([check.missing, check.added, check.missingAttributes, check.addedAttributes]).toEqual([[], [], [], []]); + expect(expected.applied.map(describeDeclared)).toEqual([{ reason: "social icon alt text is the editor's", detail: 'alt "Facebook" → "Facebook icon"' }]); + }); + + it("change only what they name: another alt change, another image, or other text still fails as content", () => { + // The alt text the migration writes isn't the declared one. + const wrongAlt = compareText(applyDeclared(page(icon("Facebook")), [ALT]).html, page(icon("Facebook logo"))); + expect(wrongAlt.missingAttributes).toContain("alt Facebook icon"); + expect(wrongAlt.addedAttributes).toContain("alt Facebook logo"); + // The original's alt isn't the one declared: nothing changes, and the difference stays. + const otherFrom = applyDeclared(page(icon("FB")), [ALT]); + expect(otherFrom.applied).toEqual([]); + expect(compareText(otherFrom.html, page(icon("Facebook icon"))).missingAttributes).toContain("alt FB"); + // Another image with the same alt text isn't touched. + const otherImage = applyDeclared(page(icon("Facebook", "https://cdn.example.com/photo.png")), [ALT]); + expect(otherImage.applied).toEqual([]); + // A glyph outside the element it's declared in is content. + const loose = applyDeclared(page('

Menu ☰

☰'), [GLYPH]); + expect(loose.applied).toEqual([{ declaration: GLYPH, count: 1 }]); + expect(compareText(loose.html, page("

Menu

")).missing).toEqual(["☰"]); + }); + + it("leave out a declared glyph, and keep the rest of the document byte for byte", () => { + const original = page('
Shop
'); + const { html, applied } = applyDeclared(original, [GLYPH]); + expect(html).toBe(original.replace("☰", "")); + expect(applied).toHaveLength(1); + expect(compareText(html, page('
Shop
')).missing).toEqual([]); + }); + + it("swap a declared image source, and add an alt text the original's image lacks", () => { + const swapped = applyDeclared(page(''), [ + { kind: "src", from: "/mjml/facebook.png", to: "/unlayer/facebook.png", reason: "r" }, + { kind: "alt", src: "/mjml/facebook.png", from: "", to: "Facebook icon", reason: "r" }, + ]); + expect(swapped.html).toBe(page('Facebook icon')); + expect(swapped.applied.map((a) => a.count)).toEqual([1, 1]); + }); + + it("show elements of a declared class the original hides at some widths (a menu folded into a hamburger)", () => { + const css = ""; + const original = `${css}
Shop
`; + const migrated = page('
Shop
'); + expect(compareText(original, migrated).phone.added).toEqual(["Shop"]); + const { html, applied } = applyDeclared(original, [{ kind: "shown", className: "menu-links", reason: "r" }]); + expect(html).toContain('
'); + expect(applied).toHaveLength(1); + expect(compareText(html, migrated).phone.added).toEqual([]); + }); + + it("hide elements of a declared class (a hamburger button and its icon)", () => { + const original = page('Shop'); + const migrated = page('Shop'); + expect(compareText(original, migrated).missingAttributes.length).toBeGreaterThan(0); + const { html, applied } = applyDeclared(original, [{ kind: "hidden", className: "menu-trigger", reason: "r" }]); + expect(html).toContain('