Skip to content

refactor(studio): compile the first eleven hooks under the React Compiler - #3783

Draft
miguel-heygen wants to merge 1 commit into
refactor/studio-c1-compiler-bailout-ratchetfrom
refactor/studio-c2a1-compiler-hooks-a
Draft

refactor(studio): compile the first eleven hooks under the React Compiler#3783
miguel-heygen wants to merge 1 commit into
refactor/studio-c1-compiler-bailout-ratchetfrom
refactor/studio-c2a1-compiler-hooks-a

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Lands unit C2a, first half (hooks) of the Studio React Compiler coverage plan. Stacked on the ratchet PR (#3773). Ratchet 126 to 115. Also closes a pre-existing floating promise in the block-install handlers.

What

Eleven hooks under packages/studio/src/hooks, timelineAudioGroupCreate.ts through useElementLifecycleOps.ts in path order, now compile under the React Compiler. The per-file bail-out baseline drops from 126 to 115, one entry per file.

It also fixes a defect found while testing one of them: a failed block install left its rejection unhandled, so the user saw an "Adding…" toast and then nothing.

Five new test files pin the paths whose structure changed.

This is the first of two stacked branches. The second takes the remaining eleven hooks.

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. These eleven hooks were all being skipped, and none of it was visible.

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
timelineAudioGroupCreate.ts throw inside try/catch KTD3: the log-toast-rethrow moves to a module function taking the write as a thunk useAudioGroupCarveAssignment.test.tsx "rejects, and toasts, when an id resolves to no clip"
useAnimatedPropertyCommit.ts throw inside try/catch KTD3: the guarded body becomes a local async function, the catch becomes .catch() on its promise useAnimatedPropertyCommit.test.tsx "rethrows a persistence failure to the telemetry wrapper"
useAppHotkeys.ts ref written during render KTD2a: the fifteen-callback bundle is written in an effect; every reader is a keydown handler useAppHotkeys.test.ts + .textEditing + .previewForwarding (14 cases)
useBlockCatalog.ts try/finally in the load effect KTD3: the fetch returns a tagged union from a module function; the effect has one branch and no try new useBlockCatalog.test.tsx (3 cases: loaded, rejected, not-ok)
useBlockHandlers.ts react-hooks suppression; try/finally KTD4: the deps are destructured so the list is eight plain names, no suppression. KTD3: the install latch's finally moves to a module function new useBlockHandlers.test.tsx (4 cases, see the defect note below)
useClipboard.ts ref written during render KTD2c: the ref was standing in for a memo; the value is read from the current render and added to the dep list new useClipboard.test.tsx (paste uses the current project; no project, no read)
useConsoleErrorCapture.ts locals reassigned after render KTD3-shaped: attach/detach moves to a module function returning its own undo; the hook keeps one effect new useConsoleErrorCapture.test.tsx (4 cases incl. restore on unmount)
useDomEditPreviewSync.ts ref written during render KTD2a: written in an effect declared above its reader, so commit order still gives it this render's callback new useDomEditPreviewSync.test.tsx (reveal fires; newest callback wins)
useDomEditSession.ts logical assignment ??= Assigned inside the ?? instead; the compiler cannot lower a logical assignment useDomEditSession.test.tsx, .membersForDelete
useDomSelection.ts five refs written during render KTD2a: one effect resyncs all five; the handlers already wrote them eagerly ahead of state useDomSelection.test.ts, useDomSelectionSelectionGuards.test.ts
useElementLifecycleOps.ts two throws in try/catch; try/finally KTD3: both become local async functions with .catch() / .finally() on their promises useElementLifecycleOps.test.tsx, .multiDelete (21 cases)

The block-install defect

Writing the useBlockHandlers test surfaced a real one. All four block handlers drop the install promise: three call runBlockInstall as void, and the fourth is awaited from a JSX handler that is itself unawaited. A rejection therefore reached nothing that could report it, and the user was left with "Adding …" and silence.

Every caller routes through runBlockInstall, so the report goes there rather than into four call sites, on the same toast surface the install itself already uses. It resolves to null, which each caller already reads as "nothing installed". Two new cases cover it, and both fail without the catch.

Reachability, stated honestly: addBlockToProject currently catches everything internally and returns null, so no production path reaches the rejection today. The floating promise is the defect; this closes it at the one place all four handlers pass through, so it stays closed if that installer ever throws.

Two 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.
  • .catch() and .finally() on a promise are fine where try/catch and try/finally statements are not, which 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 126 down to 115, exactly these eleven entries removed.
  • Full Studio suite: bunx vitest run --maxWorkers=4, 433 passed, 1 skipped (434 files); 4772 passed, 18 todo. The base branch is 428 files and 4753 tests; this adds 5 files and 19 cases.
  • bun run typecheck, clean.
  • oxlint and oxfmt --check on the 16 changed source files, from the repo root, clean.
  • fallow audit --base origin/main --fail-on-issues, passes. One complexity exemption moves with the code it was already granted for; the remaining finding is inherited, in a function this branch does not touch.
  • 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 other eleven hooks, and the two call-site files they touch, are in the stacked branch on top of this one. The ratchet is per file, so this branch is green on its own with those still at their baseline.
  • useDomEditSession.ts appears in both halves: the ??= fix here, one dead argument removed in the next branch. Nothing else in that file changes.
  • No browser gesture review here. The full render-parity capture and the two CI Studio gates were run on the combined tree before the split, not per branch.
  • The design-shot computed-style table and both arms of the timeline viewport gate were verified on the combined tree; splitting only partitions the same file changes across two commits.

Each of these hooks was emitted uncompiled because it did something the
React Compiler declines: wrote a ref during render, held a try/finally or a
throw inside try/catch, carried a react-hooks suppression, or used a
lowering the compiler does not support. None of that is visible at runtime,
so every memo in them was silently lost.

Each is fixed where the value is read, not by changing 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, and one
suppressed dependency list is made true rather than silenced.

The bail-out baseline falls from 126 to 115, one per file.
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