Skip to content

refactor(studio): compile the remaining hooks under the React Compiler - #3777

Draft
miguel-heygen wants to merge 2 commits into
refactor/studio-c1-compiler-bailout-ratchetfrom
refactor/studio-c2b-compiler-hooks-b
Draft

refactor(studio): compile the remaining hooks under the React Compiler#3777
miguel-heygen wants to merge 2 commits into
refactor/studio-c1-compiler-bailout-ratchetfrom
refactor/studio-c2b-compiler-hooks-b

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Lands unit C2b (hooks, second half) of the Studio React Compiler coverage plan. Stacked on the ratchet PR (#3773). Ratchet 126 to 104. The stack diff shown here includes C1 until it lands.

What

The second half of the Studio hooks now compile under the React Compiler, with their bail-out ratchet entries at zero.

Baseline total: 126 to 104. Twenty-two files, each 1 to 0:

File Cause the scan reported Fix Test that pins it
hooks/useLintModal.ts try/finally KTD3: the catch never rethrows, so the finally body becomes the statement after the try/catch new useLintModal.test.tsx, 3 cases, watched failing with the flag reset reachable on one path only
hooks/useLivePlayheadTime.ts refs during render (write on pause, read on play) KTD2 a: the live time becomes state the subscription publishes; null means "nothing from this run yet", so the first playing frame falls back to the store exactly as the ref sync did new useLivePlayheadTime.test.tsx, 4 cases including the stale-previous-run one, watched failing with the reset removed
hooks/useMountEffect.ts react-hooks suppression KTD4: the mount closure moves into a useRef, so the empty dependency list is true; useRef keeps its initial value, so it is still the first render's effect the whole suite; this hook is used across Studio and every mount-effect test runs through it
hooks/useMusicBeatAnalysis.ts refs during render KTD2 a: the file-IO ref is refreshed in a commit effect declared before the two effects that read it new useMusicBeatAnalysis.test.tsx, 3 cases, watched failing with the refreshing effect moved after the loader
hooks/usePanelLayout.ts refs during render (four writes) KTD2 a: one commit effect refreshes all four; effectiveLeftCollapsedRef is seeded with the real first-render value instead of false usePanelLayout.test.ts
hooks/usePersistentEditHistory.ts refs during render KTD2 a: the active-project ref moves to a commit effect declared before the loader usePersistentEditHistory.test.ts, usePersistentEditHistory.projectOwnership.test.tsx
hooks/usePreviewPersistence.ts member reorder + refs during render KTD2 a: the save queue is built on first use through one accessor instead of a lazy ref write in render; the toast and apply-edits refs move to commit effects; the two "default to the current frame" parameters resolve in the body usePreviewPersistence.test.ts, externalFileChangeOwnership.test.ts, studio load smoke
hooks/useProjectCompositionVariables.ts react-hooks suppression, then try/finally KTD4: the re-read token is named in the effect body so the list is true; KTD3: the open/mutate/serialize block becomes a plain function, finally and all none of its own; see Not covered
hooks/useProjectSignaturePoll.ts refs during render KTD2 a: both latest refs move to a commit effect declared before the poll useProjectSignaturePoll.test.tsx
hooks/useRazorSplit.ts logical assignment + refs during render KTD2 a for the project-id ref; ??= written longhand with the same nullish test useRazorSplit.test.ts, useRazorSplit.test.tsx, useRazorSplit.history.test.tsx
hooks/useRemoveBackground.ts try/finally without catch KTD3: awaiting the job and releasing the controller moves into a plain function, finally and all none of its own; see Not covered
hooks/useSdkSession.ts refs during render (three writes) KTD2 a: one commit effect declared before the session effect useSdkSession.test.ts, useSdkSession.lifecycle.test.tsx
hooks/useSlideshowTabState.ts react-hooks suppression (over a ref read in a memo) KTD2 a: the scene list is read from the preview frame on commit, into state; the previous array is kept when nothing moved, so no extra commit useSlideshowTabState.test.ts, 3 new cases for scene derivation, empty manifest and a throwing frame
hooks/useThumbnailLease.ts refs during render KTD2 a: getSnapshot takes the request from its arguments (it is read during render); subscribe keeps the identity-keyed ref, refreshed on commit before React subscribes useThumbnailLease.test.tsx, including "resubscribes when the request work shape changes"
hooks/useTimelineDeleteOps.ts throw inside try/catch (three) KTD3: the delete transaction becomes a plain function that throws; the hook awaits it and reports through .catch useTimelineEditing.test.tsx, "rolls back the store duration and live root when a delete persist fails"
hooks/useTimelineEditing.ts refs during render KTD2 a useTimelineEditing.test.tsx
hooks/useTimelineGroupEditing.ts try/finally without catch (two) KTD3: .finally on the promise; the callee is async, so it can only reject useTimelineEditing.test.tsx, the group-move and group-resize cases that assert the GSAP cache is invalidated on both the fallback and track-only paths
hooks/useTimelineSelectionPreviewSync.ts react-hooks suppression KTD4: the two canvas-selection refs move to a commit effect declared first; the dependency list was already complete, so the comment goes with them useTimelineSelectionPreviewSync.test.tsx
hooks/useToast.ts update expression on a module binding The id counter moves into a plain function none of its own; see Not covered
player/hooks/usePlaybackKeyboard.ts refs during render (two writes) KTD2 a: the two handler refs move to a commit effect usePlaybackKeyboard.test.ts, which dispatches through the ref
player/hooks/useTimelinePlayer.ts logical assignment (two), hook used as a value, refs during render ??= longhand; the store's action handles are read through a plain function; the adapter ref moves to a commit effect useTimelinePlayer.test.ts, useTimelinePlayer.seek.test.ts, timeline viewport gate
webmcp/useStudioAgentTools.ts refs during render KTD2 a: the deps ref moves to a commit effect declared before registration useStudioAgentTools.test.tsx

Why

react({ compiler: true }) compiles what it can and silently emits the rest as written, so a hook that opts itself out costs nothing visible and nothing catches it. The per-file ratchet turns that silence into a number; this unit takes twenty-two of them to zero so the number can eventually reach zero and stay there.

Every one of these bail-outs is also a real React rule being bent. A ref written during render is a value the compiler cannot see change, and a render that is thrown away still writes it. Moving the write to the commit is the correction, not a workaround for the compiler.

How

Three shapes, applied at the root rather than suppressed:

  • A latest-value ref refreshed on commit instead of during render. Every one of these refs is read from an effect, an event handler or an async callback, none of which can run before the commit that produced the value. Where the reader is another effect in the same hook, the refreshing effect is declared first so effect order guarantees it has already run; the new useMusicBeatAnalysis suite pins exactly that ordering. Two cases needed more than a move: useThumbnailLease reads its request during render, so getSnapshot takes it from its arguments and only subscribe keeps the ref; usePreviewPersistence builds its save queue on first use through one accessor, because the compiler follows a ref through any function render calls, including a useState initializer.
  • try/finally in a hook body extracted to a plain function, or replaced by .finally on the promise where a single await was being guarded. In useLintModal no extraction was needed at all: its catch swallows rather than rethrows, so the finally body is simply the next statement.
  • Dependency lists made true so their suppressions could go. Two kept an extra re-read token, which is now named in the effect body; one had a complete list already and only carried the comment.

No "use no memo", no new lint suppression, no ref.current mirrored into state.

Two incidental cleanups that fell out: usePreviewPersistence stopped returning a queue ref nothing outside it read, and useTimelineDeleteOps shares one dependency interface between the hook and the extracted function instead of repeating nine fields.

Test plan

  • Bail-out ratchet green, baseline lowered from 126 to 104 with the write flag, JSON diff committed.
  • Full Studio suite: 431 files passed, 1 skipped; 4766 passed, 18 todo. That is the base branch plus the 13 tests added here, with nothing else moving.
  • Typecheck, build, oxlint (0 warnings, 0 errors), oxfmt check, fallow audit (gate passes; the two complexity findings and every clone group are inherited).
  • Studio load smoke, the CI job's own command: PASS: studio loaded with schema-valid API fixtures and no runtime errors.
  • Timeline viewport gate, both arms, the CI job's own commands and budgets. Virtualized arm: interaction p95 32.5ms, frame interval p95 16.8ms, 5 of 5 runs passing, budget 75ms. Unvirtualized arm: interaction p95 33.0ms, frame interval p95 16.8ms, 5 of 5 passing.
  • Design-review capture before and after: computed-style table byte-identical across all five measured controls.
  • All three new suites were watched failing with the change deliberately backed out, so none of them is vacuous.

Not covered

  • Three files changed with no test of their own. useProjectCompositionVariables and useRemoveBackground each moved a try/finally block verbatim into a plain function, and useToast moved a counter increment into one; the guarded statements and the finally clauses are unchanged, so there is no branch to pin that did not exist before. Neither hook has a test today and this change does not add one.
  • No browser gesture pass. The gestures whose reads moved (panel resize, razor split, timeline delete, playback keys) are covered by their unit tests and the two CI gates, not by driving a real browser.
  • The .finally conversions in useTimelineGroupEditing assume the callee cannot throw before returning its promise. That holds because it is declared async; it is not enforced by a type.
  • useMountEffect now holds the first render's closure rather than the committed render's. Under a discarded render both closures compute from the same inputs, so this is not observable, but it is a difference on paper.
  • The repo's lint run does not include react-hooks/exhaustive-deps (it is not in the correctness category), so removing three suppressions was checked against the compiler and by running that rule explicitly, not by the standard lint gate.
  • Files owned by later units in this sequence still bail; the baseline is 104, not 0.

Latest-value ref writes move out of render into commit effects, `try`/`finally`
in hook bodies becomes a plain helper or a promise finalizer, and three
dependency lists are made true so their suppressions can go. Where a value is
read changes; what happens does not.

Lowers the bail-out baseline for these 22 files to zero.
…hanged

`useLintModal` lost a `finally` clause and `useMusicBeatAnalysis` moved a ref
write into an effect that has to run before the loader. Neither had a test.
Both new suites were watched failing with the change backed out.
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