mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-08 02:36:10 +00:00
fix(skills,lint): correct composition-contract claims the code contradicts (#3468)
The runtime absorbed a series of authoring mistakes over time and `runtime/init.ts` says so in its own comments, but the skills kept teaching the old rules. Four of them actively cost an agent a failing run: add `crossorigin` (lint rejects it unconditionally), never build a timeline inside `async` (lint calls that the documented contract), never `gsap.set` later-scene clips (two fixHints instruct exactly that), and 12 copyable media snippets with no `id`, which render silent. Corrected in every place each claim appeared, including `hyperframes-animation`, three workflow scripts, the scaffolded project instructions, the CLI `docs` command, and the public docs site: `data-track-index` is a Studio display lane the render never reads, `class="clip"` is a layout convention rather than a visibility requirement, timed elements may nest, the visibility window is half-open, sub-composition host dimensions are backfilled, and the root-fill rule applies only to the layered-composite path. Behaviour changes, each backed by a render rather than by reading code: - `timeline_registry_missing_init` deleted. The runtime creates the registry before any inline script; a composition without the guard line renders and animates correctly. - `video_nested_in_timed_element` kept, message corrected. A rendered repro shows the nested-with-local-start case really does break, so the rule guards a real defect, but nothing is "FROZEN": the extractor ignores the wrapper's offset while visibility uses it, so the clip shows wrong frames and then vanishes. - `mediaRenderIds` now stamps media whose source is a `<source>` child, closing a duplicate-id gap the old `[src]`-only selector left open. - Stale messages fixed on `subcomposition_root_styled_by_class` and `deprecated_data_layer`. `coreSkillContent.test.ts` pinned the literal sentence that made root `data-start` look required, so it is narrowed to structure plus the regression it genuinely catches. Not covered, and flagged in the PR: the media global-vs-local start heuristic in `runtime/init.ts` is the root cause behind the nested-video defect. Removing it changes the meaning of existing compositions and needs its own deprecation.
This commit is contained in:
Miguel Ángel
@@ -432,7 +432,8 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding
|
||||
severity: "error",
|
||||
message: `<${tag.name}${elementId ? ` id="${elementId}"` : ""}> uses data-layer instead of data-track-index.`,
|
||||
elementId,
|
||||
fixHint: "Replace data-layer with data-track-index. The runtime reads data-track-index.",
|
||||
fixHint:
|
||||
"Replace data-layer with data-track-index, which is the canonical name Studio and the linter read. Neither name is read by the render.",
|
||||
snippet: truncateSnippet(tag.raw),
|
||||
});
|
||||
}
|
||||
@@ -935,10 +936,13 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding
|
||||
// can't leak styles into each other. A rule whose LEFTMOST selector is the ROOT
|
||||
// element's own class (e.g. `.frame { ... }` on the same element that carries
|
||||
// data-composition-id) therefore becomes a DESCENDANT selector that can never
|
||||
// match the root — the whole scene renders unstyled (tiny text top-left, images
|
||||
// at natural size). lint/validate/inspect evaluate the file in isolation (no
|
||||
// scoping) and Studio previews each scene in its own iframe (no scoping), so the
|
||||
// break is invisible until the composited MP4 render. Style the root via `#root`
|
||||
// match the SCOPED element itself. NOTE on the symptom: since #1886 the producer
|
||||
// preserves the authored root as a `data-hf-inner-root` wrapper INSIDE the scoped
|
||||
// element (regression fixture packages/producer/tests/sub-comp-class-selector),
|
||||
// so the class still matches as a descendant and the scene no longer renders
|
||||
// unstyled. This rule is now a consistency constraint, not a render-bug guard:
|
||||
// `#root` is the shape the registry blocks model and the one the scoper
|
||||
// special-cases. Style the root via `#root`
|
||||
// (the scoper special-cases the root id) and descendants via plain selectors,
|
||||
// like the registry blocks — the runtime already scopes each scene by id, so a
|
||||
// class namespace on the root is redundant.
|
||||
@@ -960,10 +964,10 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding
|
||||
severity: "error",
|
||||
message:
|
||||
`Root element has class="${rootClasses.join(" ")}" and is styled by ${offenders.length} rule(s) keyed off that class (e.g. ${example}). ` +
|
||||
`At render, every sub-composition rule is scoped to [data-composition-id="${rootCompositionId}"] <selector>, so a selector whose leftmost part is the ROOT's own class becomes a descendant selector that cannot match the root — the scene renders unstyled (tiny text top-left, full-size images). ` +
|
||||
`lint/validate/inspect and Studio's per-frame iframe preview do not scope, so this passes every static check and looks correct in preview.`,
|
||||
`At render, every sub-composition rule is scoped to [data-composition-id="${rootCompositionId}"] <selector>, so a selector whose leftmost part is the ROOT's own class becomes a descendant selector that cannot match the scoped element itself. ` +
|
||||
`Since #1886 the producer preserves the authored root as an inner wrapper, so this no longer renders the scene unstyled, but #root is the shape the scoper special-cases and the registry blocks model. Use it so preview, render, and Studio agree.`,
|
||||
selector: example,
|
||||
fixHint: `Give the root id="root" and style it with \`#root { ... }\` plus plain descendant selectors (\`.kicker\`, \`#hero\`) — the runtime already scopes each sub-composition by data-composition-id, so a class namespace on the root is redundant and breaks under scoping.`,
|
||||
fixHint: `Give the root id="root" and style it with \`#root { ... }\` plus plain descendant selectors (\`.kicker\`, \`#hero\`) — the runtime already scopes each sub-composition by data-composition-id, so a class namespace on the root is redundant.`,
|
||||
snippet: truncateSnippet(rootTag.raw),
|
||||
},
|
||||
];
|
||||
|
||||
@@ -227,7 +227,15 @@ describe("core rules", () => {
|
||||
expect(finding).toBeDefined();
|
||||
});
|
||||
|
||||
it("reports error when timeline registry is assigned without initializing", async () => {
|
||||
// The runtime creates `window.__timelines` at script-evaluation time
|
||||
// (runtime/entry.ts), before any inline composition script runs, so a bare
|
||||
// assignment needs no `window.__timelines = window.__timelines || {}` guard.
|
||||
// Verified by rendering a composition whose only registration is the bare
|
||||
// assignment: it renders and animates correctly. The old
|
||||
// `timeline_registry_missing_init` error therefore failed a working file, and
|
||||
// because a lint ERROR also suppresses the layout and contrast audits in
|
||||
// `check`, it cost far more than the line it asked for.
|
||||
it("accepts a bracket registry assignment with no init guard", async () => {
|
||||
const html = `
|
||||
<html><body>
|
||||
<div id="root" data-composition-id="c1" data-width="1920" data-height="1080">
|
||||
@@ -240,13 +248,13 @@ describe("core rules", () => {
|
||||
</script>
|
||||
</body></html>`;
|
||||
const result = await lintHyperframeHtml(html);
|
||||
const finding = result.findings.find((f) => f.code === "timeline_registry_missing_init");
|
||||
expect(finding).toBeDefined();
|
||||
expect(finding?.severity).toBe("error");
|
||||
expect(finding?.message).toContain("without initializing");
|
||||
expect(
|
||||
result.findings.find((f) => f.code === "timeline_registry_missing_init"),
|
||||
).toBeUndefined();
|
||||
expect(result.findings.find((f) => f.code === "missing_timeline_registry")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("reports error when dot timeline registry is assigned without initializing", async () => {
|
||||
it("accepts a dot registry assignment with no init guard", async () => {
|
||||
const html = `
|
||||
<html><body>
|
||||
<div id="root" data-composition-id="c1" data-width="1920" data-height="1080">
|
||||
@@ -259,9 +267,10 @@ describe("core rules", () => {
|
||||
</script>
|
||||
</body></html>`;
|
||||
const result = await lintHyperframeHtml(html);
|
||||
const finding = result.findings.find((f) => f.code === "timeline_registry_missing_init");
|
||||
expect(finding).toBeDefined();
|
||||
expect(finding?.severity).toBe("error");
|
||||
expect(
|
||||
result.findings.find((f) => f.code === "timeline_registry_missing_init"),
|
||||
).toBeUndefined();
|
||||
expect(result.findings.find((f) => f.code === "missing_timeline_registry")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not flag timeline assignment when init guard is present", async () => {
|
||||
|
||||
@@ -248,7 +248,7 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
|
||||
];
|
||||
},
|
||||
|
||||
// missing_timeline_registry + timeline_registry_missing_init
|
||||
// missing_timeline_registry
|
||||
// fallow-ignore-next-line complexity
|
||||
({ source, rawSource, rootTag, options }) => {
|
||||
// Sub-compositions inherit window.__timelines from the host composition
|
||||
@@ -269,19 +269,17 @@ export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
|
||||
fixHint: "Register each composition timeline on `window.__timelines[compositionId]`.",
|
||||
});
|
||||
}
|
||||
if (
|
||||
TIMELINE_REGISTRY_ASSIGN_PATTERN.test(source) &&
|
||||
!TIMELINE_REGISTRY_INIT_PATTERN.test(source)
|
||||
) {
|
||||
findings.push({
|
||||
code: "timeline_registry_missing_init",
|
||||
severity: "error",
|
||||
message:
|
||||
"`window.__timelines[…] = …` is used without initializing `window.__timelines` first.",
|
||||
fixHint:
|
||||
"Add `window.__timelines = window.__timelines || {};` before any timeline assignment.",
|
||||
});
|
||||
}
|
||||
// `timeline_registry_missing_init` used to fire here, demanding
|
||||
// `window.__timelines = window.__timelines || {}` before any assignment.
|
||||
// The runtime already owns that invariant: runtime/entry.ts creates the
|
||||
// registry at script-evaluation time, before any inline composition script
|
||||
// runs, and both injection paths put the runtime bundle in <head> ahead of
|
||||
// the body scripts that build timelines. Verified by rendering a
|
||||
// composition whose only registration is a bare
|
||||
// `window.__timelines["main"] = gsap.timeline(...)`: it renders and
|
||||
// animates correctly. The rule made a working file fail lint, and a lint
|
||||
// ERROR also suppresses the layout and contrast audits in `check`, so it
|
||||
// cost far more than the line it was protecting.
|
||||
return findings;
|
||||
},
|
||||
|
||||
|
||||
@@ -411,10 +411,10 @@ export const mediaRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> =
|
||||
findings.push({
|
||||
code: "video_nested_in_timed_element",
|
||||
severity: "error",
|
||||
message: `<video> with data-start is nested inside <${parent.name}${parent.id ? ` id="${parent.id}"` : ""}> which also has data-start. The framework cannot manage playback of nested media — video will be FROZEN in renders.`,
|
||||
message: `<video> with data-start is nested inside <${parent.name}${parent.id ? ` id="${parent.id}"` : ""}> which also has data-start. The frame extractor resolves the video's start from its own data-start without the wrapper's offset, while visibility uses the wrapper's window, so the two disagree: the clip shows the wrong source frames and then disappears partway through its slot.`,
|
||||
elementId: readAttr(tag.raw, "id") || undefined,
|
||||
fixHint:
|
||||
"Move the <video> to be a direct child of the stage, or remove data-start from the wrapper div (use it as a non-timed visual container).",
|
||||
"Time the wrapper OR the video, never both: remove data-start from the wrapper (use it as a non-timed visual container), or move the <video> up to be a direct child of the stage.",
|
||||
snippet: truncateSnippet(tag.raw),
|
||||
});
|
||||
break;
|
||||
|
||||
Reference in New Issue
Block a user