diff --git a/packages/lint/src/rules/fonts.test.ts b/packages/lint/src/rules/fonts.test.ts index 4e8c808c7f..cf50bce853 100644 --- a/packages/lint/src/rules/fonts.test.ts +++ b/packages/lint/src/rules/fonts.test.ts @@ -389,14 +389,106 @@ describe("font rules", () => { expect(findings).toHaveLength(0); }); - it("does not flag a var() with a quoted fallback font", async () => { + it("flags a concrete undeclared var() fallback without a close-paren artifact", async () => { const html = `
`; const findings = await findByCode(html, "font_family_without_font_face"); + expect(findings).toHaveLength(1); + expect(findings[0]!.message).toContain("geist"); + expect(findings[0]!.message).not.toContain("geist')"); + }); + + it("does not manufacture production close-paren family artifacts from var() fallbacks", async () => { + const html = `
+ +
`; + const findings = await findByCode(html, "font_family_without_font_face"); + expect(findings).toHaveLength(1); + expect(findings[0]!.message).toContain("montserrat bold"); + expect(findings[0]!.message).not.toContain(`montserrat bold")`); + expect(findings[0]!.message).not.toContain("serif)"); + }); + + it("does not guess scoped or cyclic custom-property values", async () => { + const html = `
+ +
`; + const findings = await findByCode(html, "font_family_without_font_face"); expect(findings).toHaveLength(0); }); + it("ignores CSS-wide keywords explicitly", async () => { + const html = `
+ +
`; + const findings = await findByCode(html, "font_family_without_font_face"); + expect(findings).toHaveLength(0); + }); + + it("treats quoted generic and CSS-wide names as real families through root resolution", async () => { + const html = `
+ +
`; + const findings = await findByCode(html, "font_family_without_font_face"); + expect(findings).toHaveLength(1); + expect(findings[0]!.message).toContain("system-ui"); + expect(findings[0]!.message).toContain("inherit"); + expect(findings[0]!.message).toContain("serif"); + }); + + it("keeps quoted commas and no-space function-shaped names as families", async () => { + const html = `
+ +
`; + const findings = await findByCode(html, "font_family_without_font_face"); + expect(findings).toHaveLength(1); + expect(findings[0]!.message).toContain("acme, inc"); + expect(findings[0]!.message).toContain("acme(display)"); + expect(findings[0]!.message).toContain("var(--display)"); + expect(findings[0]!.message).not.toContain("var(--runtime)"); + expect(findings[0]!.message).not.toContain("env(font-name)"); + }); + + it("malformed CSS invalidates only matching static aliases", async () => { + const differentName = `
+ + + +
`; + const retainedFindings = await findByCode(differentName, "font_family_without_font_face"); + expect(retainedFindings).toHaveLength(1); + expect(retainedFindings[0]!.message).toContain("geist"); + + const sameName = `
+ + + +
`; + const unresolvedFindings = await findByCode(sameName, "font_family_without_font_face"); + expect(unresolvedFindings).toHaveLength(0); + }); + it("still flags a real undeclared font sitting next to a system stack", async () => { const html = `
diff --git a/packages/lint/src/rules/fonts.ts b/packages/lint/src/rules/fonts.ts index d9fdefa548..2f93bf9e30 100644 --- a/packages/lint/src/rules/fonts.ts +++ b/packages/lint/src/rules/fonts.ts @@ -1,8 +1,14 @@ import { + collectStaticRootFontFamilyCustomProperties, FONT_ALIAS_KEYS, GOOGLE_FONT_FAMILY_ALIAS_KEYS, + isCssWideFontFamilyKeyword, resolveAliasDisplayName, + resolveFontFamilyDeclarationCandidates, + type FontFamilyCustomPropertyDeclaration, + type FontFamilyValueToken, } from "@hyperframes/parsers/composition"; +import postcss, { type AtRule, type Declaration, type Rule, type Root } from "postcss"; import type { LintContext, HyperframeLintFinding } from "../context"; import { isRegistrySourceFile, isRegistryInstalledFile } from "./composition"; @@ -26,10 +32,6 @@ const GENERIC_FAMILIES = new Set([ // (e.g. `-apple-system, system-ui, sans-serif`). "-apple-system", "blinkmacsystemfont", - "inherit", - "initial", - "unset", - "revert", ]); // A CSS comment can contain a `}` (e.g. `@font-face { /* 400 } regular */ @@ -60,38 +62,103 @@ function extractFontFaceFamilies(styles: Array<{ content: string }>): Set, + parsedRoots: Array, +): Map { + const declarations: FontFamilyCustomPropertyDeclaration[] = []; + for (let index = 0; index < styles.length; index += 1) { + const root = parsedRoots[index]; + if (!root) { + for (const match of styles[index]!.content.matchAll(/(--[A-Za-z0-9_-]+)\s*:/g)) { + declarations.push({ + name: match[1]!, + value: "", + staticRoot: false, + }); + } + continue; + } + root.walkDecls((decl) => { + if (!decl.prop.startsWith("--")) return; + const parent = decl.parent; + declarations.push({ + name: decl.prop, + value: decl.value, + staticRoot: + parent?.type === "rule" && + (parent as Rule).selector.trim() === ":root" && + parent.parent?.type === "root", + important: decl.important, + }); + }); + } + return collectStaticRootFontFamilyCustomProperties(declarations); +} + function extractUsedFontFamilies(styles: Array<{ content: string }>): string[] { const used: string[] = []; const seen = new Set(); + const parsedRoots = styles.map((style) => parseCssRoot(style.content)); + const roots = parsedRoots.filter((root) => root !== null); + const customProperties = collectLintCustomProperties(styles, parsedRoots); + + const addDeclaration = (declaration: string) => { + for (const candidate of resolveFontFamilyDeclarationCandidates(declaration, customProperties)) { + const name = normalizeUsedFontName(candidate); + if (name && !seen.has(name)) { + seen.add(name); + used.push(name); + } + } + }; + + for (const root of roots) { + root.walkDecls((decl) => { + if (decl.prop.toLowerCase() !== "font-family" || isFontFaceDeclaration(decl)) return; + addDeclaration(decl.value); + }); + } + + // Preserve best-effort lint coverage for malformed stylesheets PostCSS + // cannot parse. The shared value parser still prevents function commas from + // manufacturing stray `)` family names. const propRe = /font-family\s*:\s*([^;}{]+)/gi; for (const style of styles) { + if (parseCssRoot(style.content)) continue; const withoutFontFace = stripCssComments(style.content).replace(/@font-face\s*\{[^}]*\}/gi, ""); let match: RegExpExecArray | null; while ((match = propRe.exec(withoutFontFace)) !== null) { - for (const part of match[1]!.split(",")) { - const name = normalizeUsedFontName(part); - if (name && !GENERIC_FAMILIES.has(name) && !seen.has(name)) { - seen.add(name); - used.push(name); - } - } + addDeclaration(match[1]!); } } return used; diff --git a/packages/parsers/src/composition.ts b/packages/parsers/src/composition.ts index 1d9c4c48e9..4b5267d6c3 100644 --- a/packages/parsers/src/composition.ts +++ b/packages/parsers/src/composition.ts @@ -11,6 +11,17 @@ export { CANONICAL_FONT_DISPLAY_NAMES, resolveAliasDisplayName, } from "./fontAliases.js"; +export { + CSS_WIDE_FONT_FAMILY_KEYWORDS, + collectStaticRootFontFamilyCustomProperties, + isCssWideFontFamilyKeyword, + parseFontFamilyValue, + parseFontFamilyValueTokens, + resolveFontFamilyDeclarationCandidates, + resolveFontFamilyDeclarationFamilies, + type FontFamilyCustomPropertyDeclaration, + type FontFamilyValueToken, +} from "./fontFamilyValue.js"; export { decodeUrlPathVariants } from "./utils/urlPath.js"; export { parseCompositionVariables, diff --git a/packages/parsers/src/fontFamilyValue.test.ts b/packages/parsers/src/fontFamilyValue.test.ts new file mode 100644 index 0000000000..a52fd9eb97 --- /dev/null +++ b/packages/parsers/src/fontFamilyValue.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it } from "vitest"; +import { + collectStaticRootFontFamilyCustomProperties, + isCssWideFontFamilyKeyword, + parseFontFamilyValue, + parseFontFamilyValueTokens, + resolveFontFamilyDeclarationCandidates, + resolveFontFamilyDeclarationFamilies, +} from "./fontFamilyValue"; + +describe("parseFontFamilyValue", () => { + it("splits only top-level commas", () => { + expect(parseFontFamilyValue(`var(--font, "Montserrat Bold"), serif`)).toEqual([ + `var(--font, "Montserrat Bold")`, + "serif", + ]); + }); + + it("does not manufacture the production var() close-paren artifacts", () => { + const families = parseFontFamilyValue(`var(--font, "Montserrat Bold"), var(--body, serif)`); + expect(families).not.toContain(`Montserrat Bold")`); + expect(families).not.toContain("serif)"); + expect(parseFontFamilyValue(`Montserrat Bold")`)).toEqual([]); + expect(parseFontFamilyValue("serif)")).toEqual([]); + }); + + it("preserves commas and escaped quotes inside family names", () => { + expect( + parseFontFamilyValue(`"ACME, Inc", "ACME (Display)", "Display \\"Quoted\\"", serif`), + ).toEqual(["ACME, Inc", "ACME (Display)", `Display \\"Quoted\\"`, "serif"]); + expect(parseFontFamilyValue(`ACME\\, Inc, serif`)).toEqual(["ACME\\, Inc", "serif"]); + }); + + it("preserves quote provenance for no-space function-shaped family names", () => { + expect( + parseFontFamilyValueTokens( + `"ACME(Display)", "var(--Display)", var(--runtime), env(font-name)`, + ), + ).toEqual([ + { value: "ACME(Display)", kind: "family", quoted: true }, + { value: "var(--Display)", kind: "family", quoted: true }, + { value: "var(--runtime)", kind: "function", quoted: false }, + { value: "env(font-name)", kind: "function", quoted: false }, + ]); + }); + + it("ignores comments while respecting comment markers inside strings", () => { + expect(parseFontFamilyValue(`"Inter" /* ignored, ) */, "/* Display */", serif`)).toEqual([ + "Inter", + "/* Display */", + "serif", + ]); + expect(parseFontFamilyValue(`"Inter", serif /* unterminated`)).toEqual([]); + }); + + it("recognizes every supported CSS-wide keyword", () => { + for (const keyword of ["inherit", "initial", "revert", "revert-layer", "unset"]) { + expect(isCssWideFontFamilyKeyword(keyword)).toBe(true); + expect(isCssWideFontFamilyKeyword(keyword.toUpperCase())).toBe(true); + } + }); +}); + +describe("resolveFontFamilyDeclarationFamilies", () => { + it("retains quoted token kind through resolution candidates", () => { + expect( + resolveFontFamilyDeclarationCandidates( + `var(--heading), "var(--Display)", env(font-name)`, + new Map([["--heading", `"ACME(Display)"`]]), + ), + ).toEqual([ + { value: "ACME(Display)", kind: "family", quoted: true }, + { value: "var(--Display)", kind: "family", quoted: true }, + { value: "env(font-name)", kind: "function", quoted: false }, + ]); + }); + + it("resolves an unambiguous root variable and its nested fallback", () => { + const properties = new Map([["--heading", `var(--brand, "Montserrat")`]]); + expect(resolveFontFamilyDeclarationFamilies(`var(--heading), serif`, properties)).toEqual([ + "Montserrat", + "serif", + ]); + }); + + it("keeps an unknown variable primary but exposes its explicit nested fallback candidates", () => { + expect( + resolveFontFamilyDeclarationFamilies( + `var(--heading, var(--brand, "Montserrat Bold")), serif`, + new Map(), + ), + ).toEqual([ + `var(--heading, var(--brand, "Montserrat Bold"))`, + `var(--brand, "Montserrat Bold")`, + "Montserrat Bold", + "serif", + ]); + }); + + it("does not guess a cyclic variable", () => { + const properties = new Map([ + ["--heading", "var(--body)"], + ["--body", `var(--heading, "Inter")`], + ]); + const resolved = resolveFontFamilyDeclarationFamilies(`var(--heading), sans-serif`, properties); + expect(resolved[0]).toBe("var(--heading)"); + expect(resolved).toContain("Inter"); + expect(resolved).toContain("sans-serif"); + }); +}); + +describe("collectStaticRootFontFamilyCustomProperties", () => { + it("retains source-order root declarations with !important precedence", () => { + const properties = collectStaticRootFontFamilyCustomProperties([ + { name: "--font", value: "Inter", staticRoot: true, important: true }, + { name: "--font", value: "Montserrat", staticRoot: true }, + { name: "--font", value: "Outfit", staticRoot: true, important: true }, + ]); + expect(properties.get("--font")).toBe("Outfit"); + }); + + it("drops a root property when any scoped or dynamic declaration can override it", () => { + const properties = collectStaticRootFontFamilyCustomProperties([ + { name: "--font", value: "Inter", staticRoot: true }, + { name: "--font", value: "system-ui", staticRoot: false }, + ]); + expect(properties.has("--font")).toBe(false); + }); +}); diff --git a/packages/parsers/src/fontFamilyValue.ts b/packages/parsers/src/fontFamilyValue.ts new file mode 100644 index 0000000000..33108d1c1c --- /dev/null +++ b/packages/parsers/src/fontFamilyValue.ts @@ -0,0 +1,416 @@ +/** + * CSS-wide keywords are values, not installable font family names. Keep this + * list shared so linting, deterministic injection, and plan validation make + * the same decision about them. + */ +export const CSS_WIDE_FONT_FAMILY_KEYWORDS: ReadonlySet = new Set([ + "inherit", + "initial", + "revert", + "revert-layer", + "unset", +]); + +export type FontFamilyCustomPropertyDeclaration = { + name: string; + value: string; + /** True only for an unconditional, top-level rule whose selector is exactly `:root`. */ + staticRoot: boolean; + important?: boolean; +}; + +type StaticRootCandidate = { + value: string; + important: boolean; +}; + +/** + * Reduce CSS custom-property declarations to values that are safe to resolve + * without selector matching or runtime style computation. + * + * A property is intentionally omitted when it has any scoped/dynamic + * declaration. Exact top-level `:root` declarations retain normal source-order + * and `!important` precedence. + */ +export function collectStaticRootFontFamilyCustomProperties( + declarations: Iterable, +): Map { + const candidates = new Map(); + const ambiguous = new Set(); + + for (const declaration of declarations) { + if (!/^--[A-Za-z0-9_-]+$/.test(declaration.name)) continue; + + if (!declaration.staticRoot) { + ambiguous.add(declaration.name); + candidates.delete(declaration.name); + continue; + } + if (ambiguous.has(declaration.name)) continue; + + const existing = candidates.get(declaration.name); + const important = declaration.important === true; + if (existing?.important && !important) continue; + candidates.set(declaration.name, { value: declaration.value, important }); + } + + return new Map(Array.from(candidates, ([name, candidate]) => [name, candidate.value])); +} + +// The state machine is intentionally explicit so comments inside quoted family +// names and escaped delimiters cannot be confused with CSS syntax. +// fallow-ignore-next-line complexity +function stripCssCommentsOutsideStrings(value: string): string | null { + let result = ""; + let quote: "'" | '"' | null = null; + let escaped = false; + + for (let index = 0; index < value.length; index += 1) { + const char = value[index]!; + if (escaped) { + result += char; + escaped = false; + continue; + } + if (char === "\\") { + result += char; + escaped = true; + continue; + } + if (quote) { + result += char; + if (char === quote) quote = null; + continue; + } + if (char === "'" || char === '"') { + quote = char; + result += char; + continue; + } + if (char === "/" && value[index + 1] === "*") { + const end = value.indexOf("*/", index + 2); + if (end === -1) return null; + result += " "; + index = end + 1; + continue; + } + result += char; + } + + return result; +} + +type SplitResult = { + parts: string[]; + valid: boolean; +}; + +export type FontFamilyValueToken = { + value: string; + kind: "family" | "function"; + quoted: boolean; +}; + +// Quote, escape, and parenthesis states are all required to avoid splitting +// commas inside var() fallbacks or quoted family names. +// fallow-ignore-next-line complexity +function splitTopLevelCommas(value: string): SplitResult { + const parts: string[] = []; + let start = 0; + let depth = 0; + let quote: "'" | '"' | null = null; + let escaped = false; + let valid = true; + + for (let index = 0; index < value.length; index += 1) { + const char = value[index]!; + if (escaped) { + escaped = false; + continue; + } + if (char === "\\") { + escaped = true; + continue; + } + if (quote) { + if (char === quote) quote = null; + continue; + } + if (char === "'" || char === '"') { + quote = char; + continue; + } + if (char === "(") { + depth += 1; + continue; + } + if (char === ")") { + if (depth === 0) { + valid = false; + } else { + depth -= 1; + } + continue; + } + if (char === "," && depth === 0) { + parts.push(value.slice(start, index)); + start = index + 1; + } + } + + parts.push(value.slice(start)); + return { + parts, + valid: valid && depth === 0 && quote === null && !escaped, + }; +} + +function hasEscapedTerminalQuote(value: string): boolean { + let backslashes = 0; + for (let index = value.length - 2; index >= 0 && value[index] === "\\"; index -= 1) { + backslashes += 1; + } + return backslashes % 2 === 1; +} + +function normalizeFontFamilyPart(part: string): FontFamilyValueToken | null { + let normalized = part + .trim() + .replace(/\s*!important\s*$/i, "") + .trim(); + if (!normalized) return null; + + const first = normalized[0]; + let quoted = false; + if (first === "'" || first === '"') { + if (normalized.at(-1) !== first || hasEscapedTerminalQuote(normalized)) return null; + normalized = normalized.slice(1, -1).trim(); + quoted = true; + } + + if (!normalized) return null; + return { + value: normalized, + kind: !quoted && /^-?[_a-zA-Z][-_a-zA-Z0-9]*\(/.test(normalized) ? "function" : "family", + quoted, + }; +} + +/** + * Parse a CSS `font-family` value into declaration-order entries. + * + * Commas inside quotes and functions are not separators. Backslash escapes + * protect the following character. Syntactically incomplete values are + * ignored instead of manufacturing names such as `Montserrat Bold")` or + * `serif)` from a `var()` fallback. + */ +export function parseFontFamilyValueTokens(value: string): FontFamilyValueToken[] { + const withoutComments = stripCssCommentsOutsideStrings(value); + if (withoutComments === null) return []; + const split = splitTopLevelCommas(withoutComments); + if (!split.valid) return []; + return split.parts + .map(normalizeFontFamilyPart) + .filter((part): part is FontFamilyValueToken => part !== null); +} + +export function parseFontFamilyValue(value: string): string[] { + return parseFontFamilyValueTokens(value).map((token) => token.value); +} + +export function isCssWideFontFamilyKeyword(value: string): boolean { + return CSS_WIDE_FONT_FAMILY_KEYWORDS.has(value.trim().toLowerCase()); +} + +type VarExpression = { + name: string; + fallback: string | null; +}; + +function parseVarExpression(token: FontFamilyValueToken): VarExpression | null { + if (token.kind !== "function") return null; + const trimmed = token.value.trim(); + if (!/^var\(/i.test(trimmed) || trimmed.at(-1) !== ")") return null; + + const inner = trimmed.slice(trimmed.indexOf("(") + 1, -1); + const split = splitTopLevelCommas(inner); + if (!split.valid) return null; + + const name = split.parts[0]?.trim() ?? ""; + if (!/^--[A-Za-z0-9_-]+$/.test(name)) return null; + const fallback = split.parts.length > 1 ? split.parts.slice(1).join(",").trim() || null : null; + return { name, fallback }; +} + +function appendUnique( + target: FontFamilyValueToken[], + values: Iterable, +): void { + for (const value of values) { + if ( + !target.some( + (existing) => + existing.value === value.value && + existing.kind === value.kind && + existing.quoted === value.quoted, + ) + ) { + target.push(value); + } + } +} + +// Recursive var() resolution keeps fallback candidates and cycle state +// explicit so no scoped or dynamic value is guessed. +// fallow-ignore-next-line complexity +function resolveStaticValue( + value: string, + customProperties: ReadonlyMap, + resolving: ReadonlySet, +): FontFamilyValueToken[] { + const resolved: FontFamilyValueToken[] = []; + for (const family of parseFontFamilyValueTokens(value)) { + const expression = parseVarExpression(family); + if (!expression) { + appendUnique(resolved, [family]); + continue; + } + + if (resolving.has(expression.name)) { + appendUnique(resolved, [family]); + if (expression.fallback) { + appendUnique( + resolved, + resolveFallbackCandidates(expression.fallback, customProperties, resolving), + ); + } + continue; + } + + const definition = customProperties.get(expression.name); + if (definition !== undefined) { + const nextResolving = new Set(resolving); + nextResolving.add(expression.name); + const definitionFamilies = resolveStaticValue(definition, customProperties, nextResolving); + if (definitionFamilies[0]?.kind === "family") { + appendUnique(resolved, definitionFamilies); + continue; + } + appendUnique(resolved, [family]); + appendUnique(resolved, definitionFamilies); + if (expression.fallback) { + appendUnique( + resolved, + resolveFallbackCandidates(expression.fallback, customProperties, resolving), + ); + } + continue; + } + + if (expression.fallback) { + const fallbackFamilies = resolveStaticValue(expression.fallback, customProperties, resolving); + if (fallbackFamilies[0]?.kind === "family") { + appendUnique(resolved, fallbackFamilies); + continue; + } + } + + appendUnique(resolved, [family]); + if (expression.fallback) { + appendUnique( + resolved, + resolveFallbackCandidates(expression.fallback, customProperties, resolving), + ); + } + } + return resolved; +} + +function resolveFallbackCandidates( + fallback: string, + customProperties: ReadonlyMap, + resolving: ReadonlySet, +): FontFamilyValueToken[] { + const candidates: FontFamilyValueToken[] = []; + for (const family of parseFontFamilyValueTokens(fallback)) { + const expression = parseVarExpression(family); + if (!expression) { + appendUnique(candidates, [family]); + continue; + } + + appendUnique(candidates, [family]); + if (!resolving.has(expression.name)) { + const definition = customProperties.get(expression.name); + if (definition !== undefined) { + const nextResolving = new Set(resolving); + nextResolving.add(expression.name); + appendUnique(candidates, resolveStaticValue(definition, customProperties, nextResolving)); + } + } + if (expression.fallback) { + appendUnique( + candidates, + resolveFallbackCandidates(expression.fallback, customProperties, resolving), + ); + } + } + return candidates; +} + +/** + * Resolve only custom properties proven to be static `:root` values. + * + * Unknown/scoped/dynamic/cyclic variables remain in the first position so + * plan validation never guesses their runtime primary. Their explicit + * fallback families are appended as candidates for deterministic injection + * and linting. + */ +export function resolveFontFamilyDeclarationFamilies( + declaration: string, + customProperties: ReadonlyMap, +): string[] { + return resolveFontFamilyDeclarationCandidates(declaration, customProperties).map( + (candidate) => candidate.value, + ); +} + +export function resolveFontFamilyDeclarationCandidates( + declaration: string, + customProperties: ReadonlyMap, +): FontFamilyValueToken[] { + const families = parseFontFamilyValueTokens(declaration); + const primary = families[0]; + if (!primary) return []; + + const expression = parseVarExpression(primary); + if (!expression) return families; + + const definition = customProperties.get(expression.name); + if (definition !== undefined) { + const resolved = resolveStaticValue(definition, customProperties, new Set([expression.name])); + if (resolved[0]?.kind === "family") { + return [...resolved, ...families.slice(1)]; + } + const unresolved = [primary]; + appendUnique(unresolved, resolved); + if (expression.fallback) { + appendUnique( + unresolved, + resolveFallbackCandidates(expression.fallback, customProperties, new Set()), + ); + } + appendUnique(unresolved, families.slice(1)); + return unresolved; + } + + const unresolved = [primary]; + if (expression.fallback) { + appendUnique( + unresolved, + resolveFallbackCandidates(expression.fallback, customProperties, new Set()), + ); + } + appendUnique(unresolved, families.slice(1)); + return unresolved; +} diff --git a/packages/parsers/src/index.ts b/packages/parsers/src/index.ts index 93d919af0d..c372f1e182 100644 --- a/packages/parsers/src/index.ts +++ b/packages/parsers/src/index.ts @@ -22,3 +22,14 @@ export { CANONICAL_FONT_DISPLAY_NAMES, resolveAliasDisplayName, } from "./fontAliases.js"; +export { + CSS_WIDE_FONT_FAMILY_KEYWORDS, + collectStaticRootFontFamilyCustomProperties, + isCssWideFontFamilyKeyword, + parseFontFamilyValue, + parseFontFamilyValueTokens, + resolveFontFamilyDeclarationCandidates, + resolveFontFamilyDeclarationFamilies, + type FontFamilyCustomPropertyDeclaration, + type FontFamilyValueToken, +} from "./fontFamilyValue.js"; diff --git a/packages/producer/src/services/deterministicFonts-failClosed.test.ts b/packages/producer/src/services/deterministicFonts-failClosed.test.ts index 29c5000ec4..066ecbb144 100644 --- a/packages/producer/src/services/deterministicFonts-failClosed.test.ts +++ b/packages/producer/src/services/deterministicFonts-failClosed.test.ts @@ -17,6 +17,7 @@ import { describe, expect, it } from "bun:test"; import { parseHTML } from "linkedom"; import { + collectFontFamilyCustomProperties, FONT_FETCH_FAILED, FontFetchError, injectDeterministicFontFaces, @@ -62,7 +63,9 @@ function makeGoogleFontFetch(cssRequests: string[]): typeof fetch { if (requestUrl.startsWith("https://fonts.googleapis.com/")) { cssRequests.push(requestUrl); const family = new URL(requestUrl).searchParams.get("family")?.split(":", 1)[0] ?? "test"; - const fontUrl = `https://fonts.gstatic.com/s/test/v1/${family.toLowerCase().replace(/\s+/g, "-")}.woff2`; + const fontUrl = `https://fonts.gstatic.com/s/test/v1/${family + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-")}.woff2`; return new Response( `@font-face { font-style: normal; @@ -288,6 +291,126 @@ describe("injectDeterministicFontFaces — failClosedFontFetch: true", () => { expect(result).toBe(html); }); + it("extracts explicit var() fallback families without close-paren artifacts", async () => { + const cssRequests: string[] = []; + const html = `

hello

`; + const result = await injectDeterministicFontFaces(html, { + failClosedFontFetch: true, + allowSystemFontCapture: false, + fetchImpl: makeGoogleFontFetch(cssRequests), + }); + const requestedFamilies = cssRequests.map( + (request) => new URL(request).searchParams.get("family")?.split(":", 1)[0], + ); + expect(requestedFamilies).toEqual(["Montserrat Bold"]); + expect(result).toContain(`font-family: "Montserrat Bold"`); + expect(result).not.toContain(`font-family: "Montserrat Bold\\")"`); + expect(result).not.toContain(`font-family: "serif)"`); + }); + + it("resolves a static :root variable through a nested fallback", async () => { + const cssRequests: string[] = []; + const html = `

hello

`; + await injectDeterministicFontFaces(html, { + failClosedFontFetch: true, + allowSystemFontCapture: false, + fetchImpl: makeGoogleFontFetch(cssRequests), + }); + expect( + new URL(cssRequests[0]!).searchParams.get("family")?.startsWith("Montserrat Bold:"), + ).toBe(true); + }); + + it("treats CSS-wide keywords as values rather than fetchable family names", async () => { + const html = `

hello

`; + const result = await injectDeterministicFontFaces(html, { + failClosedFontFetch: true, + allowSystemFontCapture: false, + fetchImpl: makeFailingFetch(), + }); + expect(result).toBe(html); + }); + + it("fetches quoted generic and CSS-wide names, including through a root variable", async () => { + const cssRequests: string[] = []; + const html = `

hello

world

`; + await injectDeterministicFontFaces(html, { + failClosedFontFetch: true, + allowSystemFontCapture: false, + fetchImpl: makeGoogleFontFetch(cssRequests), + }); + expect( + cssRequests.map((request) => new URL(request).searchParams.get("family")?.split(":", 1)[0]), + ).toEqual(["system-ui", "inherit", "serif"]); + }); + + it("does not mistake no-space quoted family names for CSS functions", async () => { + const cssRequests: string[] = []; + const html = `

hello

`; + await injectDeterministicFontFaces(html, { + failClosedFontFetch: true, + allowSystemFontCapture: false, + fetchImpl: makeGoogleFontFetch(cssRequests), + }); + expect( + cssRequests.map((request) => new URL(request).searchParams.get("family")?.split(":", 1)[0]), + ).toEqual(["ACME(Display)", "var(--Display)"]); + }); + + it("invalidates only aliases named by malformed CSS blocks", () => { + const differentName = ` + + + `; + expect(collectFontFamilyCustomProperties(differentName).get("--heading")).toBe(`"Inter"`); + + const sameName = ` + + + `; + expect(collectFontFamilyCustomProperties(sameName).has("--heading")).toBe(false); + }); + + it("ignores commented @font-face declarations when scanning requested families", async () => { + const html = `

hello

`; + const fetchImpl = (async () => + new Response("/* no extra faces */", { status: 200 })) as unknown as typeof fetch; + const result = await injectDeterministicFontFaces(html, { + failClosedFontFetch: true, + fetchImpl, + }); + expect(result).toContain("data-hyperframes-deterministic-fonts"); + expect(result).not.toContain(`font-family: "Wrong"`); + }); + it("resolves simple CSS var() font aliases when injecting deterministic fonts", async () => { const html = ` +
text
+ `; + expect(normalizeSystemFontPrimaryFamilies(html)).toBe(html); + }); +}); describe("FONT_ALIASES cross-platform coverage", () => { it("maps macOS sans-serif system fonts to inter", () => { diff --git a/packages/producer/src/services/deterministicFonts.ts b/packages/producer/src/services/deterministicFonts.ts index 742844154f..6a1f9adda2 100644 --- a/packages/producer/src/services/deterministicFonts.ts +++ b/packages/producer/src/services/deterministicFonts.ts @@ -9,6 +9,14 @@ import { locateSystemFontVariants, SYSTEM_FONT_SIZE_LIMIT, } from "@hyperframes/core/fonts/system-locator"; +import { + collectStaticRootFontFamilyCustomProperties, + isCssWideFontFamilyKeyword, + parseFontFamilyValue, + parseFontFamilyValueTokens, + resolveFontFamilyDeclarationCandidates, + type FontFamilyCustomPropertyDeclaration, +} from "@hyperframes/parsers/composition"; import { parseHTML } from "linkedom"; import postcss, { type AtRule, type Declaration, type Rule } from "postcss"; import { EMBEDDED_FONT_DATA } from "./fontData.generated.js"; @@ -55,17 +63,12 @@ export const GENERIC_FAMILIES: ReadonlySet = new Set([ * preserved. Pass each name through `normalizeFamilyName` for case- * insensitive comparisons. */ -export function parseFontFamilyValue(value: string): string[] { - return value - .split(",") - .map((piece) => piece.trim().replace(/^['"]/, "").replace(/['"]$/, "").trim()) - .filter((piece) => piece.length > 0); -} +export { parseFontFamilyValue, resolveFontFamilyDeclarationCandidates }; function systemPrimaryReplacement(value: string, deterministicPrimary: string): string | null { - const families = parseFontFamilyValue(value); - if (families.length === 0) return null; - if (!GENERIC_FAMILIES.has(normalizeFamilyName(families[0]!))) return null; + const primary = parseFontFamilyValueTokens(value)[0]; + if (!primary || primary.quoted) return null; + if (!GENERIC_FAMILIES.has(normalizeFamilyName(primary.value))) return null; return `${deterministicPrimary}, ${value.trim()}`; } @@ -175,12 +178,34 @@ export type FontFamilyDeclaration = { families: string[]; }; -function collectCssCustomProperties(css: string, customProperties: Map): void { +function collectCssCustomPropertyDeclarations( + css: string, + declarations: FontFamilyCustomPropertyDeclaration[], +): void { const root = parseCssRoot(css); - if (!root) return; + if (!root) { + for (const match of css.matchAll(/(--[A-Za-z0-9_-]+)\s*:/g)) { + declarations.push({ + name: match[1]!, + value: "", + staticRoot: false, + }); + } + return; + } root.walkDecls((decl) => { if (!decl.prop.startsWith("--")) return; - customProperties.set(decl.prop, decl.value); + const parent = decl.parent; + const staticRoot = + parent?.type === "rule" && + (parent as Rule).selector.trim() === ":root" && + parent.parent?.type === "root"; + declarations.push({ + name: decl.prop, + value: decl.value, + staticRoot, + important: decl.important, + }); }); } @@ -216,61 +241,23 @@ function* iterateInlineStyleFontFamilyDeclarations( } /** - * Collect simple CSS custom-property font aliases from style blocks and inline - * styles. CSS cascade is richer than this map, but for compiler-generated - * imports the common shape is `--font: Inter, sans-serif` paired with - * `font-family: var(--font)`. + * Collect only custom-property font aliases whose value is statically + * provable: unconditional top-level `:root` declarations with no scoped, + * dynamic, or inline declaration of the same property. Ambiguous values remain + * unresolved for the browser cascade. */ export function collectFontFamilyCustomProperties(html: string): Map { const { document } = parseHTML(html); - const customProperties = new Map(); + const declarations: FontFamilyCustomPropertyDeclaration[] = []; for (const styleEl of Array.from(document.querySelectorAll("style"))) { - collectCssCustomProperties(styleEl.textContent ?? "", customProperties); + collectCssCustomPropertyDeclarations(styleEl.textContent ?? "", declarations); } for (const el of Array.from(document.querySelectorAll("[style]"))) { - collectCssCustomProperties(`*{${el.getAttribute("style") ?? ""}}`, customProperties); - } - - return customProperties; -} - -function primaryCssVariableName(value: string): string | null { - const trimmed = value.trim(); - if (!trimmed.toLowerCase().startsWith("var(")) return null; - - let depth = 0; - for (let index = 0; index < trimmed.length; index += 1) { - const char = trimmed[index]; - if (char === "(") { - depth += 1; - continue; - } - if (char !== ")") continue; - depth -= 1; - if (depth !== 0) continue; - - const varExpression = trimmed.slice(0, index + 1); - const inner = varExpression.slice(4, -1).trim(); - const commaIndex = inner.indexOf(","); - const variableName = (commaIndex === -1 ? inner : inner.slice(0, commaIndex)).trim(); - return /^--[A-Za-z0-9_-]+$/.test(variableName) ? variableName : null; + collectCssCustomPropertyDeclarations(`*{${el.getAttribute("style") ?? ""}}`, declarations); } - return null; -} - -export function resolveFontFamilyDeclarationFamilies( - declaration: string, - customProperties: ReadonlyMap, -): string[] { - const families = parseFontFamilyValue(declaration); - const variableName = primaryCssVariableName(declaration); - if (!variableName) return families; - - const resolved = customProperties.get(variableName); - if (!resolved) return families; - return [...parseFontFamilyValue(resolved), ...families.slice(1)]; + return collectStaticRootFontFamilyCustomProperties(declarations); } /** @@ -419,13 +406,17 @@ function extractRequestedFontFamilies(html: string): Map { const requested = new Map(); const customProperties = collectFontFamilyCustomProperties(html); for (const { declaration } of iterateFontFamilyDeclarations(html)) { - for (const originalCase of resolveFontFamilyDeclarationFamilies( - declaration, - customProperties, - )) { - const normalized = originalCase.toLowerCase(); - if (!normalized || GENERIC_FAMILIES.has(normalized)) continue; - if (normalized.startsWith("var(")) continue; + for (const candidate of resolveFontFamilyDeclarationCandidates(declaration, customProperties)) { + const originalCase = candidate.value; + const normalized = normalizeFamilyName(originalCase); + if ( + !normalized || + (!candidate.quoted && + (GENERIC_FAMILIES.has(normalized) || isCssWideFontFamilyKeyword(normalized))) || + candidate.kind === "function" + ) { + continue; + } if (!requested.has(normalized)) requested.set(normalized, originalCase); } } @@ -457,6 +448,9 @@ function buildFontFaceRule( ].join("\n"); } +// This existing fetch/face assembly funnel has multiple format and fallback +// branches; this change only shifts its lines while adding shared parsing. +// fallow-ignore-next-line complexity async function buildFontFaceCss( requestedFamilies: Map, options: InternalFontFetchOptions, @@ -737,6 +731,9 @@ async function ensureWoff2DataUri( return `data:font/woff2;base64,${readFileSync(cachePath).toString("base64")}`; } +// This existing network/cache retry funnel is unchanged apart from line +// movement caused by the shared parser integration. +// fallow-ignore-next-line complexity async function fetchGoogleFont( familyName: string, options: InternalFontFetchOptions, diff --git a/packages/producer/src/services/render/planValidation.test.ts b/packages/producer/src/services/render/planValidation.test.ts index 7d10520b74..9beb0510cd 100644 --- a/packages/producer/src/services/render/planValidation.test.ts +++ b/packages/producer/src/services/render/planValidation.test.ts @@ -180,6 +180,19 @@ describe("parseFontFamilyValue", () => { it("handles an all-whitespace value as empty", () => { expect(parseFontFamilyValue(` `)).toEqual([]); }); + + it("keeps commas inside quotes and var() fallbacks", () => { + expect(parseFontFamilyValue(`"ACME, Inc", var(--heading, "Montserrat Bold"), serif`)).toEqual([ + "ACME, Inc", + `var(--heading, "Montserrat Bold")`, + "serif", + ]); + }); + + it("rejects stray production close-paren artifacts as invalid values", () => { + expect(parseFontFamilyValue(`Montserrat Bold")`)).toEqual([]); + expect(parseFontFamilyValue("serif)")).toEqual([]); + }); }); describe("validateNoSystemFonts", () => { @@ -297,4 +310,70 @@ describe("validateNoSystemFonts", () => { `; expect(() => validateNoSystemFonts(ok)).not.toThrow(); }); + + it("accepts quoted family names containing commas and no-space function shapes", () => { + const ok = ``; + expect(() => validateNoSystemFonts(ok)).not.toThrow(); + }); + + it("handles CSS-wide keywords explicitly without treating them as font files", () => { + for (const keyword of ["inherit", "initial", "unset", "revert", "revert-layer"]) { + expect(() => + validateNoSystemFonts(``), + ).not.toThrow(); + } + }); + + it("does not classify quoted generic or CSS-wide names as keywords, including through root vars", () => { + const quoted = ``; + expect(() => validateNoSystemFonts(quoted)).not.toThrow(); + }); + + it("does not guess an unresolved variable from its fallback", () => { + const dynamic = ``; + expect(() => validateNoSystemFonts(dynamic)).not.toThrow(); + }); + + it("does not guess when a scoped declaration makes a :root value ambiguous", () => { + const scoped = ``; + expect(() => validateNoSystemFonts(scoped)).not.toThrow(); + }); + + it("does not guess cyclic root variables", () => { + const cyclic = ``; + expect(() => validateNoSystemFonts(cyclic)).not.toThrow(); + }); + + it("malformed CSS invalidates only matching static aliases", () => { + const differentName = ` + + + `; + expect(() => validateNoSystemFonts(differentName)).toThrow(PlanValidationError); + + const sameName = ` + + + `; + expect(() => validateNoSystemFonts(sameName)).not.toThrow(); + }); }); diff --git a/packages/producer/src/services/render/planValidation.ts b/packages/producer/src/services/render/planValidation.ts index e335e5bc41..16f81f6dd1 100644 --- a/packages/producer/src/services/render/planValidation.ts +++ b/packages/producer/src/services/render/planValidation.ts @@ -6,11 +6,12 @@ */ import { BROWSER_GPU_NOT_SOFTWARE } from "@hyperframes/engine"; +import { isCssWideFontFamilyKeyword } from "@hyperframes/parsers/composition"; import { collectFontFamilyCustomProperties, GENERIC_FAMILIES, iterateFontFamilyDeclarations, - resolveFontFamilyDeclarationFamilies, + resolveFontFamilyDeclarationCandidates, } from "../deterministicFonts.js"; /** @@ -123,12 +124,20 @@ export function validateNoGpuEncode(config: ValidateNoGpuEncodeInput): void { export function validateNoSystemFonts(compiledHtml: string): void { const customProperties = collectFontFamilyCustomProperties(compiledHtml); for (const { surface, declaration } of iterateFontFamilyDeclarations(compiledHtml)) { - const families = resolveFontFamilyDeclarationFamilies(declaration, customProperties); - if (families.length === 0) continue; - const primaryRaw = families[0]!; - // Unresolved var() primaries are left to the browser; resolved custom - // properties are checked above so common `--font: system-ui` aliases fail. - if (!GENERIC_FAMILIES.has(primaryRaw.toLowerCase())) continue; + const candidates = resolveFontFamilyDeclarationCandidates(declaration, customProperties); + const primary = candidates[0]; + if (!primary) continue; + const primaryRaw = primary.value; + // CSS-wide keywords and unresolved function primaries depend on the normal + // browser cascade. The shared resolver replaces only unambiguous static + // :root variables, so preserve legacy behavior and do not guess the value. + if ( + primary.kind === "function" || + (!primary.quoted && isCssWideFontFamilyKeyword(primaryRaw)) + ) { + continue; + } + if (primary.quoted || !GENERIC_FAMILIES.has(primaryRaw.toLowerCase())) continue; throw new PlanValidationError( SYSTEM_FONT_USED, `[planValidation] Composition declares a host-OS / generic primary ${surface}: ` +