Repository navigation
fix(sdk): restore saved edits for dotted element IDs - #6287
Conversation
Preview approval requiredThis pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit |
miguel-heygen
left a comment
There was a problem hiding this comment.
Thanks for the clear report and the fix. I reproduced #6286 on the published 0.8.195 SDK, and the new tests are good: with main's patches.ts the three dotted-ID tests fail and the two compatibility tests still pass, so they guard exactly this change. The full SDK suite and typecheck pass at this head.
One regression to fix before merge. keyToPath used to return null for a key it could not place, and applyOverrideSet skips those (apply-patches.ts:144). Now its last line, return elementKeyToPath(key, true) ?? elementPath(key) (patches.ts:200), turns every unrecognised key into a whole-element path. With a non-null scalar value that reaches the element replace branch, which sets tmp.innerHTML = v.html with v.html undefined (apply-patches.ts:309), and openComposition throws TypeError: Cannot read properties of undefined (reading 'length'). For example {"hf-title.timing.bogus": 2}, {"meta.fps": 30} and {"style.foo": "x"} each throw at this head and open unchanged on main. The SDK never writes those keys itself, but a delta saved by a newer SDK version, or edited outside the SDK, would stop the whole composition from opening instead of skipping one key. An unknown key whose value is an object with html now inserts that markup too.
Since a dotted removal marker and an unknown key look the same, the value is what tells them apart. This guard right after if (!path) continue; in applyOverrideSet worked when I tried it:
if (value !== null && /^\/elements\/[^/]+$/.test(path) && !isElementSnapshotOverride(key, value)) continue;With it, all of the keys above open the same as the base document and the full SDK suite still passes. A test that opens with an unknown key or two and expects serialize() to equal the base would pin it.
Verdict: REQUEST_CHANGES
Reasoning: The dotted-ID fix is correct and well tested, but it makes openComposition throw on override keys that main skips, and the fix for that is one guard.
|
Fixed in Replay now skips non-null whole-element values unless they are saved snapshots. The shared snapshot check requires both HTML and sibling position, so scalar values and incomplete objects neither throw nor insert markup. Null removals and valid legacy/current snapshots still replay. The five new cases fail on the previous PR head and pass after this correction. All 702 SDK tests pass; independent GPT-6.1 Sol xhigh review passed 184 focused tests plus Node/Bun replay and compatibility probes. |
miguel-heygen
left a comment
There was a problem hiding this comment.
Thanks for the quick fix. The guard sits in the one place every saved override is replayed, and requiring html in isElementSnapshotOverride also covers a snapshot with no html, which the guard I suggested would have let through on its own. Every snapshot the SDK writes includes html, parentId and siblingIndex, so nothing it saved before is now skipped.
I checked that the new tests guard both parts: with the previous apply-patches.ts all five new cases fail, and with only the html check removed the { siblingIndex: 0 } case fails. The full SDK suite and typecheck pass at this head, and CI is green.
Verdict: APPROVE
Reasoning: The crash on unknown override keys is fixed at the shared replay step with tests that fail without each half of the change, and CI is green.
What
Edits to an authored ID such as
hf-title.maindisappeared when an SDK embedder reopened its saved override set. The live edit worked and the delta contained the new value, but replay discarded the key.Restore those edits and removal markers, including scoped IDs with periods in the host or leaf.
Why
Template-driven products store the SDK's sparse delta as the user's saved state. An accepted authored ID should not make those edits disappear on the next session.
Related work
Closes #6286
#1350 corrected deletion replay and escaped attribute names; #1434 added scoped IDs. #5243 fixed JSON Pointer decoding during scoped patch replay. This preserves those contracts and corrects the separate dotted-ID decoding failure in saved override keys.
How
Keep existing plain-ID key families and namespaces ahead of a dotted-ID fallback. One element-key parser recognizes the full edit-family separator and preserves the remaining property name, including periods in attributes. The wire format stays unchanged. Replay accepts only null removal markers or complete saved snapshots at whole-element paths, so unknown scalar values and incomplete snapshots cannot prevent opening or insert stray markup. Literal IDs that collide with reserved override-key syntax remain outside this correction.
Test plan
var.Independent GPT-6.1 Sol xhigh review cleared exact signed commit
739a909fb91fb64da752f50efddf220b05ee2b62. Direct Node and Bun public probes verify unknown-value guards, valid legacy/current snapshots, and legacy key precedence; 184 focused SDK tests pass.The initial full workspace build passed at
221f3671f; SDK checks were rerun after updating to38afc0b01. No dependencies or package budgets changed. Existing compatible dependencies were reused. No clean install, browser/Studio interaction or video export is claimed. Mintlify validation was unavailable becausemintis absent.