fix(studio): wire onTrySdkPersist to sdkCutoverPersist (cutover was unwired)#1463
Conversation
…nwired) Stage 7 s7.5 removed the feature flag and declared cutover 'always-on', but onTrySdkPersist was never actually passed to useDomEditCommits — the sdkCutoverPersist function was dead code in production. Thread sdkSession through useDomEditSession params, build the onTrySdkPersist closure there (all CutoverDeps are already in scope), and pass sdkSession from App.tsx. Style/text/attribute/html-attribute commits now route through SDK dispatch instead of the server patch path. Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>
vanceingalls
left a comment
vanceingalls
left a comment
There was a problem hiding this comment.
On the matter of #1463 — onTrySdkPersist finally meets sdkCutoverPersist.
Intent
A correctness fix bolted to the end of #1462: s7.5 declared the cutover always-on but never actually threaded onTrySdkPersist into useDomEditCommits, leaving sdkCutoverPersist orphaned in production. This PR adds the missing closure and the sdkSession parameter that feeds it.
Per-file observations
packages/studio/src/hooks/useDomEditSession.ts:231-245— the newonTrySdkPersistbuilds the closure inside the hook body using deps already in scope (editHistory,writeProjectFile,reloadPreview,domEditSaveTimestampRef). Conditional construction (sdkSession ? ... : undefined) keeps the no-session path identical to pre-PR behavior. Clean.packages/studio/src/App.tsx:307—sdkSessionis restored as a parameter touseDomEditSession. This is the very line #1462 removed two PRs earlier; the symmetry is intentional but worth pointing out for archaeology: #1462's deletion was removing the shadow-dispatch wiring, and #1463's re-addition is adding the cutover wiring. Same prop, different consumer. A one-line comment on the call-site noting "now powers cutover, no longer shadow dispatch" would save a future grep.
Cross-PR contract drift
Per #1471's later rework, this onTrySdkPersist closure grows a fifth options argument (label/coalesceKey/skipRefresh) and a compositionPath field on the CutoverDeps. Both are forward-compatible — undefined trailing args are tolerated — so #1463 standing alone is not broken by the later expansion. But the fix shipped in #1463 silently loses the @font-face prepareContent guard until #1471 lands; any in-between merge of this PR would route font-bearing edits through the SDK and drop the injected font. Worth flagging in the PR description so the team treats #1471 as a same-train co-requisite, not a follow-up.
Stale-base hazard intersection
App.tsx is one of the stack's hotspot files (also touched by main's #1491 master-view fix and #1492 telemetry funnel). The diff at line 307 here is tiny — a single property addition inside the useDomEditSession props object — and unlikely to conflict with #1491/#1492's region of App.tsx. But auditable only against the actual rebase.
CI
Same inherited preflight format drift on gsapSerialize.ts; same downstream cascade. No logic failures on this PR's diff.
Verdict
clear-with-nits at 9bde352198a02352849ff8b722787d44fc11029a — the bug being fixed is real (dead sdkCutoverPersist since s7.5 shipped) and the fix is minimal. Worth noting in the PR body that prepareContent-bearing edits (custom-font injection) are silently routed through the SDK between this PR and #1471, so the train should land together. Re-verify if HEAD moves before stamp.
Review by Via
1 participant
What
Stage 7 s7.5 declared the SDK cutover "always-on" by removing the feature flag, but
onTrySdkPersistwas never actually passed touseDomEditCommits—sdkCutoverPersistwasdead code in production. Every DOM edit still went through the server patch path.
This PR wires the cutover for the op types already supported by
sdkCutover.ts:inline-style→setStyle,text-content→setText,attribute/html-attribute→setAttribute.Why
Without this wiring, Stages 3.1–3.5 (delete, timing, GSAP panel) have no foundation to build on.
Also fixes a correctness gap: s7.5 said cutover was active but it wasn't.
How
sdkSession?: Composition | nulltoUseDomEditSessionParamsuseDomEditSession, importedsdkCutoverPersistand built anonTrySdkPersistclosureusing deps already in scope (
editHistory,writeProjectFile,reloadPreview,domEditSaveTimestampRef)sdkSessionfromApp.tsxintouseDomEditSession(all other CutoverDeps were already threaded)onTrySdkPersistisundefinedwhensdkSessionis null/absent — no behavior change without a sessionApp.tsxis exactly 600 lines (at the Studio file-size gate limit).Test plan
bun run build— passes (all packages)bun run testinpackages/sdk— 226/226 passbun run testinpackages/studio— 3 pre-existing failures inuseDomEditSession.test.ts(stale test calling
shouldUseSdkCutoverwith a now-removed flag param); all other tests passbunx oxlint+bunx oxfmt --check— no warnings or errors on changed filessdk_cutover_successtelemetry fires (not_fallback)