diff --git a/packages/lint/src/rules/captions.test.ts b/packages/lint/src/rules/captions.test.ts index 6977acb6e..57fedcda9 100644 --- a/packages/lint/src/rules/captions.test.ts +++ b/packages/lint/src/rules/captions.test.ts @@ -163,4 +163,33 @@ describe("caption rules", () => { expect(finding).toBeDefined(); expect(finding?.severity).toBe("error"); }); + + describe("caption_text_overflow_risk — its fix must not create an error", () => { + const cap = (css: string) => ` + +
+
+
+ +`; + + it("clears when the fixHint is applied as written", async () => { + // The hint used to say "and overflow: hidden", which is exactly what + // caption_overflow_clips_scaled_words errors on. Following the warning + // produced an error. + const before = await lintHyperframeHtml(cap("position:absolute;white-space:nowrap")); + expect(before.findings.find((f) => f.code === "caption_text_overflow_risk")).toBeDefined(); + + const after = await lintHyperframeHtml( + cap("position:absolute;white-space:nowrap;max-width:1600px;overflow:visible"), + ); + const blocking = after.findings.filter((f) => f.severity !== "info"); + expect(blocking.map((f) => f.code)).not.toContain("caption_text_overflow_risk"); + expect(blocking.map((f) => f.code)).not.toContain("caption_overflow_clips_scaled_words"); + }); + }); }); diff --git a/packages/lint/src/rules/captions.ts b/packages/lint/src/rules/captions.ts index 410024257..17faf97dd 100644 --- a/packages/lint/src/rules/captions.ts +++ b/packages/lint/src/rules/captions.ts @@ -57,7 +57,11 @@ export const captionRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> selector: (selector ?? "").trim(), message: `Caption selector "${(selector ?? "").trim()}" has white-space: nowrap but no max-width. Long phrases will clip off-screen.`, fixHint: - "Add max-width: 1600px (landscape) or max-width: 900px (portrait) and overflow: hidden.", + // Deliberately does NOT say `overflow: hidden`: caption words are scaled + // above 1.0x, and clipping them is exactly what caption_overflow_clips_scaled_words + // errors on. Recommending it here made this warning's own fix produce an error. + "Add max-width: 1600px (landscape) or max-width: 900px (portrait). Keep " + + "overflow visible so scaled emphasis words are not clipped.", }); } } diff --git a/packages/lint/src/rules/composition.test.ts b/packages/lint/src/rules/composition.test.ts index a508e7e7c..68881f4d0 100644 --- a/packages/lint/src/rules/composition.test.ts +++ b/packages/lint/src/rules/composition.test.ts @@ -528,78 +528,6 @@ describe("composition rules", () => { }); }); - describe("overlapping_clips_same_track", () => { - it("flags overlapping clips on the same track", async () => { - const html = ` - -
-
A
-
B
-
- -`; - const result = await lintHyperframeHtml(html); - const finding = result.findings.find((f) => f.code === "overlapping_clips_same_track"); - expect(finding).toBeDefined(); - expect(finding?.severity).toBe("error"); - }); - - it("does not flag clips on different tracks", async () => { - const html = ` - -
-
A
-
B
-
- -`; - const result = await lintHyperframeHtml(html); - const finding = result.findings.find((f) => f.code === "overlapping_clips_same_track"); - expect(finding).toBeUndefined(); - }); - - it("does not flag sequential clips on the same track", async () => { - const html = ` - -
-
A
-
B
-
- -`; - const result = await lintHyperframeHtml(html); - const finding = result.findings.find((f) => f.code === "overlapping_clips_same_track"); - expect(finding).toBeUndefined(); - }); - - it("does not flag adjacencies where parseFloat + add drifts by a few ulps", async () => { - // parseFloat("0.1") + parseFloat("0.2") = 0.30000000000000004 - const html = ` - -
-
A
-
B
-
- -`; - const result = await lintHyperframeHtml(html); - const finding = result.findings.find((f) => f.code === "overlapping_clips_same_track"); - expect(finding).toBeUndefined(); - }); - }); - describe("root_composition_missing_html_wrapper", () => { it("flags bare composition div as error", async () => { // Exact scenario from the screenshot — bare div with composition attributes, no HTML wrapper diff --git a/packages/lint/src/rules/composition.ts b/packages/lint/src/rules/composition.ts index 2b4ea76f9..021826864 100644 --- a/packages/lint/src/rules/composition.ts +++ b/packages/lint/src/rules/composition.ts @@ -59,14 +59,6 @@ const HEAVY_OVERLAY_CSS_PATTERN = /(?:filter\s*:[^;}]*\bblur\s*\()|(?:clip-path\s*:(?!\s*(?:none|inherit|initial|unset)\b)\s*[^;}]+)|(?:radial-gradient\s*\()/i; const INLINE_STYLE_DISPLAY_NONE_PATTERN = /(?:^|;)\s*display\s*:\s*none\b/i; -// `parseFloat("0.1") + parseFloat("0.2") = 0.30000000000000004`. Sub-second -// authored adjacencies survive parse + add as a value a few ulps above the -// next clip's start; a strict `>` fires the overlap rule on adjacencies that -// are exact in the source HTML. 1μs sits ~11 orders of magnitude above the -// observed drift (worst ~2e-16s across every realistic decimal pair) and 4 -// below one 60fps frame (~16.67ms), so this only ever swallows float slop. -const OVERLAP_EPSILON_SECONDS = 1e-6; - function readTagTiming(rawTag: string) { return readClipTiming({ getAttribute: (name) => readAttr(rawTag, name) }); } @@ -164,6 +156,7 @@ function leftmostCompoundId(selector: string): string | null { // are scanned — the flat `[^{}]*` body class naturally skips @keyframes // bodies (which contain nested `{...}` stops) and other @-rules, so keyframe // selectors like `0%`/`100%` don't leak in. +// fallow-ignore-next-line complexity function collectHeavyOverlayHooks(styles: ExtractedBlock[]): { classes: Set; ids: Set; @@ -557,75 +550,6 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding return findings; }, - // overlapping_clips_same_track - // fallow-ignore-next-line complexity - ({ tags }) => { - const findings: HyperframeLintFinding[] = []; - - type ClipInfo = { start: number; end: number; elementId?: string; snippet: string }; - const trackMap = new Map(); - - for (const tag of tags) { - const trackStr = readAttr(tag.raw, COMPOSITION_ATTRIBUTES.trackIndex); - if (!trackStr) continue; - const timing = readTagTiming(tag.raw); - const { start, duration } = timing; - const track = trackStr; - - // Skip non-numeric (relative timing references like "intro-comp") - if (start == null || duration == null) continue; - - const clips = trackMap.get(track) || []; - clips.push({ - start, - end: start + duration, - elementId: readAttr(tag.raw, "id") || undefined, - snippet: truncateSnippet(tag.raw) || "", - }); - trackMap.set(track, clips); - } - - for (const [track, clips] of trackMap) { - clips.sort((a, b) => a.start - b.start); - for (let i = 0; i < clips.length - 1; i++) { - const current = clips[i]; - const next = clips[i + 1]; - if (!current || !next) continue; - if (current.end - next.start > OVERLAP_EPSILON_SECONDS) { - findings.push({ - code: "overlapping_clips_same_track", - severity: "error", - message: `Track ${track}: clip ending at ${current.end}s overlaps with clip starting at ${next.start}s. Overlapping clips on the same track cause rendering conflicts.`, - fixHint: - "Adjust data-start or data-duration so clips on the same track do not overlap, or move one clip to a different data-track-index.", - }); - } - } - } - - return findings; - }, - - // root_composition_missing_data_start - ({ rootTag, options }) => { - const findings: HyperframeLintFinding[] = []; - if (options.isSubComposition) return findings; - if (!rootTag) return findings; - const compId = readDecodedAttr(rootTag.raw, "data-composition-id"); - if (!compId) return findings; - const hasStart = readAttr(rootTag.raw, "data-start") !== null; - if (!hasStart) { - findings.push({ - code: "root_composition_missing_data_start", - severity: "error", - message: `Root composition "${compId}" is missing data-start. The runtime needs data-start="0" on the root element to begin playback.`, - fixHint: 'Add data-start="0" to the root composition element.', - snippet: truncateSnippet(rootTag.raw), - }); - } - return findings; - }, - // standalone_composition_wrapped_in_template ({ rawSource, options }) => { const findings: HyperframeLintFinding[] = []; diff --git a/packages/lint/src/rules/core.test.ts b/packages/lint/src/rules/core.test.ts index e4d7fc281..4015cf96d 100644 --- a/packages/lint/src/rules/core.test.ts +++ b/packages/lint/src/rules/core.test.ts @@ -708,4 +708,64 @@ describe("core rules", () => { expect(finding).toBeUndefined(); }); }); + + describe("non_deterministic_code — determinism is about execution, not text", () => { + const comp = (script: string) => ` + +
+ + +`; + + it("does not flag new Date() with a fixed timestamp", async () => { + // Deterministic, and the fixHint ("remove time-dependent code") cannot be + // applied without deleting the label the composition renders. + const result = await lintHyperframeHtml( + comp(`const label = new Date("2026-01-01T00:00:00Z").toISOString();`), + ); + expect(result.findings.find((f) => f.code === "non_deterministic_code")).toBeUndefined(); + }); + + it("does not flag non-deterministic APIs quoted inside a string literal", async () => { + // Code-display compositions render source they never execute. + const result = await lintHyperframeHtml(comp(`const SNIPPET = "const x = Math.random();";`)); + expect(result.findings.find((f) => f.code === "non_deterministic_code")).toBeUndefined(); + }); + + it("still flags a bare new Date()", async () => { + const result = await lintHyperframeHtml(comp(`const now = new Date();`)); + expect(result.findings.find((f) => f.code === "non_deterministic_code")).toBeDefined(); + }); + + it("still flags Math.random() in executed code", async () => { + const result = await lintHyperframeHtml(comp(`const r = Math.random();`)); + expect(result.findings.find((f) => f.code === "non_deterministic_code")).toBeDefined(); + }); + }); + + describe("timeline_id_mismatch — only top-level registry keys are composition ids", () => { + const comp = (script: string) => ` + +
+ + +`; + + it("does not flag the one-liner registration form", async () => { + // The inlined options object is not a registration. Reading `paused` as a + // composition id produced an error whose fixHint named a registration that + // did not exist, so it could never be applied. + const result = await lintHyperframeHtml( + comp(`window.__timelines = { main: gsap.timeline({ paused: true }) };`), + ); + expect(result.findings.find((f) => f.code === "timeline_id_mismatch")).toBeUndefined(); + }); + + it("still flags a genuinely mismatched id", async () => { + const result = await lintHyperframeHtml( + comp(`window.__timelines = { wrongid: gsap.timeline({ paused: true }) };`), + ); + expect(result.findings.find((f) => f.code === "timeline_id_mismatch")).toBeDefined(); + }); + }); }); diff --git a/packages/lint/src/rules/core.ts b/packages/lint/src/rules/core.ts index de31a2cfc..4ef3245f0 100644 --- a/packages/lint/src/rules/core.ts +++ b/packages/lint/src/rules/core.ts @@ -6,6 +6,7 @@ import { readDecodedAttr, truncateSnippet, stripJsComments, + stripStringLiterals, extractCompositionIdsFromCss, extractTimelineRegistryKeys, getInlineScriptSyntaxError, @@ -204,6 +205,7 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [ }, // root_missing_composition_id + root_missing_dimensions + // fallow-ignore-next-line complexity ({ rootTag }) => { const findings: HyperframeLintFinding[] = []; if (!rootTag || !readDecodedAttr(rootTag.raw, "data-composition-id")) { @@ -247,6 +249,7 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [ }, // missing_timeline_registry + timeline_registry_missing_init + // fallow-ignore-next-line complexity ({ source, rawSource, rootTag, options }) => { // Sub-compositions inherit window.__timelines from the host composition if (options.isSubComposition || rawSource.trimStart().toLowerCase().startsWith(" HyperframeLintFinding[]> = [ // non_deterministic_code ({ scripts }) => { const findings: HyperframeLintFinding[] = []; - const patterns: Array<{ pattern: RegExp; label: string; hint: string }> = [ + const patterns: Array<{ + pattern: RegExp; + label: string; + hint: string; + /** Match against raw source, because the value being matched is a string GSAP parses. */ + scansStrings?: boolean; + }> = [ { pattern: /Math\.random\s*\(/, label: "Math.random()", @@ -451,7 +460,10 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [ hint: "Remove time-dependent code. Use GSAP timeline position instead of wall-clock time.", }, { - pattern: /new\s+Date\s*\(/, + // Zero-arg only. `new Date()` is fully deterministic and is how + // a composition labels a fixed date on an axis or card; the hint ("remove + // time-dependent code") cannot be applied to it without deleting the label. + pattern: /new\s+Date\s*\(\s*\)/, label: "new Date()", hint: "Remove time-dependent code. Use GSAP timeline position instead of wall-clock time.", }, @@ -472,16 +484,25 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [ }, { // GSAP string form: "random(...)" / "+=random(...)" — re-rolls at tween init. + // `scansStrings` because here the string IS the executed value: GSAP parses it. + // Every other pattern above matches executable code, so a match inside a string + // literal is inert text and must not be reported. pattern: /["'`](?:[+-]=)?random\(\s*[-\d[]/, + scansStrings: true, label: '"random(...)" tween value', hint: "GSAP random string values re-roll at tween init and each render worker initializes independently. Use fixed values or precompute with a seeded PRNG.", }, ]; for (const script of scripts) { - const stripped = stripJsComments(script.content); - for (const { pattern, label, hint } of patterns) { - if (pattern.test(stripped)) { + const withoutComments = stripJsComments(script.content); + // Strings are content, not code. A composition that DISPLAYS source (the + // code-snippet blocks, /pr-to-video) carries `Math.random()` inside a string + // literal it never executes, and reported itself non-deterministic with no + // way to clear the error while still rendering the snippet. + const executable = stripStringLiterals(withoutComments); + for (const { pattern, label, hint, scansStrings } of patterns) { + if (pattern.test(scansStrings ? withoutComments : executable)) { findings.push({ code: "non_deterministic_code", severity: "error", diff --git a/packages/lint/src/rules/gsap.test.ts b/packages/lint/src/rules/gsap.test.ts index b39ca5d2f..33f3083c6 100644 --- a/packages/lint/src/rules/gsap.test.ts +++ b/packages/lint/src/rules/gsap.test.ts @@ -240,27 +240,6 @@ describe("GSAP rules", () => { ).toHaveLength(1); }); - it("errors when a full-frame transition flash uses a GSAP from reveal", async () => { - const html = ` - -
-

Scene 1

- - -`; - const result = await lintHyperframeHtml(html); - const finding = result.findings.find( - (f) => f.code === "gsap_fullscreen_overlay_starts_visible", - ); - expect(finding).toBeDefined(); - expect(finding?.selector).toBe("#tr-flash-1"); - }); - it("errors when a grouped GSAP selector targets a visible full-frame flash", async () => { const html = ` @@ -3051,4 +3030,44 @@ describe("SVG draw-on rules", () => { const finding = result.findings.find((f) => f.code === "svg_measure_before_path_d"); expect(finding).toBeUndefined(); }); + + describe("gsap_fullscreen_overlay_starts_visible — the from() shape is not a defect", () => { + const overlay = (style: string, script: string) => ` + +
+
+
+ + +`; + + it("does not flag a from() reveal, which already seats opacity 0 at t=0", async () => { + // This used to error, and BOTH its fixHints (authored CSS opacity:0, or an + // immediate gsap.set) produce gsap_from_opacity_noop — whose own fixHint says + // to remove exactly what was just added. Applying either hint looped forever. + const result = await lintHyperframeHtml( + overlay("", `tl.from("#flash", { opacity: 0, duration: 1 }, 2);`), + ); + expect( + result.findings.find((f) => f.code === "gsap_fullscreen_overlay_starts_visible"), + ).toBeUndefined(); + }); + + it("still flags an overlay that is revealed and later hidden again", async () => { + const result = await lintHyperframeHtml( + overlay( + "", + `tl.to("#flash", { opacity: 1, duration: 1 }, 2);\n tl.to("#flash", { opacity: 0, duration: 1 }, 5);`, + ), + ); + expect( + result.findings.find((f) => f.code === "gsap_fullscreen_overlay_starts_visible"), + ).toBeDefined(); + }); + }); }); diff --git a/packages/lint/src/rules/gsap.ts b/packages/lint/src/rules/gsap.ts index 05a56934a..c87d75e13 100644 --- a/packages/lint/src/rules/gsap.ts +++ b/packages/lint/src/rules/gsap.ts @@ -1181,10 +1181,30 @@ export const gsapRules: LintRule[] = [ ) || selectors[0] || tag.name; + // A window only re-hides the overlay if it is a DIFFERENT tween that ends + // hidden. Two exclusions matter: + // - `win !== firstVisible`: the reveal counted itself. + // - `method !== "from"`: a from-tween's recorded propertyValues are its + // START state. `from({opacity: 0})` ENDS visible, so reading those values + // as an end state made every from-reveal look like its own later hide. + // Together these are what made the `from` shape report at all. const laterHidden = visibilityWindows.some( - (win) => win.position >= firstVisible.position && isHiddenGsapState(win.propertyValues), + (win) => + win !== firstVisible && + win.method !== "from" && + win.position >= firstVisible.position && + isHiddenGsapState(win.propertyValues), ); - if (firstVisible.method !== "from" && !laterHidden) continue; + // Only the later-hidden shape is a real defect. The `from` shape is not: + // gsap.from() seats its start values immediately, so on a paused timeline the + // overlay already measures opacity 0 at t=0 and never covers an early frame. + // + // Worse, it had no exit. Both fixHints below (authored CSS `opacity: 0`, or an + // immediate `gsap.set`) turn a working composition into a real defect that + // `gsap_from_opacity_noop` correctly errors on — and that rule's fixHint says to + // remove the very thing we just asked for, closing the loop. An agent applying + // either hint bounces between the two errors forever. + if (!laterHidden) continue; reportedVisibleOverlayKeys.add(overlayKey); findings.push({ diff --git a/packages/lint/src/utils.ts b/packages/lint/src/utils.ts index 39b231538..4b665dcbe 100644 --- a/packages/lint/src/utils.ts +++ b/packages/lint/src/utils.ts @@ -46,7 +46,11 @@ const TIMELINE_REGISTRY_KEY_PATTERN = // The `window.__timelines = { ... }` object-literal body (group 1), captured so its // `key: value` entries can be scanned for registered keys. -const TIMELINE_REGISTRY_OBJECT_BODY_PATTERN = /window\.__timelines\s*=\s*\{([\s\S]*?)\}/i; +// Locates the START of a `window.__timelines = { ... }` literal. Deliberately does +// not try to match the closing brace: see readTimelineRegistryObjectBody, which walks +// braces instead. A regex cannot tell the registry's own `}` from the `}` of an +// inlined options object. +const TIMELINE_REGISTRY_OBJECT_OPEN_PATTERN = /window\.__timelines\s*=\s*\{/i; // A single object-literal entry whose value is an identifier (real timeline registration), // e.g. `main: tl` or `"comp-1": tl`. Captures the key in group 1 (quoted) or 2 (bare). const TIMELINE_REGISTRY_OBJECT_ENTRY_PATTERN = @@ -246,20 +250,71 @@ export function extractTimelineRegistryKeys(source: string): string[] { const key = match[1] ?? match[2]; if (key) keys.add(key); } - const objectBody = TIMELINE_REGISTRY_OBJECT_BODY_PATTERN.exec(source)?.[1]; - if (objectBody) { - const entryPattern = new RegExp( - TIMELINE_REGISTRY_OBJECT_ENTRY_PATTERN.source, - TIMELINE_REGISTRY_OBJECT_ENTRY_PATTERN.flags, - ); - while ((match = entryPattern.exec(objectBody)) !== null) { - const key = match[1] ?? match[2]; - if (key) keys.add(key); - } - } + for (const entry of readTimelineRegistryTopLevelKeys(source)) keys.add(entry); return [...keys]; } +/** + * Top-level keys of a `window.__timelines = { ... }` literal. + * + * Walks brace depth rather than regex-matching the body. The previous non-greedy + * body match stopped at the first `}` it saw, which for the legal one-liner + * + * window.__timelines = { main: gsap.timeline({ paused: true }) }; + * + * was the brace of the INLINED OPTIONS OBJECT. The entry scanner then harvested + * `paused` as a composition id and timeline_id_mismatch reported a timeline + * "registered as paused" — a registration that does not exist, so its fixHint + * could never be applied. Hoisting the timeline to a variable was the only escape, + * and nothing said so. + */ +/** Index of the brace that closes the group opened just before `bodyStart`. */ +function findMatchingBrace(source: string, bodyStart: number): number { + let depth = 1; + for (let i = bodyStart; i < source.length; i += 1) { + if (source[i] === "{") depth += 1; + else if (source[i] === "}" && (depth -= 1) === 0) return i; + } + return source.length; +} + +/** Replace every nested brace group with spaces so only depth-0 text remains. */ +function blankNestedBraceGroups(body: string): string { + let out = ""; + let depth = 0; + for (const ch of body) { + if (ch === "{") depth += 1; + else if (ch === "}") depth = Math.max(0, depth - 1); + else if (depth === 0) { + out += ch; + continue; + } + out += " "; + } + return out; +} + +function readTimelineRegistryTopLevelKeys(source: string): string[] { + const open = TIMELINE_REGISTRY_OBJECT_OPEN_PATTERN.exec(source); + if (!open) return []; + + const bodyStart = open.index + open[0].length; + const body = source.slice(bodyStart, findMatchingBrace(source, bodyStart)); + const flattened = blankNestedBraceGroups(body); + + const keys: string[] = []; + const entryPattern = new RegExp( + TIMELINE_REGISTRY_OBJECT_ENTRY_PATTERN.source, + TIMELINE_REGISTRY_OBJECT_ENTRY_PATTERN.flags, + ); + let entry: RegExpExecArray | null; + while ((entry = entryPattern.exec(flattened)) !== null) { + const key = entry[1] ?? entry[2]; + if (key) keys.push(key); + } + return keys; +} + export function getInlineScriptSyntaxError(source: string): string | null { if (!source.trim()) return null; try { @@ -272,6 +327,27 @@ export function getInlineScriptSyntaxError(source: string): string | null { } } +// fallow-ignore-next-line complexity +/** + * Blank the contents of every `'...'` and `"..."` literal, keeping the quotes so + * the source stays the same shape. + * + * Needed because a composition that *displays* source code carries things like + * `Math.random()` inside a string it never executes. Scanning raw script text for + * non-determinism reported those compositions as non-deterministic, and no edit + * could clear it while keeping the displayed snippet intact. + * + * Template literals are deliberately left alone: `${Math.random()}` inside one IS + * executed, and blanking it would hide real non-determinism. A snippet stored in a + * backtick string therefore still reports — a narrower gap than the one this closes. + */ +export function stripStringLiterals(source: string): string { + return source.replace( + /(['"])(?:\\.|(?!\1)[^\\\n])*\1?/g, + (literal) => literal[0] + " ".repeat(Math.max(0, literal.length - 1)), + ); +} + // fallow-ignore-next-line complexity export function stripJsComments(source: string): string { let out = "";