Skip to content
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 into
mainfrom
posthog-code/canvas-deeplink-load-and-refresh
Jul 22, 2026
Merged

fix(canvas): load deep-linked canvases reliably + add manual Refresh#3656
trunk-io[bot] merged 4 commits into
mainfrom
posthog-code/canvas-deeplink-load-and-refresh

Conversation

@raquelmsmith

@raquelmsmith raquelmsmith commented Jul 21, 2026

Copy link
Copy Markdown
Member

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

  • Deep-link load fix: register the canvas's pool slot in a layout effect ordered before the rect measurement, so the rect is populated deterministically on first commit and the canvas shows without a hard refresh. Guarded by a deterministic regression test (the jsdom ResizeObserver stub never fires, reproducing the "settled layout" condition).
  • Manual Refresh: add a Refresh item to the canvas options (⋯) menu that fully remounts the mounted iframe element — not just reloads its document — so it also recovers from a wedged host-side/pool state, and invalidates the host-side canvas read cache so the reload fetches fresh data. Implemented as a per-slot "remount generation" folded into the frame's React key; it's keyed by slot index and bumped only by an explicit refresh, so navigation between canvases still reuses the warm frames (no reload).

How did you test this?

  • pnpm --filter @posthog/ui typecheck — clean.
  • biome lint on all touched files — clean.
  • pnpm --filter @posthog/ui test — full suite passes (incl. new CanvasFramePlaceholder rect regression test and canvasFrameStore remount tests). Verified the rect test fails when the fix is reverted.
  • Manual verification in the running app is still pending (needs the project-bluebird flag); 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

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Created with PostHog Code

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
@trunk-io

trunk-io Bot commented Jul 21, 2026

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit bfd86f7.

@raquelmsmith
raquelmsmith marked this pull request as ready for review July 21, 2026 19:31
@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
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
@raquelmsmith raquelmsmith added the Stamphog This will request an autostamp by stamphog on small changes label Jul 22, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contained UI-layer bug fix with good test coverage. The useEffectuseLayoutEffect 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.

@raquelmsmith

Copy link
Copy Markdown
Member Author

/trunk merge

@trunk-io
trunk-io Bot merged commit a4bb8b3 into main Jul 22, 2026
37 checks passed
@trunk-io
trunk-io Bot deleted the posthog-code/canvas-deeplink-load-and-refresh branch July 22, 2026 00:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Stamphog This will request an autostamp by stamphog on small changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant