Skip to content

refactor(core): swap studio-api read path from recast to acorn parser (T6e)#1392

Open
vanceingalls wants to merge 3 commits into
06-12-feat_sdk_core_phase_3b_8_gsap_label_ops_setclassstylefrom
06-12-refactor_core_swap_studio-api_read_path_from_recast_to_acorn_parser_t6e_
Open

refactor(core): swap studio-api read path from recast to acorn parser (T6e)#1392
vanceingalls wants to merge 3 commits into
06-12-feat_sdk_core_phase_3b_8_gsap_label_ops_setclassstylefrom
06-12-refactor_core_swap_studio-api_read_path_from_recast_to_acorn_parser_t6e_

Conversation

@vanceingalls

@vanceingalls vanceingalls commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

What

Swaps the GSAP read path in packages/core/src/studio-api/routes/files.ts from the recast-backed parseGsapScript to the browser-safe parseGsapScriptAcorn introduced in T6b (#1368).

Changed call sites (4):

  • requireAnimation() helper — finds an animation by ID before a write op
  • delete-all-for-selector case — enumerates animations for bulk delete
  • GET /gsap-parse route — returns the parsed timeline to the studio UI
  • POST /gsap-mutations response — re-parses after a mutation and returns the updated state

Not changed: the lazy loadGsapParser() for write ops. The acorn writer (gsapWriterAcorn.ts, T6c/#1369) covers the 8 standard GSAP ops, but four write ops used by files.ts are not yet ported: convertToKeyframesInScript, removeAllKeyframesFromScript, materializeKeyframesInScript, unrollDynamicAnimations. loadGsapParser stays until those are ported.

Why

gsapParser.ts transitively imports recast and @babel/parser, both of which call require("fs") and cannot bundle for the browser. The acorn parser was built to eliminate this constraint. T6b implemented it; T6c wired the write path for SDK ops; this PR completes the adoption by moving the studio-api read path off recast. Without this, recast stays live indefinitely despite a complete replacement existing.

Stack

Test plan

  • All 76 core test files pass (1567 tests)
  • TypeScript build clean (bunx tsc --noEmit)
  • Studio UI: open a composition, GSAP panel loads animations correctly
  • Studio UI: mutate an animation (position, ease, duration) — response parses via acorn

Copy link
Copy Markdown
Collaborator Author

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean mechanical swap of the four read-path call sites from parseGsapScript (recast-backed) to parseGsapScriptAcorn (browser-safe acorn). Both return the same ParsedGsap shape from gsapSerialize.ts, so the downstream consumers (requireAnimation, delete-all-for-selector, GET /gsap-parse, POST /gsap-mutations response) are unaffected structurally.

The loadGsapParser() update comment (files.ts:291–299) correctly documents why the lazy loader stays: the four write ops (convertToKeyframesInScript, removeAllKeyframesFromScript, materializeKeyframesInScript, unrollDynamicAnimations) are not yet ported to the acorn writer. The boundary is clean.

All CI green — all 8 regression shards pass, all Perf:* jobs pass, Preflight / Preview parity / player-perf pass.

Verdict: APPROVE
Reasoning: Diff is correct, minimal, and well-scoped. CI all-green confirms no behavioral regression from the parser swap.

— magi

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hyperframes#1392refactor(core): swap studio-api read path from recast to acorn parser (T6e)

Looks good — cutover is clean, but two non-blocking gaps worth naming before this lands.

The diff is structurally tight: 4 call-site swaps in packages/core/src/studio-api/routes/files.ts (requireAnimation, delete-all-for-selector, GET /gsap-animations, POST /gsap-mutations re-parse). Recast stays loaded lazily for the 4 unported write ops (convertToKeyframesInScript, removeAllKeyframesFromScript, materializeKeyframesInScript, unrollDynamicAnimations) — appropriate boundary. The lint rule (packages/core/src/lint/rules/gsap.ts:174) intentionally continues to use recast since it's a server-side path; correct call.

The two correctness fixes folded into the swap commit (c0dcc1e) — lookupBindingFromAncestors program-scope fallback (gsapParserAcorn.ts:153-158) and fromTo arity guard (gsapParserAcorn.ts:475-499) — are well-targeted. The writer-side guards (valueToCode NaN→"0", safeKey ASCII regex tightening, removing the silent ID-rewrite fallback in removeAnimationFromScript) are also correct.

Concerns

packages/core/src/parsers/gsapParser.stress.test.ts — the 947-line stress suite (unicode selectors, NaN/Infinity, ASI/malformed scripts, deeply nested staggers, template literals, etc.) still runs only against recast. The T6d parity suite (gsapParserAcorn.full.test.ts) covers basic shapes well, but doesn't replicate the stress-test corpus. At cutover, the acorn parser's behavior on those edge cases is uncovered by automated parity. Suggest a small follow-up (T6f?) that points the stress suite at acorn — or at minimum, confirm whether the regression-shards currently in flight exercise any of those input shapes against the new read path.

• No feature flag on the swap. If a parse-shape regression hits studio prod, the rollback is "revert + deploy", which is a multi-hour window. For a parser swap of this surface this is defensible — the parity suite is real coverage — but worth saying out loud: there is no instant-rollback. If you wanted to add one in a follow-up (HF_USE_ACORN_READ_PATH env-flag check at the 4 call sites), the cost is small and the optionality is real.

Nits

packages/core/src/studio-api/routes/files.ts:454-455 — the executeGsapMutation destructure of parser no longer pulls parseGsapScript, but the variable name parser still suggests "the recast parser" to a reader. Optional rename to recastWriteOps would signal the shrinking surface. (nit)

packages/core/src/parsers/gsapParserAcorn.full.test.ts:6-12 — trust-model header is a nice add. One tweak: the note "motionPath parity tests live in the Phase 3b commit (PR #1379)" reads correctly today, but once #1379 lands and the commit reference becomes stale, future readers will hunt for it. A pointer to the test file path (or the describe("motionPath parsing") block at line 925 of this same file after #1379 lands) would age better. (nit)

Questions

• What's the concrete plan to port the 4 remaining write ops off recast? Once done, loadGsapParser and the /gsap-parser subpath disappear entirely. Is there a T6f / T6g lined up, or is that out-of-scope for this workstream and recast stays parked indefinitely for those 4 ops?

What I didn't verify

• Whether the regression-shards (gsap-letters-render-compat, style-N-prod shards, etc.) currently in flight on this PR HEAD actually exercise the studio-api /gsap-animations and /gsap-mutations routes against real production-shape scripts. If they do, the stress-coverage concern above shrinks meaningfully. If they're render-only, it doesn't change.

• Behavioral parity on the very specific edge cases the stress suite covers — I did not hand-trace acorn output for malformed-ASI or unicode-selector scripts against recast baseline. The T6d full suite is what gives me confidence here, not a hand-check.

Cross-PR context I did verify

• motionPath gap from HF#1370 R2 does NOT propagate to this PR — #1379 (which adds the acorn motionPath parser AND its parity tests) is a downstack ancestor of #1392 in the Graphite stack. By the time T6e lands, motionPath is parity-tested.

• Empty-IR-on-parse-error behavior is identical between recast (gsapParser.ts:1123) and acorn (gsapParserAcorn.ts:1134) — both silent-catch and return { animations: [], timelineVar: "tl", preamble: "", postamble: "" }. No new behavioral divergence at the cutover. The /gsap-animations UI consumer will continue to show "no animations" on parse failure (pre-existing, not introduced here).

• No sibling test hardcodes a parser-name list that would break on the swap. No feature-flag default-flip lurking in the diff.

Review by Rames D Jusso

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@vanceingalls vanceingalls force-pushed the 06-12-feat_sdk_core_phase_3b_8_gsap_label_ops_setclassstyle branch from 0417207 to 74fcd32 Compare June 12, 2026 22:24
vanceingalls and others added 3 commits June 12, 2026 15:25
- gsapParserAcorn: top-level variable targets now resolved via program-scope
  null-key fallback in lookupBindingFromAncestors (const el = querySelector...)
- gsapParserAcorn: fromTo guard requires args.length >= 3, preventing undefined
  args[2]/args[3] access when fewer args supplied
- gsapWriterAcorn: remove fuzzing fallback in removeAnimationFromScript that
  silently deleted the wrong animation (from→to ID conversion)
- gsapWriterAcorn: valueToCode guards NaN → "0" to avoid broken tween props;
  safeKey regex aligned to ASCII-only (matching gsapSerialize)
- mutate: handleSetGsapTween now includes stagger in extras (was in addGsapTween
  but missing from setGsapTween)
- apply-patches: script case now mirrors stylesheet — op=remove calls
  setGsapScript("") instead of silently ignoring the patch

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents the recast-baseline trust relationship and clarifies that
motionPath parity tests live in the Phase 3b commit (PR #1379) since
the acorn motionPath parser is also added there.

Addresses #1370 R1-N1 (Rames).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vanceingalls vanceingalls force-pushed the 06-12-refactor_core_swap_studio-api_read_path_from_recast_to_acorn_parser_t6e_ branch from a3ea6ad to 6cca945 Compare June 12, 2026 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants