fix(viewer): keep surface frames sized when two viewers share one realm#228
Open
benvinegar wants to merge 1 commit into
Open
fix(viewer): keep surface frames sized when two viewers share one realm#228benvinegar wants to merge 1 commit into
benvinegar wants to merge 1 commit into
Conversation
An embedding host can mount two viewer instances into one JS realm at once (cross-fading an outgoing viewer into an incoming one during an in-place navigation). The engine loads as a single shared module, so both instances shared the module-level surface-frame registry (cardEls) and the single message bridge listener. Both instances route to the same URL and render a Card for the same post, so keying cardEls by post id let them clobber each other's entry — and the outgoing instance's teardown deleted the entry the still-visible instance needed, dropping its surface resize messages so the frames stayed at their seed height until a full reload. The shared listener had the same hazard: the first teardown's removeEventListener tore it out from under the survivor. Key cardEls by a per-Card token instead (frameForSource resolves by contentWindow; new cardForPost handles the scroll-to-card pill), and reference-count the bridge listener so it lives while any viewer is mounted. Self-hosted single-instance behavior is unchanged. Co-Authored-By: Claude Opus 4.8 (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.
Problem
An embedding host can mount two viewer instances into one JS realm at once — e.g. sideshow cloud's double-buffered in-place workspace switch keeps the outgoing viewer mounted (cross-faded) until the incoming one is ready. The engine loads as a single cached ES module, so both instances share the viewer's module scope.
Two module-level singletons weren't safe under that:
cardEls(the surface-frame registry that sizes iframes) was keyed by post id. Both instances route to the same URL and render aCardfor the same post, so they clobbered each other's entry — and when the outgoing instance tore down, itscardEls.delete(id)removed the entry the still-visible instance needed.frameForSourcethen returned null, the surface'sresizepostMessages were dropped, and the frames stayed at their seed height until a full reload.messagelistener was one module-level function added/removed per mount.addEventListener(sameRef)dedups to a single listener, so the first teardown'sremoveEventListenertore it out from under the survivor.Downstream symptom (sideshow cloud): clicking a post in the org/multiuser feed crossed into another member's workspace and left every surface iframe stuck at ~120px until a hard refresh.
Fix
cardEls: eachCardregisters under a unique token, not the post id.frameForSourcealready resolves bycontentWindow(key-agnostic); a newcardForPost(id)handles the "new post" scroll-to-card pill.Self-hosted single-instance behavior is unchanged.
Testing
npm test(425) +typecheck+lintgreen.{h:932}butframeForSourceresolved false; the same post mounted twice / cleaned up once;cardElsemptied after promotion).🤖 Generated with Claude Code