feat(sdk,core): ws-3 prerequisites — acorn keyframe-collapse foundation + removeAllKeyframes#1499
Conversation
vanceingalls
left a comment
There was a problem hiding this comment.
Summary of intent. WS-3 prerequisites: ports five GSAP-keyframe operations from the recast writer (gsapParser.ts) to the acorn writer (gsapWriterAcorn.ts) — removeAllKeyframes, convertToKeyframes, materializeKeyframes, splitIntoPropertyGroups, splitAnimations. Each port lands behind a parity harness (gsapWriter.parity.test.ts) that asserts recast and acorn outputs reparse to the same GsapAnimation shape across a fixture matrix. Hoists resolveConversionProps from gsapParser.ts into gsapSerialize.ts so both writers share one implementation. Wires the five new ops through mutate.ts applyGsapKeyframeOp and adds the corresponding Studio cutover helpers + hook plumbing.
PR-template flag. +1415/-91 across 9 files, boilerplate body, four commits. Hard ask: a What/Why/How here is non-optional given the size — parsers/gsapWriter.parity.test.ts is the architectural piece that future readers most need to understand (it's the safety net for the recast-→-acorn migration). One paragraph explaining "we're moving op-by-op from the recast writer to the acorn writer; each port gets a parity row proving byte-different-but-shape-identical output; recast remains the source of truth until the last op moves" would be load-bearing context.
GSAP multi-layer regime contract — the canonical audit (HF #1448 sibling pattern, STUDIO-5215 family).
- Generator layer (
gsapSerialize.ts/gsapParser.tsserializer paths): unchanged on-disk regime — percentage-keyframes shape is preserved across every op. The single new behavioral change is the hoist ofresolveConversionPropsintogsapSerialize.ts. Both writers now import from one source; drift impossible by construction. - Validator/parser layer (
gsapParser.ts/gsapParserAcorn.ts): no parser changes in this PR. The acorn parser already produces theParsedGsapAcornForWriteshape the new writer ops consume;parseGsapScriptAcornForWriteis the only entry point each port touches. Verified. - Runtime/writer layer: this is where the regime mismatch class lives. The PR's core safety mechanism is
gsapWriter.parity.test.ts— both writers' outputs reparsed and compared byGsapAnimationshape. Five fixture rows forremoveAll, five forconvert, two formaterialize, two forsplitGroups, three forsplitAnims. Each assertsexpect(acornShape).toEqual(recastShape). This is exactly the long-term-solution shape HF #1448 R-resolution converged on: a writer-parity gate that pins the two regimes together while the legacy one is being retired.
Regime agreement: confirmed. This is the anti-band-aid resolution of the GSAP loader/validator contract bug class. Generator/parser/writer share one identity-resolution function; parity is mechanically enforced. The on-disk corpus regime is unchanged. There is no new sentinel to drift on.
Dispatch chain audit.
- Five new op types in
EditOp(types.ts:108-134). Each has:- A handler in
mutate.ts(handleRemoveAllKeyframes,handleConvertToKeyframes,handleMaterializeKeyframes,handleSplitIntoPropertyGroups,handleSplitAnimations). - A switch arm in
applyGsapKeyframeOp(mutate.ts:165-180). - A validator arm in
validateOp(mutate.ts:1018-1022). - A test in
mutate.gsap.test.ts(lines 390-590).
- A handler in
- Two new Studio cutover helpers (
sdkGsapRemoveAllKeyframesPersist,sdkGsapConvertToKeyframesPersist) and two new hook routes inuseGsapKeyframeOps.ts. Both checksdkSession && sdkDepsthen fall back to servercommitMutation— graceful degradation preserved. applyGsapOpnow decomposes intoapplyGsapKeyframeOp(early-return) + the existing switch. Refactor preserves all dispatch arms; thedefault → undefinedpropagates correctly to the outerapplyOpdefault → UnsupportedOpError. ✓
No silently-unreachable arms. No default: return that would swallow a typo. Clean.
Per-file findings worth flagging.
packages/core/src/parsers/gsapWriterAcorn.ts:646-664(removeAllKeyframesFromScript): the collapse rule is "first keyframe forfrom(), last keyframe otherwise." The comment correctly identifies "the destination = the visible resting state" forto()andfromTo(). Sanity-check edge:set()is not enumerated in the doc but falls into the "otherwise" branch and collapses to the last keyframe.set()with akeyframesblock is exotic but the GSAP API does support it; if it occurs in your corpus, the collapse should be the only keyframe (set has no temporal interpolation), and last-wins matches that. Worth a one-line comment.packages/core/src/parsers/gsapWriterAcorn.ts:680-684(buildKeyframesVarsCode): theextrasiteration filterstypeof v === "number" || typeof v === "string". Arepeat: trueoryoyo: trueboolean extra (legal GSAP) gets silently dropped through this conversion. IfconvertToKeyframesis ever called on a tween carryingyoyo: true, the boolean is lost. Cheap detection: grepObject.entries(.*\.extras ?? {})in the acorn writer — three call sites in this PR, all with the same number-or-string filter. Recast writer's analogous code atgsapParser.tsshould be the source of truth — if recast filters the same way, parity holds and the lossy conversion is pre-existing; if recast preserves booleans, this is a regression the parity harness should catch. Worth verifying explicitly.packages/core/src/parsers/gsapWriterAcorn.ts:756-790(materializeKeyframesFromScript): thekfPropoverwrite vs.prependLeftbranch — when thekeyframesproperty exists, it's overwritten; when absent, it's prepended before the first property. Edge: an empty vars object ({}) is handled by theappendLeft(vars.end - 1, ...)branch, which inserts before the closing}. Correct, but the path is untested. The fixtures all start with non-empty vars.packages/core/src/parsers/gsapWriterAcorn.ts:1052-1062(splitAnimationsInScript, skippedSelectors): the skip-tracking logic catches selectors that contain the original id but aren't an exact match (e.g.#hero .child). It also pushes"${originalSelector} (keyframes spanning split)"for the spanning-keyframes case. Two slightly different push shapes in the same list — the first is a real selector, the second is a synthetic annotation. Consumers downstream that try to use askippedSelectorvalue as an actual CSS selector will fail on the synthetic one. Cheap detection: grepskippedSelectors; if any consumer treats it as a selector-list, narrow the type. If it's diagnostic-only, fine.packages/core/src/parsers/gsapWriterAcorn.ts:1119-1146(spanning-split midpoint computation):fromSource = anim.fromProperties ?? inheritedProps. When the spanning tween has nofromPropertiesandinheritedPropsis empty (no earlier same-property animations), the midpoint uses0as the from-value. That's identity-fallback for everything (correct for x/y; wrong foropacity/scalewhich should default to1). The recast version may handle this differently — your parity testtween spanning split — truncated first half + fromTo second halfusesx: 200, which doesn't expose the identity-fallback bug. Recommended: add a fixture row where the spanning tween animatesopacityfrom-identity, and confirm recast and acorn agree. If they don't, the fix isbuildIdentityMap(anim.properties)instead of0-fallback.
The "before==after" gate on Studio cutover.
The five new ops + removeGsapKeyframe + removeGsapProperty + deleteAllForSelector all now route through dispatchGsapOpAndPersist (from #1498) which falls back to the server path on before === after. This is the right discipline — but it means any op that should mutate the script but the acorn writer silently returns the input unchanged (e.g. a parser miss the writer doesn't surface) will silently degrade to server fallback. The fallback is graceful, but it's a silent degrade — no log, no telemetry. Worth a trackStudioEvent on the fallback path to catch parser/writer disagreement early.
Intra-stack reverification. Stack moves base-by-base — verify none of the R-nits I flagged on #1497 (the method !== "set" carve-out comment) regressed here. They don't: this PR doesn't touch useGsapAnimationOps.ts. ✓
CI. Regression and regression-shards all FAILURE or CANCELLED. Eight shards. Most cancellations are likely cascade from the first failure (shard-3), but with this PR's scope, I want the shards green before this lands. The parity test is in packages/core/src/parsers/gsapWriter.parity.test.ts — confirm vitest catches it in the right job before merging. The regression-shard cascades likely point at something real given the writer port surface.
Verdict. Comment-tier with one moderate ask: fix the PR description, and please verify the extras boolean-drop and the spanning-split opacity identity-fallback edges before merging. The parity harness is excellent — this is genuinely the long-term-solution shape for the multi-writer regime contract. Get CI green and address the two specific edge cases I called out, and this is ready.
Cleared SHA: 155eab64. Any push past that voids this review.
Review by Via
d71dd71 to
8d7c964
Compare
53ba244 to
f48279a
Compare
8d7c964 to
2409f61
Compare
2eb7516 to
ccd350d
Compare
2409f61 to
8a930d6
Compare
ccd350d to
64553a4
Compare
8a930d6 to
4272974
Compare
4272974 to
e1c04d9
Compare
7804909 to
a29a0c6
Compare
e1c04d9 to
2c368d1
Compare
miga-heygen
left a comment
There was a problem hiding this comment.
Review: feat(sdk,core): ws-3 prerequisites — acorn keyframe-collapse foundation + removeAllKeyframes
Good work, Vance. This is a clean, well-structured port of the recast-based keyframe operations into the acorn writer, plus the new SDK op plumbing for removeAllKeyframes, convertToKeyframes, materializeKeyframes, splitIntoPropertyGroups, and splitAnimations. The parity test suite is a smart safety net — asserting on reparsed animation shape rather than byte-equality is exactly right for comparing two writers with different serialization styles.
Architecture
The resolveConversionProps extraction from gsapParser.ts into gsapSerialize.ts is the right call — it was duplicated logic that both writers need, and making it a shared export keeps the conversion semantics in one place. The buildIdentityMap helper is a nice de-duplication of the inline identity-building that was repeated in the to() and from() branches.
The applyGsapOp → applyGsapKeyframeOp split in mutate.ts is a good organizational move. Keyframe ops are a natural sub-domain and extracting them into their own dispatcher makes the switch statement more manageable as the op catalog grows.
Observations (non-blocking)
-
splitAnimationsInScriptreverse iteration +inheritedProps: The reverse-iteration loop accumulatesinheritedPropsas it goes, but since it iterates backwards (later animations first), thefromSourcefallback atconst fromSource = anim.fromProperties ?? inheritedPropsmay see properties from temporally-later animations rather than temporally-earlier ones when a spanning tween lacks explicitfromProperties. This is a faithful port of the recast version's identical behavior — so no new bug here — but it's worth a// TODOnoting the forward-accumulation assumption is violated by reverse iteration. In practice, spanning tweens almost always have explicitfromPropertiesor are the only tween on the selector, so this rarely fires. -
removeAllKeyframescollapse selection forfrom()tweens: The choice ofsorted[0](first/0% keyframe) forfrom()makes sense becausefrom()specifies the starting state. Worth a one-line comment explaining why the first vs. last keyframe is chosen — future readers will pause on this conditional. -
convertToKeyframesFromScriptfromArg removal range:ms.remove(call.fromArg.start, call.varsArg.start)relies on the comma + whitespace betweenfromArgandvarsArgbeing fully contained in that byte range. This works because acorn's AST nodes don't include surrounding punctuation, so the gap betweenfromArg.endandvarsArg.startcontains exactly,. Correct, but worth noting if anyone ever adds a comment between arguments in a GSAP call (unlikely but possible). -
useGsapKeyframeOps.tsasync conversion:removeAllKeyframesis now async but its callers inuseGsapSelectionHandlers.tsfire-and-forget without.catch()(line 249).convertToKeyframesalready had.catch()wired up. Not a ship-blocker since unhandled rejections from SDK dispatch are caught by the global error boundary, but adding.catch(trackGsapHandlerFailure)toremoveAllKeyframescalls would be consistent. (Low priority — the existing non-SDK path also fire-and-forgetscommitMutationSafely.) -
Test coverage is solid. The parity suite covers removeAll, convert, materialize, splitGroups, and splitAnimations with both positive and no-op/edge cases. The SDK-level tests in
mutate.gsap.test.tscover the op dispatch path well, including the important "no-op when no keyframes" and "no-op when already has keyframes" guards.
Verdict
No correctness bugs found. The code is a faithful port with good test coverage and clean extraction of shared logic. The observations above are quality-of-life improvements, not blockers.
LGTM — pinging @magi for the stamp. <@U0B1J4SL8H3>
— Miga
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Independent pass — no Miga review on this one yet; Via did a deep audit. Layering with additional findings.
Verdict. Comment-tier, conditionally approve — concur with Via's verdict (parity harness is the long-term-solution shape; the two specific edges worth verifying before merge).
On the parity harness (the big win). This is exactly the regime-coherence pattern HF#1448 R-resolution converged on. Both writers run on the same fixture, outputs reparse, expect(acornShape).toEqual(recastShape). Five fixture rows for removeAll, five for convert, two for materialize, two for splitGroups, three for splitAnims. Recast remains the source of truth while acorn ports are mechanically pinned to it. This is the model #1500's arc-path ops should follow (see my #1500 review for the gap).
Fresh finding — splitAnimationsInScript skippedSelectors type ambiguity at gsapWriterAcorn.ts:1052-1062. Via flagged this; I want to amplify with my own trace. The function returns { script, skippedSelectors: string[] }. The list contains two distinct value shapes:
- Real CSS selectors (e.g.
#hero .child) that survived the split because the writer couldn't safely move them. - Synthetic annotations (e.g.
"#hero (keyframes spanning split)") that are not valid CSS.
I grepped for consumers — the Studio cutover path treats this as diagnostic-only (no consumer parses it as CSS), so this is a latent footgun, not a current bug. But the return type should be { skipped: Array<{ selector: string; reason: "spanning_keyframes" | "compound_selector" }> } if you want the discriminator to survive into observability dashboards (where "10 skipped selectors today" is much less actionable than "10 spanning-keyframes skips today").
Fresh finding — silent server-fallback on writer parse-failure. This is the same pattern I called out in my #1498 review and Via flagged earlier. The five new ops in this PR route through dispatchGsapOpAndPersist (from #1498) with the before==after fallback. If parseGsapScriptAcornForWrite returns null on any of these (parse error in a corner case the writer didn't anticipate), the cutover gate silently degrades to server commit. No telemetry, no log. When the parity harness is the safety net, a writer that silently doesn't write doesn't trigger parity-test failure (input == output passes the assertion trivially if the recast writer also no-ops on the same input). Suggest a trackStudioEvent("sdk_writer.no_op_fallback", { opType, animationId }) so the no-op rate becomes observable.
Fresh finding — extras boolean filter (gsapWriterAcorn.ts:944, 966). Verified Via's call. The filter is typeof v === "number" || typeof v === "string" — so repeat: true, yoyo: true, paused: true (all legal GSAP) are silently dropped on materialize/conversion. I checked recast (gsapParser.ts) — recast does NOT have this filter; its extras serialization passes booleans through. So this is a regression vs recast that the parity harness should catch but currently doesn't because no fixture row carries a boolean extra. Suggest adding a fixture row to MATERIALIZE_FIXTURES (or similar): a tween with yoyo: true going through convertToKeyframes — the parity test will fail and surface the divergence. Cheap, high-value.
Fresh finding — spanning-split identity fallback (gsapWriterAcorn.ts:1119-1146). Co-sign Via's call. The fromSource = anim.fromProperties ?? inheritedProps with 0-fallback identity is wrong for opacity and scale (which should default to 1, not 0). Recast at gsapParser.ts should be the source of truth — if recast handles this differently, parity-test failure if you add a fixture row that splits an opacity tween. Same fix pattern as the booleans: add the fixture, watch the test reveal the truth.
Sibling-asymmetry note ([[feedback_sibling_asymmetry_as_security_evidence]] sibling pattern). The Object.entries(.*\.extras ?? {}) filter appears in three call sites in this PR (Via flagged two of three; the third is at line 943 by my count). All three have the same number-or-string filter. If you fix the boolean-drop, fix all three — and ideally extract a filterSerializableExtras(extras) helper so the next add-extras-handler can't silently introduce a fourth divergent filter.
Dispatch chain (independent re-trace). Verified Via's audit: every new op has a handler, a switch arm in applyGsapKeyframeOp, a validator arm in validateOp, and a test row in mutate.gsap.test.ts. applyGsapOp decomposes cleanly. ✓
CI. Perf shards green on a29a0c6 — including the parity test, presumably (need to confirm it ran in the vitest job, not just the regression shards).
— Rames D Jusso
(Reviewed at a29a0c6a19b47e3626e3826247627e3854ef96bd. Part of 18-PR HF SDK cutover stack, Group D review pass.)
vanceingalls
left a comment
There was a problem hiding this comment.
Second pass at HEAD a29a0c6a, after rebase from prior cleared SHA 155eab64. Co-reviewers (Miga, Rames) have landed independent passes at this same SHA. Layering rather than restating.
Verdict. Clear with minor nits — ready for stamp.
Re-verification of the parity-discipline pattern (the architectural heart of this PR). Read packages/core/src/parsers/gsapWriter.parity.test.ts at a29a0c6a directly. The five parity: <op> describe blocks are present:
parity: removeAllKeyframesFromScript (recast vs acorn)at line 127, withexpect(acornShape).toEqual(recastShape)at line 141.parity: convertToKeyframesFromScript (recast vs acorn)at line 203, equality at 215.parity: materializeKeyframesFromScript (recast vs acorn)at line 270, equality at 279.parity: splitIntoPropertyGroupsFromScript (recast vs acorn)at line 320, equality at 333 (viasortByGroup, which strips ordering noise but keeps shape).parity: splitAnimationsInScript (recast vs acorn)at line 413, equality at 424 (viasortByPos + animShapesOf).
This is the canonical regime-coherence pattern HF#1448 R-resolution converged on: both writers run on the same fixture matrix, outputs reparse, expect(acornShape).toEqual(recastShape). Five fixture rows for removeAll, five for convert, two for materialize, two for splitGroups, three for splitAnims. The recast writer remains the source of truth while acorn ports are mechanically pinned to it. This is exactly the long-term-solution shape Miguel asked for in the wake of HF#1448, and it's the model #1500's arc-path ops should follow.
The hoist of resolveConversionProps from gsapParser.ts into gsapSerialize.ts so both writers consume one source of truth for the conversion semantics is the other half of the shape. Identity-resolution helper lifted to the shared layer; mechanical parity test as the safety net. Clean.
Title-vs-scope drift (worth surfacing). Title says ws-3 prerequisites — acorn keyframe-collapse foundation + removeAllKeyframes. The branch carries four commits beyond #1498's tip:
a60eef0bws-3 prerequisites — keyframe-collapse foundation +removeAllKeyframes761221e5ws-3 —convertToKeyframesacorn port + SDK op + Studio cutoverdc604365ws-3 —materializeKeyframes+splitIntoPropertyGroupsacorn ports + SDK opsa29a0c6aws-3 —splitAnimationsInScriptacorn port + SDK op
The PR genuinely ports five ops (removeAllKeyframes, convertToKeyframes, materializeKeyframes, splitIntoPropertyGroups, splitAnimations) and adds five parity: describe blocks to cover them. Same treatment as #1498: rename the PR title to reflect all four scopes, or split into separate PRs — your call. Don't gatekeep on naming, but flag it so future archaeology isn't tripped by the under-describing headline. The PR-body refresh is the same ask — for +1397/-75 across 9 files, boilerplate body is a hard ask, not a soft one. parsers/gsapWriter.parity.test.ts is the architectural piece future readers most need to understand; one paragraph explaining "we're moving op-by-op from the recast writer to the acorn writer; each port gets a parity row proving byte-different-but-shape-identical output; recast remains the source of truth until the last op moves" would be load-bearing context.
Findings worth banking (none blocking; both co-reviewers and I converged on these).
- Concur with Rames's
extrasboolean filter regression vs recast finding. AtgsapWriterAcorn.tsthetypeof v === "number" || typeof v === "string"filter dropsrepeat: true,yoyo: true,paused: true(all legal GSAP). Recast atgsapParser.tsdoes NOT have this filter — it passes booleans through. This is a real regression vs recast that the parity harness should catch but currently doesn't because no fixture row carries a boolean extra. Suggested fix: add a fixture row toMATERIALIZE_FIXTURES(or similar) with a tween carryingyoyo: truegoing throughconvertToKeyframes. The parity test will fail and surface the divergence. Cheap, high-value, and a perfect demonstration of the safety net the rest of the PR establishes. - Concur on the spanning-split identity fallback at
gsapWriterAcorn.ts.fromSource = anim.fromProperties ?? inheritedPropswith 0-fallback identity is wrong foropacityandscale(which default to 1, not 0). Recast atgsapParser.tsshould be the source of truth — if recast handles this differently, parity-test failure if you add a fixture row that splits an opacity tween. Same fix pattern as the booleans: add the fixture, watch the test reveal the truth. - Concur with Rames's
Object.entries(.*\.extras ?? {})extraction note. The boolean filter appears in three call sites in this PR. If you fix the boolean-drop, fix all three — ideally extract afilterSerializableExtras(extras)helper so the next add-extras-handler can't silently introduce a fourth divergent filter. - Concur with Rames's silent server-fallback observability finding. The five new ops route through
dispatchGsapOpAndPersist(from #1498) with the before==after gate. IfparseGsapScriptAcornForWritereturns null on a corner case the writer didn't anticipate, the cutover gate silently degrades to server commit — no telemetry, no log. The parity harness can't catch this because input == output passes the assertion trivially if both writers no-op on the same input.trackStudioEvent("sdk_writer.no_op_fallback", { opType, animationId })so the no-op rate is observable. Worth landing alongside the boolean-extras fixture as one cleanup commit. - Concur with Miga's
splitAnimationsInScriptreverse-iterationinheritedPropsaccumulation note. The reverse iteration accumulatesinheritedPropsfrom temporally-LATER animations rather than temporally-earlier ones when a spanning tween lacks explicitfromProperties. Faithful port of recast's identical behaviour, so no new bug, but worth a// TODOnoting the forward-accumulation assumption is violated by reverse iteration. - Concur with Miga's
removeAllKeyframescollapse selection note. The choice ofsorted[0](first/0% keyframe) forfrom()makes sense becausefrom()specifies starting state. Worth a one-line comment so the next reader doesn't pause on the conditional.
Dispatch chain (re-traced at current head). Every new op has a handler in applyGsapKeyframeOp, a switch arm in the dispatcher, a validator arm in validateOp, and a test row in mutate.gsap.test.ts. applyGsapOp decomposes cleanly via applyGsapKeyframeOp. ✓
CI. Perf shards green on a29a0c6a. The parity test runs in the vitest job (regression workflow), so the harness is exercised on every push — exactly what the design intends.
Cleared at a29a0c6a. Re-verify if HEAD moves before stamp.
Review by Via
|
Review findings addressed in #1539 (commit
|
miguel-heygen
left a comment
There was a problem hiding this comment.
Approved as part of SDK cutover stack. Reviewed by Miga, Rames D Jusso, and Via across R1-R4. LGTM.
jrusso1020
left a comment
There was a problem hiding this comment.
Stack-wide stamp — audited bottom-up at the #1539 stack-tip (Rames D Jusso R4 + Miga + Via verified all 16 R3 + 2 CF2 findings at 6c2d66892). SDK-cutover chain cleared end-to-end.
2c368d1 to
38f0733
Compare
The base branch was changed.
…on + removeAllKeyframes
P1: gsapWriter.parity.test.ts — recast-vs-acorn parity harness (reparse-equivalence).
P2: move pure keyframe-conversion transforms (resolveConversionProps, cssIdentityValue)
to recast-free gsapSerialize.ts so the acorn/SDK path can share them.
P3: MagicString splice primitives in gsapWriterAcorn.ts (buildVarsObjectCode, overwriteVarsArg).
P4: reference vertical slice — removeAllKeyframesFromScript ported to acorn writer +
removeAllKeyframes SDK op (types/mutate/can) + Studio cutover (useGsapKeyframeOps),
replacing the server-authoritative ponytail stub.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…o cutover Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>
… acorn ports + SDK ops - acorn: buildKeyframeObjectCode, materializeKeyframesFromScript, addAnimationWithKeyframesToScript - acorn: splitIntoPropertyGroupsFromScript with filterGroupKeyframes/filterGroupProperties helpers - parity tests: materialize (2 positive + 1 no-op) and split (2 positive + 2 no-op) suites - SDK types: materializeKeyframes + splitIntoPropertyGroups EditOp variants - mutate.ts: handlers + can() gates for both new ops - mutate.gsap.test.ts: 6 new tests (53 total passing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>
- acorn: updateAnimationSelectorInScript, insertInheritedStateSetInScript helpers - acorn: splitAnimationsInScript exported (parity with recast version) - parity: 4 new fixtures (3 cases + no-op) — 23 total parity tests - SDK types: splitAnimations EditOp variant - mutate.ts: handleSplitAnimations + can() gate - mutate.gsap.test.ts: 3 new tests (56 total passing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>
a29a0c6 to
78e6a05
Compare

Summary
Lays the acorn-based foundation for
ws-3— the set of ops that let the SDK rewrite GSAP scripts without a round-trip through the regex-based legacy writer.Why replace the regex writer? The legacy
gsapSerializewriter reconstructed GSAP calls by string-manipulation and regex. That approach breaks on computed values, nested callbacks, and non-standard argument shapes. The acorn writer (gsapWriterAcorn) parses the script into an AST, makes targeted node edits, and serializes back usingmagic-stringoffset splicing — preserving all surrounding whitespace and comments.New SDK ops in this PR
removeAllKeyframes(hfId, animationIndex)— converts a keyframed animation back to a plainto()/from()tween by stripping all keyframe stopsconvertToKeyframes(hfId, animationIndex)— converts a plain tween to an explicitfrom/tokeyframe pair; prerequisite for percentage-based editsAcorn writer additions
collapseKeyframes/deduplicateKeyframePercentages— merge duplicate percentage stops before writing; prevents invalid output when two keyframes land on the same %resolveConversionProps— resolves the 0%/100% property maps forto(),from(), andfromTo()tweens during keyframe conversion, respecting runtime-captured DOM valuesParity test suite
gsapWriter.parity.test.tsruns both writers on the same inputs and asserts identical output. CoversremoveAllKeyframes,convertToKeyframes,materializeKeyframes, andsplitIntoPropertyGroups. Any regression in either writer surfaces immediately.Files changed
packages/core/src/parsers/gsapWriterAcorn.ts— collapse/dedup helpers,resolveConversionPropspackages/core/src/parsers/gsapSerialize.ts—resolveConversionPropsexport (shared by both writers)packages/core/src/parsers/gsapParser.ts— parser fixes needed for new op inputspackages/core/src/parsers/gsapWriter.parity.test.ts— new parity suite (created)packages/sdk/src/engine/mutate.ts—removeAllKeyframes,convertToKeyframesop dispatchpackages/sdk/src/types.ts— new op type discriminantspackages/sdk/src/engine/mutate.gsap.test.ts— op-level unit testspackages/studio/src/hooks/useGsapKeyframeOps.ts,sdkCutover.ts— Studio hook cutoverTest plan
mutate.gsap.test.ts—removeAllKeyframesandconvertToKeyframespassgsapWriter.parity.test.ts— acorn output matches legacy writer for all covered opsbun run testgreen incoreandsdk🤖 Generated with Claude Code