fix(lint): stop scene-exit hard-kill rules from contradicting gsap_animates_clip_element (#1846)

Two independent post-release feedback reports of the same contradiction:
scene_layer_missing_visibility_kill / gsap_exit_missing_hard_kill tell you to
add `tl.set(selector, { visibility: "hidden" }, t)` on an exiting scene
element, but when that element is also class="clip", the exact tl.set they
recommend is then flagged by gsap_animates_clip_element (the framework
already owns visibility/display on clip elements). One report worked around
it by wrapping the scene's content in an inner non-clip div and asked that
the fix hint mention that pattern.

Both rules now detect when the exiting/flagged selector is a clip element
(scene_layer_missing_visibility_kill checks the tag's class list directly;
gsap_exit_missing_hard_kill reuses the clipIds/clipClasses maps already built
in its enclosing rule) and, only in that case, point at the inner-wrapper
pattern instead of a tl.set on the clip element itself. Non-clip targets are
unaffected — same fix hint as before.
This commit is contained in:
Miguel Ángel
2026-07-02 17:45:12 -07:00
committed by GitHub
parent 8ee4b7dfda
commit c7b34d5c65
2 changed files with 80 additions and 4 deletions
+52
View File
@@ -1005,6 +1005,32 @@ describe("GSAP rules", () => {
expect(finding?.message).toContain("3.00s");
});
it("gsap_exit_missing_hard_kill points at the inner-wrapper pattern when the exiting selector is a clip element", async () => {
// Regression: a tl.set hard kill on a clip-classed selector is exactly what
// gsap_animates_clip_element then errors on — the two rules must not give
// contradictory advice for a crossfading scene that is itself class="clip".
const html = `
<html><body>
<div data-composition-id="c1" data-width="1920" data-height="1080" data-start="0" data-duration="6">
<div id="scene-a" class="clip" data-start="0" data-duration="3" data-track-index="0"></div>
<div id="scene-b" class="clip" data-start="3" data-duration="3" data-track-index="0"></div>
</div>
<script src="https://cdn.jsdelivr.net/npm/gsap@3/dist/gsap.min.js"></script>
<script>
window.__timelines = window.__timelines || {};
const tl = gsap.timeline({ paused: true });
tl.to("#scene-a", { opacity: 0, duration: 0.3 }, 2.7);
window.__timelines["c1"] = tl;
</script>
</body></html>`;
const result = await lintHyperframeHtml(html);
const finding = result.findings.find((f) => f.code === "gsap_exit_missing_hard_kill");
expect(finding).toBeDefined();
expect(finding?.fixHint).toContain("clip element");
expect(finding?.fixHint).toContain("inner");
expect(finding?.fixHint).not.toContain('tl.set("#scene-a"');
});
it("does NOT report gsap_exit_missing_hard_kill for an unresolved-target boundary exit", async () => {
// The exit tween targets an element via a value the parser cannot resolve (a helper
// call), so it collapses to the `__unresolved__` sentinel. You cannot assert a missing
@@ -1353,6 +1379,32 @@ describe("GSAP rules", () => {
expect(finding?.elementId).toBe("scene1");
});
it("scene_layer_missing_visibility_kill points at the inner-wrapper pattern when the scene element is a clip", async () => {
// Same contradiction as gsap_exit_missing_hard_kill above, via the older
// id-pattern-based rule: `tl.set("#scene1", { visibility: "hidden" }, ...)`
// on a class="clip" scene element is exactly what gsap_animates_clip_element
// then errors on.
const html = `
<html><body>
<div data-composition-id="c1" data-width="1920" data-height="1080">
<div id="scene1" class="clip"></div>
<div id="scene2" class="clip"></div>
</div>
<script>
window.__timelines = window.__timelines || {};
const tl = gsap.timeline({ paused: true });
tl.to("#scene1", { opacity: 0, duration: 0.5 }, 2.0);
window.__timelines["c1"] = tl;
</script>
</body></html>`;
const result = await lintHyperframeHtml(html);
const finding = result.findings.find((f) => f.code === "scene_layer_missing_visibility_kill");
expect(finding).toBeDefined();
expect(finding?.fixHint).toContain("clip element");
expect(finding?.fixHint).toContain("inner");
expect(finding?.fixHint).not.toContain('tl.set("#scene1"');
});
it("scene_layer_missing_visibility_kill: DOES fire when kill is only in a comment (stripJsComments guard)", async () => {
const html = `
<html><body>
+28 -4
View File
@@ -629,6 +629,20 @@ export const gsapRules: LintRule<LintContext>[] = [
);
if (hasHardKill) continue;
// A tl.set hard kill on the exiting selector itself is the fix — unless
// that selector IS a clip element, in which case gsap_animates_clip_element
// (below) errors on that exact tl.set: the framework already owns
// visibility/display on clip elements. Point at the inner-wrapper
// pattern instead so the two rules' advice doesn't contradict.
const exitClipInfo =
clipIds.get(win.targetSelector) || clipClasses.get(win.targetSelector);
const fixHint = exitClipInfo
? `"${win.targetSelector}" is a clip element — the framework already manages its visibility. ` +
"Wrap the scene's content in an inner non-clip <div>, move the exit tween and the hard kill " +
`(\`tl.set("<inner-selector>", ${hiddenStateLiteral(win.propertyValues)}, ${boundary.toFixed(2)})\`) onto that wrapper instead.`
: `Add \`tl.set("${win.targetSelector}", ${hiddenStateLiteral(win.propertyValues)}, ${boundary.toFixed(2)})\` ` +
"after the exit tween.";
findings.push({
code: "gsap_exit_missing_hard_kill",
severity: "error",
@@ -636,9 +650,7 @@ export const gsapRules: LintRule<LintContext>[] = [
`GSAP exit on "${win.targetSelector}" ends at the ${boundary.toFixed(2)}s clip start boundary ` +
"without a matching tl.set hard kill. Non-linear seeking can land after the fade and leave stale visibility state.",
selector: win.targetSelector,
fixHint:
`Add \`tl.set("${win.targetSelector}", ${hiddenStateLiteral(win.propertyValues)}, ${boundary.toFixed(2)})\` ` +
"after the exit tween.",
fixHint,
snippet: truncateSnippet(win.raw),
});
}
@@ -1014,6 +1026,18 @@ export const gsapRules: LintRule<LintContext>[] = [
const killPattern = new RegExp(`["']#${id}["'][^)]*visibility\\s*:\\s*["']hidden["']`);
const hasKill = killPattern.test(content);
if (!hasKill) {
// A tl.set on "#id" is only safe advice when the scene element isn't
// itself a clip — otherwise gsap_animates_clip_element errors on that
// exact tl.set, since the framework already owns visibility/display on
// clip elements. Point at the inner-wrapper pattern instead.
const classes = (readAttr(tag.raw, "class") || "").split(/\s+/).filter(Boolean);
const isClip = classes.includes("clip");
const fixHint = isClip
? `"#${id}" is a clip element — the framework already manages its visibility. ` +
"Wrap the scene's content in an inner non-clip <div>, move the exit tween and the hard kill " +
'(`tl.set("<inner-selector>", { visibility: "hidden" }, <exit-end-time>)`) onto that wrapper instead.'
: `Add \`tl.set("#${id}", { visibility: "hidden" }, <exit-end-time>)\` after the scene's exit tweens.`;
findings.push({
code: "scene_layer_missing_visibility_kill",
severity: "error",
@@ -1021,7 +1045,7 @@ export const gsapRules: LintRule<LintContext>[] = [
message:
`Scene layer "#${id}" exits via opacity tween but has no visibility: hidden hard kill. ` +
"When scrubbing or when tweens conflict, the scene may remain partially visible and overlap the next scene.",
fixHint: `Add \`tl.set("#${id}", { visibility: "hidden" }, <exit-end-time>)\` after the scene's exit tweens.`,
fixHint,
});
}
}