Skip to content

Commit 3bc46a8

Browse files
jrusso1020jrusso1020
authored
fix(studio-server): cascade GSAP cleanup when deleting subtrees (#3655)
1 parent ef63e02 commit 3bc46a8

5 files changed

Lines changed: 103 additions & 40 deletions

File tree

‎packages/parsers/src/htmlParser.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,19 @@ describe("removeElementFromHtml", () => {
580580
expect(updated).toContain('id="el2"');
581581
});
582582

583+
it("cascades DOM and stable ids for every descendant", () => {
584+
const html = `<!doctype html><html><body>
585+
<div id="parent"><div id="box" data-hf-id="hf-box"></div></div>
586+
<script>const tl = gsap.timeline();
587+
tl.to("#parent", { x: 10 }); tl.to("#box", { x: 20 });
588+
tl.to('[data-hf-id="hf-box"]', { x: 30 });
589+
</script></body></html>`;
590+
const updated = removeElementFromHtml(html, "parent");
591+
expect(updated).not.toContain("#parent");
592+
expect(updated).not.toContain("#box");
593+
expect(updated).not.toContain("hf-box");
594+
});
595+
583596
it("strips ALL gsap tweens for the removed element, not just the first", () => {
584597
// Two tweens on the same element → count-based ids renumber when the first is
585598
// removed, so a single up-front parse left the second tween orphaned.

‎packages/parsers/src/htmlParser.ts‎

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -725,41 +725,47 @@ export function addElementToHtml(
725725
};
726726
}
727727

728-
function selectorTargetsId(selector: string, id: string): boolean {
729-
return (
730-
selector === `#${id}` ||
731-
selector === `[data-hf-id="${id}"]` ||
732-
selector === `[data-hf-id='${id}']`
733-
);
728+
function elementSelectors(element: Element): string[] {
729+
const selectors: string[] = [];
730+
const id = element.getAttribute("id");
731+
const hfId = element.getAttribute("data-hf-id");
732+
if (id) selectors.push(`#${id}`);
733+
if (hfId) selectors.push(`[data-hf-id="${hfId}"]`, `[data-hf-id='${hfId}']`);
734+
return selectors;
734735
}
735736

736-
function stripGsapForId(script: string, elementId: string): string {
737-
// Re-parse after every removal. Animation ids are count-based (positional), so
738-
// removing one tween renumbers the survivors — ids captured from a single
739-
// up-front parse go stale and silently no-op, orphaning later tweens on the
740-
// now-deleted element. Always remove the FIRST still-matching animation in a
741-
// freshly-parsed script until none remain.
742-
let current = script;
743-
for (;;) {
744-
const parsed = parseGsapScriptAcornForWrite(current);
745-
if (!parsed) return current;
746-
const match = parsed.located.find((l) =>
747-
selectorTargetsId(l.animation.targetSelector, elementId),
748-
);
749-
if (!match) return current;
750-
const updated = removeAnimationFromScript(current, match.id);
751-
// Guard against a non-removing match (would otherwise loop forever).
752-
if (updated === current) return current;
753-
current = updated;
754-
}
755-
}
737+
/** Remove a source subtree and its unambiguous, directly targeted GSAP tweens. */
738+
export function removeElementWithGsapCascade(doc: Document, element: Element): void {
739+
const removedSelectors = new Set(elementSelectors(element));
740+
walkCompositionDescendants(element, (child) => {
741+
for (const selector of elementSelectors(child)) removedSelectors.add(selector);
742+
});
743+
element.remove();
744+
745+
// Bare selectors can target repeated sub-composition instances. Keep a tween
746+
// if any surviving element still uses its selector rather than erasing the
747+
// surviving instance's animation along with the deleted subtree.
748+
walkCompositionDescendants(doc, (survivor) => {
749+
for (const selector of elementSelectors(survivor)) removedSelectors.delete(selector);
750+
});
751+
if (removedSelectors.size === 0) return;
756752

757-
function cascadeRemoveGsapById(doc: Document, elementId: string): void {
758753
for (const script of findScriptElementsDeep(doc)) {
759-
const text = script.textContent ?? "";
760-
if (!text.includes("gsap") && !text.includes("ScrollTrigger")) continue;
761-
const updated = stripGsapForId(text, elementId);
762-
if (updated !== text) script.textContent = updated;
754+
let current = script.textContent ?? "";
755+
if (!current.includes("gsap") && !current.includes("ScrollTrigger")) continue;
756+
// Writer ids are positional: reparse after each removal so later tweens
757+
// cannot be skipped after an earlier deletion renumbers them.
758+
for (;;) {
759+
const parsed = parseGsapScriptAcornForWrite(current);
760+
const match = parsed?.located.find((located) =>
761+
removedSelectors.has(located.animation.targetSelector),
762+
);
763+
if (!match) break;
764+
const updated = removeAnimationFromScript(current, match.id);
765+
if (updated === current) break;
766+
current = updated;
767+
}
768+
if (current !== script.textContent) script.textContent = current;
763769
}
764770
}
765771

@@ -771,8 +777,8 @@ export function removeElementFromHtml(html: string, elementId: string): string {
771777
"removeElementFromHtml: input HTML is empty or could not be parsed",
772778
);
773779
}
774-
doc.getElementById(elementId)?.remove();
775-
cascadeRemoveGsapById(doc, elementId);
780+
const element = doc.getElementById(elementId);
781+
if (element) removeElementWithGsapCascade(doc, element);
776782
return "<!DOCTYPE html>\n" + doc.documentElement.outerHTML;
777783
}
778784

‎packages/studio-server/src/helpers/sourceMutation.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,55 @@ describe("removeElementFromHtml", () => {
2828
expect(updated).toContain(`data-composition-id="scene-b"`);
2929
});
3030

31+
it("removes tweens for both DOM ids and stable ids throughout the deleted subtree", () => {
32+
const html = `<!doctype html><html><body>
33+
<div id="parent"><div id="box" data-hf-id="hf-box"><span id="leaf"></span></div></div>
34+
<div id="keep"></div>
35+
<script>
36+
const tl = gsap.timeline({ paused: true });
37+
tl.to("#parent", { x: 10 });
38+
tl.to("#box", { x: 20 });
39+
tl.to('[data-hf-id="hf-box"]', { x: 30 });
40+
tl.to("#leaf", { x: 40 });
41+
tl.to("#box", { x: 50 });
42+
tl.to("#keep", { x: 60 });
43+
</script></body></html>`;
44+
const updated = removeElementFromHtml(html, { id: "parent" });
45+
expect(updated).not.toContain("#parent");
46+
expect(updated).not.toContain("#box");
47+
expect(updated).not.toContain("hf-box");
48+
expect(updated).not.toContain("#leaf");
49+
expect(updated).toContain('tl.to("#keep", { x: 60 })');
50+
});
51+
52+
it("cascades a stable-id deletion into nested composition template scripts", () => {
53+
const html = `<div id="box" data-hf-id="hf-box"></div>
54+
<template data-composition-id="outer"><template data-composition-id="inner">
55+
<script>const tl = gsap.timeline(); tl.to("#box", { x: 10 });</script>
56+
</template></template>`;
57+
const updated = removeElementFromHtml(html, { hfId: "hf-box" });
58+
expect(updated).not.toContain("#box");
59+
expect(updated).not.toContain('id="box"');
60+
});
61+
62+
it("retains shared selectors used by a surviving composition instance", () => {
63+
const html = `<div data-hf-id="remove"><span id="box" data-hf-id="hf-box"></span></div>
64+
<template data-composition-id="keep"><div id="box" data-hf-id="hf-box"></div>
65+
<script>const tl = gsap.timeline();
66+
tl.to("#box", { x: 10 }); tl.to('[data-hf-id="hf-box"]', { x: 20 });
67+
</script>
68+
</template>`;
69+
const updated = removeElementFromHtml(html, { hfId: "remove" });
70+
expect(updated).not.toContain('data-hf-id="remove"');
71+
expect(updated).toContain('tl.to("#box", { x: 10 })');
72+
expect(updated).toContain(`tl.to('[data-hf-id="hf-box"]', { x: 20 })`);
73+
});
74+
75+
it("does not strip scripts when the requested element is absent", () => {
76+
const html = `<script>const tl = gsap.timeline(); tl.to("#missing", { x: 10 });</script>`;
77+
expect(removeElementFromHtml(html, { id: "missing" })).toBe(html);
78+
});
79+
3180
it("supports fragment html by returning updated body markup", () => {
3281
const html = `<div id="photo"></div><div id="rest"></div>`;
3382

‎packages/studio-server/src/helpers/sourceMutation.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { parseHTML } from "linkedom";
2+
import { removeElementWithGsapCascade } from "@hyperframes/parsers";
23
import postcss from "postcss";
34
import selectorParser from "postcss-selector-parser";
45
import { isAllowedHtmlAttribute, isSafeAttributeValue } from "@hyperframes/core/html-attr-safety";
@@ -132,7 +133,7 @@ export function removeElementFromHtml(source: string, target: SourceMutationTarg
132133
const element = findTargetElement(document, target);
133134
if (!element) return source;
134135

135-
element.remove();
136+
removeElementWithGsapCascade(document, element);
136137
return wrappedFragment ? document.body.innerHTML || "" : document.toString();
137138
}
138139

‎packages/studio/src/hooks/useElementLifecycleOps.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,6 @@ export function useElementLifecycleOps({
180180
}
181181
const patchedContent =
182182
typeof removeData.content === "string" ? removeData.content : originalContent;
183-
// ponytail: the server remove-element route (removeElementFromHtml) strips
184-
// only the element node — it does NOT cascade-remove GSAP tweens targeting
185-
// it, unlike the SDK path (removeElement → cascadeRemoveAnimations). This
186-
// fallback runs only when the element isn't in the SDK doc (e.g. runtime-
187-
// generated / unaddressable), where targeting tweens are unlikely. Upgrade
188-
// path: cascade in removeElementFromHtml by selector/hf-id to fully match.
189183
await saveProjectFilesWithHistory({
190184
projectId: pid,
191185
label: "Delete element",

0 commit comments

Comments
 (0)