Skip to content

Commit 8708720

Browse files
vanceingallsclaude
andcommitted
fix(sdk): retire duplicate removeGsapKeyframe keyframeIndex variant (review #1498)
EditOp had two removeGsapKeyframe members with the same discriminant but different shapes (keyframeIndex vs percentage) — TS can't discriminate them and a handler could get the wrong shape. Per both reviewers (option 2): retire the keyframeIndex variant. It had no production caller (Studio dispatches percentage only); removed the dead by-index handleRemoveGsapKeyframe + simplified the dispatcher. resolveKeyframe stays (setGsapKeyframe still uses keyframeIndex). Converted the one by-index test to the percentage API. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent fb759b5 commit 8708720

3 files changed

Lines changed: 3 additions & 24 deletions

File tree

packages/sdk/src/engine/mutate.gsap.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,13 +400,13 @@ describe("setGsapKeyframe", () => {
400400
});
401401

402402
describe("removeGsapKeyframe", () => {
403-
it("removes keyframe at index 1 (50%)", () => {
403+
it("removes keyframe at 50%", () => {
404404
const parsed = fresh(KF_SCRIPT);
405405
const animId = `[data-hf-id="hf-box"]-to-0-visual`;
406406
const result = applyOp(parsed, {
407407
type: "removeGsapKeyframe",
408408
animationId: animId,
409-
keyframeIndex: 1,
409+
percentage: 50,
410410
});
411411
expect(result.forward).toHaveLength(1);
412412
const newScript = String(result.forward[0]?.value ?? "");

packages/sdk/src/engine/mutate.ts

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,7 @@ function dispatchRemoveGsapKeyframe(
150150
parsed: ParsedDocument,
151151
op: Extract<EditOp, { type: "removeGsapKeyframe" }>,
152152
): MutationResult {
153-
return "percentage" in op
154-
? handleRemoveGsapKeyframeByPercentage(parsed, op.animationId, op.percentage)
155-
: handleRemoveGsapKeyframe(parsed, op.animationId, op.keyframeIndex);
153+
return handleRemoveGsapKeyframeByPercentage(parsed, op.animationId, op.percentage);
156154
}
157155

158156
function applyGsapKeyframeOp(parsed: ParsedDocument, op: EditOp): MutationResult | undefined {
@@ -1003,24 +1001,6 @@ function handleRemoveGsapKeyframeByPercentage(
10031001
return gsapScriptChange(script, newScript);
10041002
}
10051003

1006-
function handleRemoveGsapKeyframe(
1007-
parsed: ParsedDocument,
1008-
animationId: string,
1009-
keyframeIndex: number,
1010-
): MutationResult {
1011-
const resolved = resolveKeyframe(parsed, animationId, keyframeIndex);
1012-
if (!resolved) return EMPTY;
1013-
const { script, kf, kfs } = resolved;
1014-
const pct = kf.percentage;
1015-
// removeKeyframeFromScript matches by percentage; bail if two keyframes share
1016-
// the same percentage to avoid removing the wrong one.
1017-
if (kfs.filter((k) => k.percentage === pct).length > 1) return EMPTY;
1018-
const newScript = removeKeyframeFromScript(script, animationId, pct);
1019-
if (newScript === script) return EMPTY;
1020-
setGsapScript(parsed.document, newScript);
1021-
return gsapScriptChange(script, newScript);
1022-
}
1023-
10241004
function handleAddLabel(parsed: ParsedDocument, name: string, position: number): MutationResult {
10251005
const script = getGsapScript(parsed.document);
10261006
if (!script) return EMPTY;

packages/sdk/src/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ export type EditOp =
101101
position: number;
102102
value: Record<string, unknown>;
103103
}
104-
| { type: "removeGsapKeyframe"; animationId: string; keyframeIndex: number }
105104
| { type: "removeGsapKeyframe"; animationId: string; percentage: number }
106105
| { type: "removeGsapProperty"; animationId: string; property: string; from?: boolean }
107106
| { type: "removeGsapTween"; animationId: string }

0 commit comments

Comments
 (0)