fix(inspector): mirror x/y/w/h on text nodes#34
Merged
Conversation
Two compounding bugs caused Inspector mirror divs for CoreTextNode to ignore layout/transform updates. 1. createProxy only walked one level up the prototype chain when looking up property descriptors. x/y/w/h/mountX/mountY/alpha/rotation/scale/ color/clipping/zIndex are defined on CoreNode.prototype, which is two hops up from a CoreTextNode instance. The descriptor lookup returned undefined and the proxy skipped installing traps for those props entirely — so assigning `textNode.x = 100` updated the canvas but never touched the mirror div. Now walks the full prototype chain. 2. createTextNode passed only `node.textProps` (text/font/lineHeight/ etc.) to createDiv. That bag has no x/y/w/h/mount*, so the mirror div's initial paint had no left/top and rendered at the parent's origin regardless of where the canvas drew the text. Now merges node.props and node.textProps so layout fields land in the first paint. Regular (non-text) nodes are unaffected — the prototype walk is a superset of the old single-level lookup, and createNode already passed the correct prop bag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two compounding bugs caused the Inspector's mirror
<div>forCoreTextNodeinstances to ignore layout/transform updates. SettingtextNode.x = 100(ory,w,h,mountX, etc.) updated the canvas correctly but the DOM mirror stayed at its parent's origin, making the Inspector visually misleading for any app that uses text.What was broken
Bug 1 — proxy walked only one level of the prototype chain.
createProxylooked up property descriptors withObject.getOwnPropertyDescriptor(node, …)then a singleObject.getPrototypeOf(node)step.x/y/w/h/mountX/mountY/alpha/rotation/scale/color/clipping/zIndexare defined onCoreNode.prototype, which is two hops up from aCoreTextNodeinstance (sinceCoreTextNode extends CoreNode). The lookup returnedundefinedand the proxy returned early — no trap was installed for those properties on text nodes, so updates never reached the mirror div.Bug 2 —
createTextNodepassed the wrong prop bag.createDivwas called withnode.textPropsonly (text/font/lineHeight/etc.). That bag has nox/y/w/h/mount*, so the mirror div's initial paint had noleft/topand rendered at the parent's origin regardless of where the canvas drew the text.Regular (non-text)
CoreNodeinstances were unaffected because their prototype isCoreNode.prototypedirectly, andcreateNodealready passesnode.props.Fix
createProxyuntil the descriptor is found ornullis reached.node.propsandnode.textPropsincreateTextNodeso layout fields land in the initialcreateDivcall.Test plan
textNode.x,textNode.y,textNode.w,textNode.hat runtime; mirror div tracks.mountX: 0.5/mountY: 0.5on a text node; mount-offset math runs (updateNodePropertybranch at the bottom of the file).pnpm buildclean (verified locally).Notes for the reviewer
mountX/Yis set on a text node before the text has loaded,props.w/props.hare stillundefinedand the mount-offset math computesNaN. Pre-existing; worth a follow-up but out of scope for this fix.CoreTextNodewas introduced — anyone using the Inspector to debug text positioning was getting incorrect mirror positions.🤖 Generated with Claude Code