This repository was archived by the owner on Aug 6, 2026. It is now read-only.
fix(canvas): load deep-linked canvases reliably + add manual Refresh - #3656
Merged
trunk-io[bot] merged 4 commits intoJul 22, 2026
Merged
Conversation
Fix a warm-iframe-pool mount race that left deep-linked canvases blank until a hard refresh, and add a manual Refresh action to the canvas menu. The pool only shows a canvas once its slot has a measured on-screen rect. The placeholder measured that rect in a layout effect but created the slot in a *passive* effect that ran later, so the first measurement was dropped (setRect no-ops with no slot). On a settled layout no follow-up re-measure fired, so the rect stayed null and the iframe never became visible; a hard refresh only worked because the still-settling layout triggered a later re-measure. Registering the slot in a layout effect (before the measure) populates the rect deterministically on first commit. Also wires the already-scaffolded refresh path (canvasRefreshStore nonce -> srcDoc reload) to a "Refresh" item in the canvas options menu, invalidating the host-side read cache so data queries re-run. Generated-By: PostHog Code Task-Id: bbcfbcab-0cad-4754-a22e-7d1ee4f956f8
|
😎 Merged successfully - details. |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
raquelmsmith
marked this pull request as ready for review
July 21, 2026 19:31
Contributor
|
Reviews (1): Last reviewed commit: "fix(canvas): load deep-linked canvases r..." | Re-trigger Greptile |
Deterministic regression test for the warm-frame mount race: with the jsdom ResizeObserver stub (which never fires), the placeholder must capture its slot rect from the synchronous on-mount measure. Fails when the slot is registered in a passive effect (rect stays null), passes with the layout-effect registration. Generated-By: PostHog Code Task-Id: bbcfbcab-0cad-4754-a22e-7d1ee4f956f8
Refresh previously only reloaded the iframe's document (via a srcDoc nonce) while keeping the same element, React component, and pool slot — so it couldn't recover from a wedged host-side/pool state (e.g. the kind of mount race that hid deep-linked canvases). Replace that with a true remount: a per-slot "remount generation" in the frame store, folded into the frame's React key. Bumping it tears down and recreates the slot's iframe element (fresh document, fresh in-iframe app, fresh host-side listener). The generation is keyed by slot index and only bumped by an explicit Refresh, so reassigning a slot to another canvas on navigation leaves the key unchanged and still reuses the warm iframe. Removes the now-redundant srcDoc-nonce reload path (canvasRefreshStore and the refreshKey plumbing through the placeholder/host/canvas). Generated-By: PostHog Code Task-Id: bbcfbcab-0cad-4754-a22e-7d1ee4f956f8
Generated-By: PostHog Code Task-Id: bbcfbcab-0cad-4754-a22e-7d1ee4f956f8
There was a problem hiding this comment.
Contained UI-layer bug fix with good test coverage. The useEffect→useLayoutEffect change correctly ensures the slot exists before the rect measurement runs, fixing the deep-link load regression. The frameKeys approach cleanly replaces the deleted canvasRefreshStore with no risk of silent state mismatch, and the Refresh action is purely additive.
Member
Author
|
/trunk merge |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
Clicking a deep link to a canvas after the app has been open for a while showed a blank canvas — it only appeared after a hard refresh of the app. There was also no way to manually reload a mounted canvas.
Why: Requested after canvases repeatedly failed to load from deep links on long-lived sessions, plus a desire to force-refresh a canvas on demand.
Root cause (not stale iframe content, despite the initial hunch): canvases render through a persistent warm-iframe pool that only makes a canvas visible once its slot has a measured on-screen rect. The route-level placeholder measured that rect in a layout effect but created its pool slot in a passive effect that ran afterwards, so the first measurement was dropped (the setter no-ops when the slot doesn't exist yet). On a still-settling layout (right after a hard refresh) a later resize/scroll re-measure filled the rect in; on an already-settled layout no such re-measure fired, so the rect stayed empty and the iframe never became visible.
Changes
ResizeObserverstub never fires, reproducing the "settled layout" condition).How did you test this?
pnpm --filter @posthog/ui typecheck— clean.biome linton all touched files — clean.pnpm --filter @posthog/ui test— full suite passes (incl. newCanvasFramePlaceholderrect regression test andcanvasFrameStoreremount tests). Verified the rect test fails when the fix is reverted.project-bluebirdflag); steps: warm the pool by opening 2+ canvases, then open a canvas via deep link / navigation and confirm it renders without a hard refresh; and ⋯ → Refresh recreates the iframe (brief re-boot) and re-runs data queries.Automatic notifications
Created with PostHog Code