Repository navigation
fix(sdk): keep font and image declarations after object edits - #6281
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
miguel-heygen
left a comment
There was a problem hiding this comment.
Thanks, this fixes it at the right place. The SDK was already writing the whole object into default, and the shared declaration check was the only thing rejecting it, so widening that one check fixes reads, validation, lint and the CLI registry at once. Moving FontValue/ImageValue into the parser package keeps a single definition. I checked that the new tests fail without the change: removing the two font/image lines from isCompositionVariable breaks the parser, lint and SDK tests, and dropping the object merge in the form breaks the six form tests.
Two small things:
- Studio's Bind panel will now print
([object Object]). When you bind to an existing id, the warning showsString(existingDecl.default)(VariablesBindElement.tsx:221). Those declarations used to be filtered out, so that text never got an object before.PromotableControl.tsxalready handles this by only showing string defaults. Showing the font name or image URL here would match the form. - Optional:
mergeDeclarationEditalready merges the original declaration with the edit at both call sites. Merging the object default there would avoid storingoriginalon the draft and keep that logic in one place.
Verdict: APPROVE
Reasoning: Root-cause fix at the shared declaration check with tests that fail without it; CI is green and the remaining points are small.
|
Fixed the Bind warning in It now shares the declaration form's display conversion: font objects show their name and image objects show their URL. Metadata merging and Save behavior remain covered by the existing regressions. Both object cases fail on the previous head and pass with the correction. All 25 focused tests and 11 independent controls pass; matching before/after captures were inspected and reproduced byte-for-byte in independent review. The Studio build and package gates pass without budget changes. |
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Thanks, this is the right shape: one helper for "how a declaration's default reads as text", used by both the form and the Bind warning, so they can't drift apart again. I checked that the new test catches the bug: with the warning back on String(existingDecl.default), the font and image object cases fail with ([object Object]) and the plain string cases still pass. The Studio panel tests and typecheck pass at this head.
The four red checks are from a duplicate CI run on this commit that was cancelled; the other run for the same commit passed, and all required checks are green.
Verdict: APPROVE
Reasoning: The Bind warning now shows the font name or image URL through a shared helper, pinned by a test that fails without it; required checks pass.
What
The documented font/image object forms of
setVariableValue()made their own declarations disappear. The same values then failed validation as undeclared, and saved HTML acquired two lint errors, even though the runtime displayed the objects correctly.Font and image declarations now remain available through object edits, save/reopen and history. Studio's declaration form also preserves their source and image metadata when changing a label or the primary name/URL.
Why
The SDK already persisted whole objects in the declaration JSON. Its stricter declaration reader accepted only strings for these defaults. Recognizing those existing values exposed a scalar-only form conversion, which this PR corrects too.
Related work
Closes #6279
#1569 introduced object values and B1 JSON persistence. #1588 tightened their shape, and #5314 aligned lint with the shared declaration predicate. This preserves that shared boundary and the existing storage model.
How
Keep font/image types and object guards in the parser owner and reuse them in core and SDK. String fallbacks still work, malformed values remain rejected, and image object URLs use the existing lint allowlist. The shared Studio form displays name/URL and retains the complete original object while editing those fields or declaration metadata. Bind warnings reuse that display conversion so existing font/image object defaults show their name or URL.
Before
Same synthetic composition, after SDK object writes followed by label-only form saves. The scalar-only form loses the font and logo.
After
The same form inputs and save/reopen operation retain monospace text, the blue logo and all object metadata.
Existing-variable bind warnings
Binding to an existing font or image variable now shows the same primary value as the declaration form.
([object Object])(Brand Sans)([object Object])(assets/logo.svg)The real shared React bind component and real SDK declarations were exercised in muted headless Chrome at matching settings. Both warnings are readable, the object metadata stays intact, and there are no page errors.
Test plan
Integrity-verified published SDK/core/parsers/lint 0.8.195 reproduce the defect with compatible reused dependencies. No clean consumer install, complete Studio shell interaction, remote font/media loading or MP4 export is claimed. Two existing parser todos remain. Mintlify was unavailable.
Independent GPT-6.1 Sol xhigh review cleared exact signed commit
5faa0bc8e2fb5b575637e0d2cbc1f6d13bc373b8. It passed all 25 focused tests and 11 additional controls, reproduced both captures byte-for-byte, and verified unchanged source/build files. The fresh Studio archive passes the unchanged size, import and export gates.