mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 17:27:09 +00:00
fix(lint): root_dimensions_mismatch sub-composition false positive + regex SSOT consolidation (#4012)
* fix(lint): guard root_dimensions_mismatch against sub-compositions Sub-compositions mounted via data-composition-src are fetched with DOMParser and only their matched <template>/<body> subtree is imported into the live document. A sub-composition authored as a full standalone HTML document still carries its own <html>/<head>/<meta viewport>, but none of that ever reaches the rendering document, so a mismatch there was always a false positive. Guard the rule with options.isSubComposition, matching the sibling missing_timeline_registry rule's existing pattern. Split the warning wording: html/body CSS actually sizes the element that clips the root, but the CDP viewport is set from the root's own data-width/data-height (frameCapture.ts), never from <meta viewport>. A viewport-only mismatch is real drift but not a clipping risk. Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com> * fix(cli,parsers): consolidate scaffold resolution regexes into one definition applyResolutionPreset kept its own prefix-capturing regex literals for the html/body CSS size and meta viewport content, duplicating the same three patterns already defined in @hyperframes/parsers for lint's read-only root_dimensions_mismatch comparison. Merge to one definition per pattern that captures both the prefix (for in-place replace) and the digits (for comparison), so the two callers can no longer drift on where a scaffold's resolution lives. Existing applyResolutionPreset tests stay green. Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com> --------- Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
This commit is contained in:
co-authored by
Miguel Ángel
parent
d6fe66707f
commit
2229d0d6a6
@@ -53,6 +53,11 @@ import {
|
||||
normalizeResolutionFlag,
|
||||
type CanvasResolution,
|
||||
} from "@hyperframes/core";
|
||||
import {
|
||||
HTML_BODY_CSS_HEIGHT_FIRST_RE,
|
||||
HTML_BODY_CSS_WIDTH_FIRST_RE,
|
||||
VIEWPORT_META_SIZE_RE,
|
||||
} from "@hyperframes/parsers";
|
||||
|
||||
function resolveScaffoldTemplateId(exampleFlag: string | undefined, hasMediaFile: boolean): string {
|
||||
const example = exampleFlag === "agent" ? "blank" : exampleFlag;
|
||||
@@ -527,20 +532,18 @@ export function applyResolutionPreset(destDir: string, resolution: CanvasResolut
|
||||
|
||||
// Inline `html, body { ... }` CSS: handle width-before-height and
|
||||
// height-before-width orderings. Hand-authored templates can use either.
|
||||
const bodyCssRe = /(html\s*,\s*body\s*\{[^}]*?width:\s*)\d+px([^}]*?height:\s*)\d+px/i;
|
||||
if (bodyCssRe.test(html)) {
|
||||
html = html.replace(bodyCssRe, `$1${width}px$2${height}px`);
|
||||
// Groups 1 and 3 are the text before each dimension, 2 and 4 the digits.
|
||||
if (HTML_BODY_CSS_WIDTH_FIRST_RE.test(html)) {
|
||||
html = html.replace(HTML_BODY_CSS_WIDTH_FIRST_RE, `$1${width}px$3${height}px`);
|
||||
changed = true;
|
||||
}
|
||||
const bodyCssReverseRe = /(html\s*,\s*body\s*\{[^}]*?height:\s*)\d+px([^}]*?width:\s*)\d+px/i;
|
||||
if (bodyCssReverseRe.test(html)) {
|
||||
html = html.replace(bodyCssReverseRe, `$1${height}px$2${width}px`);
|
||||
if (HTML_BODY_CSS_HEIGHT_FIRST_RE.test(html)) {
|
||||
html = html.replace(HTML_BODY_CSS_HEIGHT_FIRST_RE, `$1${height}px$3${width}px`);
|
||||
changed = true;
|
||||
}
|
||||
|
||||
const viewportRe = /(<meta[^>]*name=["']viewport["'][^>]*content=["'])width=\d+,\s*height=\d+/i;
|
||||
if (viewportRe.test(html)) {
|
||||
html = html.replace(viewportRe, `$1width=${width}, height=${height}`);
|
||||
if (VIEWPORT_META_SIZE_RE.test(html)) {
|
||||
html = html.replace(VIEWPORT_META_SIZE_RE, `$1${width}$3${height}`);
|
||||
changed = true;
|
||||
}
|
||||
|
||||
|
||||
@@ -176,6 +176,71 @@ describe("core rules", () => {
|
||||
expect(result.findings.find((f) => f.code === "root_dimensions_mismatch")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not report root_dimensions_mismatch for a full sub-composition document whose own viewport meta disagrees with its root", async () => {
|
||||
// Matches the hf2550 flowchart-vertical fixture's shape: a full standalone
|
||||
// document mounted as a sub-composition. See the rule's comment in core.ts
|
||||
// for why its own <meta viewport> never reaches the rendering document.
|
||||
const html = `
|
||||
<!doctype html>
|
||||
<html>
|
||||
<head>
|
||||
<meta name="viewport" content="width=1440, height=2560" />
|
||||
</head>
|
||||
<body>
|
||||
<div id="root" data-composition-id="c1" data-width="1080" data-height="1920"></div>
|
||||
<script>window.__timelines = {};</script>
|
||||
</body>
|
||||
</html>`;
|
||||
const result = await lintHyperframeHtml(html, { isSubComposition: true });
|
||||
expect(result.findings.find((f) => f.code === "root_dimensions_mismatch")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("still reports root_dimensions_mismatch for the same shape linted as a top-level composition, with no-clipping-risk wording since there is no html/body CSS block at all", async () => {
|
||||
const html = `
|
||||
<!doctype html>
|
||||
<html>
|
||||
<head>
|
||||
<meta name="viewport" content="width=1440, height=2560" />
|
||||
</head>
|
||||
<body>
|
||||
<div id="root" data-composition-id="c1" data-width="1080" data-height="1920"></div>
|
||||
<script>window.__timelines = {};</script>
|
||||
</body>
|
||||
</html>`;
|
||||
const result = await lintHyperframeHtml(html);
|
||||
const finding = result.findings.find((f) => f.code === "root_dimensions_mismatch");
|
||||
expect(finding).toBeDefined();
|
||||
// No html/body CSS block is present here at all (the real hf2550 fixture
|
||||
// shape) -- distinct from the "present and matching" case covered below --
|
||||
// so the "absent" and "matches" cases of describeSizeMismatch must both
|
||||
// route to the same no-clipping-risk wording, not just the "matches" one.
|
||||
expect(finding?.message).not.toContain("clips");
|
||||
expect(finding?.message.toLowerCase()).toContain("no effect on capture");
|
||||
});
|
||||
|
||||
it("uses no-clipping-risk wording when only the viewport meta disagrees and html/body CSS matches the root", async () => {
|
||||
const html = portraitCompositionWithScaffold(
|
||||
"width: 1080px; height: 1920px;",
|
||||
"width=1440, height=2560",
|
||||
);
|
||||
const result = await lintHyperframeHtml(html);
|
||||
const finding = result.findings.find((f) => f.code === "root_dimensions_mismatch");
|
||||
expect(finding).toBeDefined();
|
||||
expect(finding?.message).toContain("the viewport meta is 1440x2560");
|
||||
expect(finding?.message).not.toContain("clips");
|
||||
expect(finding?.message.toLowerCase()).toContain("no effect on capture");
|
||||
});
|
||||
|
||||
it("keeps the body-clipping wording when html/body CSS itself disagrees with the root", async () => {
|
||||
const html = portraitCompositionWithScaffold(
|
||||
"width: 1920px; height: 1080px;",
|
||||
"width=1080, height=1920",
|
||||
);
|
||||
const result = await lintHyperframeHtml(html);
|
||||
const finding = result.findings.find((f) => f.code === "root_dimensions_mismatch");
|
||||
expect(finding?.message).toContain("clips");
|
||||
});
|
||||
|
||||
it("accepts body as the composition root", async () => {
|
||||
const html = `
|
||||
<html><body data-composition-id="c1" data-width="1920" data-height="1080">
|
||||
|
||||
@@ -239,15 +239,17 @@ function findVisibleMarkupCommentLeak(source: string): string | null {
|
||||
|
||||
type ScaffoldSize = { width: string; height: string };
|
||||
|
||||
// Groups 1 and 3 are prefix text for applyResolutionPreset's in-place
|
||||
// replace; lint only reads the digit groups (2 and 4).
|
||||
function readHtmlBodyCssSize(source: string): ScaffoldSize | null {
|
||||
const widthFirst = source.match(HTML_BODY_CSS_WIDTH_FIRST_RE);
|
||||
if (widthFirst) {
|
||||
const [, width = "", height = ""] = widthFirst;
|
||||
const [, , width = "", , height = ""] = widthFirst;
|
||||
return { width, height };
|
||||
}
|
||||
const heightFirst = source.match(HTML_BODY_CSS_HEIGHT_FIRST_RE);
|
||||
if (heightFirst) {
|
||||
const [, height = "", width = ""] = heightFirst;
|
||||
const [, , height = "", , width = ""] = heightFirst;
|
||||
return { width, height };
|
||||
}
|
||||
return null;
|
||||
@@ -256,7 +258,7 @@ function readHtmlBodyCssSize(source: string): ScaffoldSize | null {
|
||||
function readViewportMetaSize(source: string): ScaffoldSize | null {
|
||||
const match = source.match(VIEWPORT_META_SIZE_RE);
|
||||
if (!match) return null;
|
||||
const [, width = "", height = ""] = match;
|
||||
const [, , width = "", , height = ""] = match;
|
||||
return { width, height };
|
||||
}
|
||||
|
||||
@@ -270,15 +272,43 @@ function describeSizeMismatch(
|
||||
return `${label} is ${size.width}x${size.height}`;
|
||||
}
|
||||
|
||||
function findScaffoldSizeMismatches(
|
||||
// Only html/body CSS actually clips the root; the CDP viewport comes from
|
||||
// data-width/data-height, not `<meta viewport>` — so a viewport-only drift gets distinct wording.
|
||||
function describeRootDimensionsDrift(
|
||||
source: string,
|
||||
dataWidth: string,
|
||||
dataHeight: string,
|
||||
): string[] {
|
||||
return [
|
||||
describeSizeMismatch("html/body CSS", readHtmlBodyCssSize(source), dataWidth, dataHeight),
|
||||
describeSizeMismatch("the viewport meta", readViewportMetaSize(source), dataWidth, dataHeight),
|
||||
].filter((mismatch): mismatch is string => mismatch !== null);
|
||||
): { message: string; fixHint: string } | null {
|
||||
const bodyCssMismatch = describeSizeMismatch(
|
||||
"html/body CSS",
|
||||
readHtmlBodyCssSize(source),
|
||||
dataWidth,
|
||||
dataHeight,
|
||||
);
|
||||
const viewportMismatch = describeSizeMismatch(
|
||||
"the viewport meta",
|
||||
readViewportMetaSize(source),
|
||||
dataWidth,
|
||||
dataHeight,
|
||||
);
|
||||
if (!bodyCssMismatch && !viewportMismatch) return null;
|
||||
|
||||
const declared = `Root composition declares data-width="${dataWidth}" data-height="${dataHeight}"`;
|
||||
if (!bodyCssMismatch) {
|
||||
return {
|
||||
message: `${declared}, but ${viewportMismatch}. The viewport meta has no effect on capture — the renderer sizes the viewport from the root's own data-width/data-height — so this is stale metadata, not a clipping risk.`,
|
||||
fixHint:
|
||||
"update the meta viewport to match, or scaffold with `hyperframes init --resolution portrait`",
|
||||
};
|
||||
}
|
||||
const mismatches = viewportMismatch
|
||||
? `${bodyCssMismatch} and ${viewportMismatch}`
|
||||
: bodyCssMismatch;
|
||||
return {
|
||||
message: `${declared}, but ${mismatches}. The scaffolded body clips the composition at its old size.`,
|
||||
fixHint:
|
||||
"update html/body CSS and the meta viewport to match, or scaffold with `hyperframes init --resolution portrait`",
|
||||
};
|
||||
}
|
||||
|
||||
export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
|
||||
@@ -339,28 +369,26 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
|
||||
// root. `hyperframes check`'s layout audits can't see it: they measure
|
||||
// against the root's own (already-correct) rect, not the body's.
|
||||
//
|
||||
// A sub-composition fragment has no html/body or viewport to compare
|
||||
// against, which makes this top-level-only without a separate guard.
|
||||
({ rootTag, source }) => {
|
||||
if (!rootTag) return [];
|
||||
// Sub-compositions are exempt: loadExternalCompositions (packages/core/src/
|
||||
// runtime/compositionLoader.ts) mounts only the matched <template>/<body>
|
||||
// subtree, so a sub-comp's own <html>/<head>/<meta viewport> never reach
|
||||
// the rendering document, even when it's a full standalone document.
|
||||
({ rootTag, source, options }) => {
|
||||
if (!rootTag || options.isSubComposition) return [];
|
||||
const dataWidth = readAttr(rootTag.raw, "data-width");
|
||||
const dataHeight = readAttr(rootTag.raw, "data-height");
|
||||
if (!dataWidth || !dataHeight) return [];
|
||||
|
||||
const mismatches = findScaffoldSizeMismatches(source, dataWidth, dataHeight);
|
||||
if (mismatches.length === 0) return [];
|
||||
const drift = describeRootDimensionsDrift(source, dataWidth, dataHeight);
|
||||
if (!drift) return [];
|
||||
|
||||
return [
|
||||
{
|
||||
code: "root_dimensions_mismatch",
|
||||
severity: "warning",
|
||||
message:
|
||||
`Root composition declares data-width="${dataWidth}" data-height="${dataHeight}", ` +
|
||||
`but ${mismatches.join(" and ")}. The scaffolded body clips the composition at its ` +
|
||||
`old size.`,
|
||||
message: drift.message,
|
||||
elementId: readAttr(rootTag.raw, "id") || undefined,
|
||||
fixHint:
|
||||
"update html/body CSS and the meta viewport to match, or scaffold with `hyperframes init --resolution portrait`",
|
||||
fixHint: drift.fixHint,
|
||||
snippet: truncateSnippet(rootTag.raw),
|
||||
},
|
||||
];
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
HTML_BODY_CSS_HEIGHT_FIRST_RE,
|
||||
HTML_BODY_CSS_WIDTH_FIRST_RE,
|
||||
VIEWPORT_META_SIZE_RE,
|
||||
} from "./canvasScaffoldPatterns.js";
|
||||
|
||||
// Both callers key off group numbers: @hyperframes/lint's
|
||||
// root_dimensions_mismatch reads the digits, @hyperframes/cli's
|
||||
// applyResolutionPreset substitutes `$1<new>$3<new>` to keep the text around
|
||||
// them. Pin the layout here so neither can renumber against the other.
|
||||
const CASES = [
|
||||
{
|
||||
name: "HTML_BODY_CSS_WIDTH_FIRST_RE",
|
||||
re: HTML_BODY_CSS_WIDTH_FIRST_RE,
|
||||
source: "html, body { margin: 0; width: 1920px; height: 1080px; overflow: hidden; }",
|
||||
digits: ["1920", "1080"],
|
||||
replacement: "$13840px$32160px",
|
||||
replaced: "html, body { margin: 0; width: 3840px; height: 2160px; overflow: hidden; }",
|
||||
},
|
||||
{
|
||||
name: "HTML_BODY_CSS_HEIGHT_FIRST_RE",
|
||||
re: HTML_BODY_CSS_HEIGHT_FIRST_RE,
|
||||
source: "html, body { margin: 0; height: 1080px; width: 1920px; overflow: hidden; }",
|
||||
digits: ["1080", "1920"],
|
||||
replacement: "$12160px$33840px",
|
||||
replaced: "html, body { margin: 0; height: 2160px; width: 3840px; overflow: hidden; }",
|
||||
},
|
||||
{
|
||||
name: "VIEWPORT_META_SIZE_RE",
|
||||
re: VIEWPORT_META_SIZE_RE,
|
||||
source: '<meta name="viewport" content="width=1920, height=1080" />',
|
||||
digits: ["1920", "1080"],
|
||||
replacement: "$13840$32160",
|
||||
replaced: '<meta name="viewport" content="width=3840, height=2160" />',
|
||||
},
|
||||
];
|
||||
|
||||
describe("canvas scaffold patterns", () => {
|
||||
it.each(CASES)("$name exposes the two dimensions as groups 2 and 4", ({ re, source, digits }) => {
|
||||
const match = source.match(re);
|
||||
expect(match).not.toBeNull();
|
||||
expect(match?.slice(1)).toHaveLength(4);
|
||||
expect([match?.[2], match?.[4]]).toEqual(digits);
|
||||
});
|
||||
|
||||
it.each(CASES)(
|
||||
"$name replaces both dimensions in place, leaving the rest untouched",
|
||||
({ re, source, replacement, replaced }) => {
|
||||
expect(source.replace(re, replacement)).toBe(replaced);
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -1,25 +1,16 @@
|
||||
/**
|
||||
* Where a scaffolded project's canvas resolution lives besides the composition
|
||||
* root's own `data-width`/`data-height`: the inline `html, body { width;
|
||||
* height }` CSS block and the `<meta name="viewport">` `content` attribute.
|
||||
* `@hyperframes/cli`'s `applyResolutionPreset` rewrites those same two places
|
||||
* when a project scaffolds with `--resolution`, but keeps its own
|
||||
* prefix-capturing regexes for that replace — these are a separate read-only
|
||||
* definition of the same locations, not the literal patterns it uses.
|
||||
*
|
||||
* Lives in `@hyperframes/parsers` rather than `@hyperframes/cli` because
|
||||
* `@hyperframes/lint` cannot depend on `cli` (`cli` depends on `lint`, not the
|
||||
* reverse), and `parsers` is a dependency both already share.
|
||||
* Where a scaffolded project's canvas resolution lives besides the root's own
|
||||
* data-width/data-height: the html/body CSS size and the meta viewport content.
|
||||
*/
|
||||
|
||||
/** Matches `html, body { ...width: <n>px... height: <n>px... }`. */
|
||||
/** html/body CSS, width first. Groups 1/3 are the surrounding text, 2/4 the digits. */
|
||||
export const HTML_BODY_CSS_WIDTH_FIRST_RE =
|
||||
/html\s*,\s*body\s*\{[^}]*?width:\s*(\d+)px[^}]*?height:\s*(\d+)px/i;
|
||||
/(html\s*,\s*body\s*\{[^}]*?width:\s*)(\d+)px([^}]*?height:\s*)(\d+)px/i;
|
||||
|
||||
/** Matches the same block with height authored before width. */
|
||||
/** Same block, height authored first. */
|
||||
export const HTML_BODY_CSS_HEIGHT_FIRST_RE =
|
||||
/html\s*,\s*body\s*\{[^}]*?height:\s*(\d+)px[^}]*?width:\s*(\d+)px/i;
|
||||
/(html\s*,\s*body\s*\{[^}]*?height:\s*)(\d+)px([^}]*?width:\s*)(\d+)px/i;
|
||||
|
||||
/** Matches `<meta ... name="viewport" ... content="width=<n>, height=<n>">`. */
|
||||
/** Viewport meta content. Shared by lint (reads groups 2/4) and cli (replaces via 1/3). */
|
||||
export const VIEWPORT_META_SIZE_RE =
|
||||
/<meta[^>]*name=["']viewport["'][^>]*content=["']width=(\d+),\s*height=(\d+)/i;
|
||||
/(<meta[^>]*name=["']viewport["'][^>]*content=["']width=)(\d+)(,\s*height=)(\d+)/i;
|
||||
|
||||
Reference in New Issue
Block a user