Skip to content

refactor(studio): compile the GSAP and gesture hooks under the React Compiler - #3784

Draft
miguel-heygen wants to merge 1 commit into
refactor/studio-c2a1-compiler-hooks-afrom
refactor/studio-c2a2-compiler-hooks-a
Draft

refactor(studio): compile the GSAP and gesture hooks under the React Compiler#3784
miguel-heygen wants to merge 1 commit into
refactor/studio-c2a1-compiler-hooks-afrom
refactor/studio-c2a2-compiler-hooks-a

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Lands unit C2a, second half (hooks) of the Studio React Compiler coverage plan. Stacked on the first half. Ratchet 115 to 104; no src/hooks entry remains. About 190 lines over the usual size guideline: a relocated commit body accounts for most of it, unedited.

What

The remaining eleven hooks under packages/studio/src/hooks, useElementPicker.ts through useGsapTweenCache.ts in path order, now compile under the React Compiler. The per-file bail-out baseline drops from 115 to 104. No hook under src/hooks bails out any more.

Two files move with them: useDomEditWiring.ts, which calls the two hooks whose parameter changed, and useDomEditSession.ts, which loses the one argument that became dead.

Three new test files and three added cases pin the paths whose structure changed.

Stacked on refactor/studio-c2a1-compiler-hooks-a, which takes the first eleven.

Why

react({ compiler: true }) is silent about what it declines. A hook the compiler skips is emitted exactly as written, so losing every memo inside it costs nothing you can see in a diff, a build log, or a test run.

How

One row per file: the cause the scan reported, the shape of the fix, and the test that holds it.

File Cause Fix Test that pins it
useElementPicker.ts two refs written during render KTD2a: options and the active-iframe alias are refreshed in effects; the iframe read is now after attach, where it is valid new useElementPicker.test.tsx (3 cases incl. the zoomed-frame override and the newest options)
useExternalFileChangeCoordinator.ts ref written during render KTD2a: written in an effect; every reader is an async callback useExternalFileChangeCoordinator.test.tsx, externalFileChangeOwnership.test.ts
useFileManager.ts two refs written during render; memo not preserved KTD2a for the refs. The optional-chained dependency is named first, so the declared list matches the inferred one useFileManager.projectOwnership.test.tsx, useEditorSave.test.tsx
useFrameCapture.ts try/finally, three throws in try/catch KTD3: the capture moves to a module function that returns the message instead of throwing new useFrameCapture.test.tsx (4 cases: download, server message, status fallback, save-queue failure)
useGestureCommit.ts ++ on a module binding; try/finally KTD3: the commit moves to a module function whose finally is the caller's cleanup. The counter gets a module-scope accessor useGestureCommit.test.tsx + new case: the recording is released when the commit rejects
useGestureRecording.ts ref read during render KTD2c: the two alias refs start empty instead of seeded from another ref; both are re-pointed the moment a recording starts useGestureRecording.test.tsx + new assertion on the live alias mid-gesture
useGsapAwareEditing.ts ++ on a module binding; pattern in a for..of init Module-scope accessor for the counter; the destructure moves into the loop body useGsapAwareEditing.test.tsx, gsapDragCommit.test.ts (33 cases)
useGsapPropertyDebounce.ts ref written during render KTD2a: written in an effect; every reader is a debounce timer or an unmount flush useGsapPropertyDebounce.test.ts, useGsapPropertyDebounceFlush.test.ts
useGsapScriptCommits.ts ref written during render KTD2a: written in an effect; every reader is an async commit callback useGsapScriptCommits.test.tsx
useGsapSelectionHandlers.ts ref written during render KTD2a: the last-selection memory is written in an effect useGsapSelectionHandlers.test.tsx + new case: an edit after the selection clears still targets the last element
useGsapTweenCache.ts two react-hooks suppressions KTD4: see below new useGsapAnimationsForElement.test.tsx (3 cases)

The one that needed more than a local edit

Removing useGsapTweenCache's two suppressions uncovered what they were hiding: two memos and an effect resolving the live preview DOM through a ref parameter, which can never be a true dependency, which is why the omission had to be suppressed to stand.

