Skip to content

fix(lint): stop gsap_repeated_fromto_without_baseline flagging a load-time gsap.set() baseline #3895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
miga-heygen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(lint): stop gsap_repeated_fromto_without_baseline flagging a load-time gsap.set() baseline #3895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 64 additions & 1 deletion packages/lint/src/rules/gsap.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3259,7 +3259,11 @@ describe("SVG draw-on rules", () => {
expect(finding).toBeUndefined();
});

it("gsap_repeated_fromto_without_baseline: rejects an earlier standalone set", async () => {
it("gsap_repeated_fromto_without_baseline: accepts an earlier load-time standalone set", async () => {
// A load-time `gsap.set(...)` runs before the paused timeline's playhead
// ever moves, so it establishes the resting state just as reliably as an
// in-timeline `tl.set(..., 0)` — and unlike that shape, it can never trip
// gsap_timeline_set_initial_hide (which already exempts `global` sets).
const html = `
<html><body>
<div data-composition-id="main" data-width="1920" data-height="1080"><div id="ring"></div></div>
Expand All @@ -3278,6 +3282,65 @@ describe("SVG draw-on rules", () => {
(candidate) => candidate.code === "gsap_repeated_fromto_without_baseline",
);

expect(finding).toBeUndefined();
});

it("gsap_repeated_fromto_without_baseline: a load-time set baseline also clears gsap_timeline_set_initial_hide", async () => {
// Regression: this rule's fixHint used to recommend an in-timeline
// `tl.set(sel, { ... }, 0)` baseline, which gsap_timeline_set_initial_hide
// then flags whenever those values hide the element — the hide-until-reveal
// shape both rules exist for. A load-time `gsap.set(...)` is the one
// baseline that satisfies both, so neither rule may fire on it.
const html = `
<html><body>
<div data-composition-id="main" data-width="1920" data-height="1080"><div id="ring"></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 });
gsap.set("#ring", { opacity: 0, scale: 0 });
tl.fromTo("#ring", { opacity: 0, scale: 0 }, { opacity: 1, scale: 1, duration: 0.5 }, 5);
tl.fromTo("#ring", { opacity: 1, scale: 1 }, { opacity: 0, scale: 0, duration: 0.5 }, 10);
window.__timelines.main = tl;
</script>
</body></html>`;
const result = await lintHyperframeHtml(html);
const codes = result.findings.map((finding) => finding.code);

expect(codes).not.toContain("gsap_repeated_fromto_without_baseline");
expect(codes).not.toContain("gsap_timeline_set_initial_hide");
});

it("gsap_repeated_fromto_without_baseline: still rejects a standalone set deferred behind a callback", async () => {
// A `gsap.set(...)` inside an event handler only runs on user interaction,
// not at load, so it cannot stand in for a resting state before the first
// tween. This rejects on the old, blunter grounds too (any `global` set
// was previously excluded outright) — it's a regression guard against a
// future change that accepts `global` unconditionally and drops the
// extractStandaloneSetSelectors reliability check, not a red/green proof
// of that check by itself (see the load-time-baseline test above for that).
const html = `
<html><body>
<div data-composition-id="main" data-width="1920" data-height="1080">
<div id="ring"></div><button id="reset"></button>
</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 });
document.getElementById("reset").addEventListener("click", () => {
gsap.set("#ring", { opacity: 0, scale: 1 });
});
tl.fromTo("#ring", { opacity: 0, scale: 1 }, { opacity: 1, duration: 0.5 }, 5);
tl.fromTo("#ring", { opacity: 1, scale: 0.4 }, { opacity: 0, duration: 0.5 }, 10);
window.__timelines.main = tl;
</script>
</body></html>`;
const result = await lintHyperframeHtml(html);
const finding = result.findings.find(
(candidate) => candidate.code === "gsap_repeated_fromto_without_baseline",
);

expect(finding?.severity).toBe("warning");
});

Expand Down
72 changes: 52 additions & 20 deletions packages/lint/src/rules/gsap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ function isHiddenGsapState(values: Record<string, string | number>): boolean {
);
}

function hiddenSetTargetSelectors(target: string, aliases: Map<string, string>): string[] {
function setTargetSelectors(target: string, aliases: Map<string, string>): string[] {
const parts =
target.startsWith("[") && target.endsWith("]") ? target.slice(1, -1).split(",") : [target];
return parts
Expand All @@ -258,7 +258,17 @@ function hiddenSetTargetSelectors(target: string, aliases: Map<string, string>):
.filter((selector) => selector.length > 0);
}

function extractStandaloneHiddenSelectors(script: string): Set<string> {
/**
* Selectors targeted by a load-time `gsap.set(...)`: one that runs as the
* script executes, not deferred behind a callback or event handler (IIFEs
* still count — they run at parse time). Callers that care about *what* the
* set applies pass `matchesValues` to filter on the object-literal source;
* callers that only need "is there a load-time set at all" omit it.
*/
function extractStandaloneSetSelectors(
script: string,
matchesValues?: (values: string) => boolean,
): Set<string> {
const selectors = new Set<string>();
const source = stripJsComments(script);
const functionRanges = collectFunctionBodyRanges(source);
Expand All @@ -274,16 +284,20 @@ function extractStandaloneHiddenSelectors(script: string): Set<string> {
while ((match = pattern.exec(source)) !== null) {
// Skip callback/handler bodies; keep IIFEs (they run at parse time).
if (indexInsideNonIifeRange(match.index, source, functionRanges)) continue;
const targets = hiddenSetTargetSelectors((match[1] ?? "").trim(), aliases);
const targets = setTargetSelectors((match[1] ?? "").trim(), aliases);
if (targets.length === 0) continue;
const body = match[2] ?? "";
if (/(?:opacity|autoAlpha)\s*:\s*0(?:\.0+)?\s*(?:,|$)/.test(body)) {
for (const selector of targets) selectors.add(selector);
}
if (matchesValues && !matchesValues(match[2] ?? "")) continue;
for (const selector of targets) selectors.add(selector);
}
return selectors;
}

function extractStandaloneHiddenSelectors(script: string): Set<string> {
return extractStandaloneSetSelectors(script, (values) =>
/(?:opacity|autoAlpha)\s*:\s*0(?:\.0+)?\s*(?:,|$)/.test(values),
);
}

function oneValue(
values: Record<string, string | number>,
keys: string[],
Expand Down Expand Up @@ -1133,6 +1147,12 @@ export const gsapRules: LintRule<LintContext>[] = [
const authoredHiddenSelectors = new Set(
scripts.flatMap((script) => [...extractStandaloneHiddenSelectors(script.content)]),
);
// Every selector with a load-time `gsap.set(...)`, whatever values it sets.
// gsap_repeated_fromto_without_baseline uses this below to tell a real
// load-time set apart from one buried in a callback.
const loadTimeSetSelectors = new Set(
scripts.flatMap((script) => [...extractStandaloneSetSelectors(script.content)]),
);

// Build clip element selector map
type ClipInfo = { tag: string; id: string; classes: string };
Expand Down Expand Up @@ -1216,16 +1236,26 @@ export const gsapRules: LintRule<LintContext>[] = [
const selector = firstFromTo.targetSelector;
const firstFromToIndex = gsapWindows.indexOf(firstFromTo);
const firstFromToPosition = Math.min(...fromToWindows.map((win) => win.position));
const hasTimelineBaseline = gsapWindows
.slice(0, firstFromToIndex)
.some(
(candidate) =>
candidate.method === "set" &&
!candidate.global &&
candidate.targetSelector === selector &&
candidate.position <= firstFromToPosition,
);
if (hasTimelineBaseline) continue;
// A load-time `gsap.set(...)` runs before the paused timeline's playhead
// ever moves, so it establishes the resting state just as reliably as an
// in-timeline `tl.set(..., 0)` — and gsap_timeline_set_initial_hide
// already treats it that way (it exempts `win.global` outright).
// Accepting it here too stops the two rules from each demanding the
// shape the other flags.
const hasBaseline = gsapWindows.slice(0, firstFromToIndex).some((candidate) => {
if (candidate.method !== "set" || candidate.targetSelector !== selector) return false;
// The acorn parser flags `global` on any bare `gsap.set(...)` whatever
// its AST nesting, so one deferred behind a callback/handler is
// indistinguishable here; only the load-time scan separates them.
// Known gap: the scan reports selectors, not occurrences, so a second,
// genuinely load-time `gsap.set` for the same selector ANYWHERE in the
// composition (even after this candidate, even in another script) would
// also satisfy this check — accepted as narrow (requires duplicate
// `gsap.set` calls to one selector) rather than tracked per-occurrence.
if (candidate.global) return loadTimeSetSelectors.has(selector);
return candidate.position <= firstFromToPosition;
});
if (hasBaseline) continue;

findings.push({
code: "gsap_repeated_fromto_without_baseline",
Expand All @@ -1237,9 +1267,11 @@ export const gsapRules: LintRule<LintContext>[] = [
`(immediateRender), not at tween position.`,
selector,
fixHint:
`Add \`immediateRender: false\` to the destination vars of each future fromTo, or set a safe ` +
`resting state with an earlier \`tl.set("${selector}", { ... }, 0)\`. Pre-first-tween seeks must not ` +
`inherit whichever fromTo call happened to author last.`,
`Add \`immediateRender: false\` to the destination vars of each future fromTo, or establish the ` +
`resting state with a load-time \`gsap.set("${selector}", { ... })\` before the first fromTo. ` +
`Prefer that over an in-timeline \`tl.set("${selector}", { ... }, 0)\`, which ` +
`gsap_timeline_set_initial_hide flags when those values hide the element. Pre-first-tween seeks ` +
`must not inherit whichever fromTo call happened to author last.`,
snippet: truncateSnippet(fromToWindows.map((win) => win.raw).join("\n")),
});
}
Expand Down
Loading