Both hooks now take the iframe element the caller already holds as state. The reads are of a value, the dependency lists are true without a suppression, and a replaced iframe re-resolves instead of waiting for the next cache-version bump. The preview document is also tagged with the composition generation that produced it, which is what the extra version dependency was standing in for: a soft reload swaps the document inside the same element, so the element alone cannot say the DOM changed.

That left one argument dead in useDomEditWiring, removed here along with the line that passed it.

A third bail-out surfaced underneath (++ on a variable an interval callback captures) and moved to a module-scope poll helper.

Three notes on fix shapes, measured against this toolchain

  • A shared useLatestRef helper does not work. The compiler recognises a ref by the useRef call site, not by the shape of what a helper returns, so a hook reading .current from a helper's return inside useCallback([]) loses its memoization instead of gaining it. The write stays inline in each hook.
  • A getter-backed alias object does not work either. get in an object expression is a lowering the compiler declines.
  • .catch() and .finally() on a promise are fine where try/catch and try/finally statements are not, which is what keeps several of these fixes inside their hooks instead of moving 200 lines to module scope.

Test plan

  • Bail-out ratchet: bunx vitest run src/styles/compilerBailouts.test.ts --maxWorkers=4, 14 passed, baseline 115 down to 104, exactly these eleven entries removed. No src/hooks entry remains.
  • Full Studio suite: bunx vitest run --maxWorkers=4, 436 passed, 1 skipped (437 files); 4784 passed, 18 todo. The branch below is 433 files and 4772 tests; this adds 3 files and 12 cases.
  • bun run typecheck, clean.
  • oxlint and oxfmt --check on the 19 changed source files, from the repo root, clean.
  • fallow audit --base origin/main --fail-on-issues, passes. Two complexity exemptions move with the code they were already granted for; the three remaining findings are inherited, in functions this stack does not touch.
  • CI's studio-load-smoke command, run locally on the combined tree: PASS: studio loaded with schema-valid API fixtures and no runtime errors.
  • CI's studio-timeline-viewport gate, both arms, run locally on the combined tree: 5/5 passing runs each. Virtualization on: interaction p95 32.4ms, frame interval p95 16.7ms. Virtualization off: 32.8ms / 16.8ms. Budget 75ms.
  • Render parity: the design-shot computed-style capture, taken before and after on the same commit of the capture script, table byte-identical.
  • Every new test was watched failing with its fix reverted, and every relocated finally or catch was watched failing with that clause broken on purpose.

Not covered

  • Behaviour is unchanged by construction: every fix moves where a value is read, never what happens. The one place that is not literally true is useElementPicker's active-iframe alias, which is now refreshed after commit rather than during render, so it reads the iframe after React has attached it instead of before.
  • useElementPicker is exported from the package root and its own signature is unchanged. The two hooks whose parameter changed are internal to this package.
  • The 104 remaining bail-outs are other units' files: the app root, contexts, the inspector, the player and timeline canvas, and the sidebar and panels. Untouched here.
  • No hook-level test existed for useGsapTweenCache before this branch, so the new one is the first thing pinning its live-DOM resolution. It covers attribution through the preview document, not the runtime keyframe enrichment below it.
  • The three CI-gate and render-parity runs above were done on the combined tree before the split. Splitting only partitions the same file changes across two commits; it does not change what renders.
  • This diff is a little over the usual cap. Roughly 450 of the lines are one function relocated to module scope without an edit, and another 300 are new tests.

The second half of the hooks folder, same causes and same rule: fix where a
value is read, never what happens. Ref writes move into an effect or drop
when the ref was standing in for a memo, guarded control flow moves to a
plain function or a promise handler.

useGsapTweenCache needed more than a local edit. Removing its two react-hooks
suppressions uncovered what they were hiding: two memos and an effect
resolving the live preview DOM through a ref parameter, which can never be a
true dependency. Both hooks now take the iframe element the caller already
holds as state, so the reads are of a value and the dependency lists are true
without a suppression. The one argument that leaves dead behind it is removed
from its caller.

The bail-out baseline falls from 115 to 104, one per file. No hook under
src/hooks bails out any more.
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.

1 participant