Skip to content

Commit a4bb8b3

Browse files
authored
fix(canvas): load deep-linked canvases reliably + add manual Refresh (#3656)
1 parent b057bfb commit a4bb8b3

8 files changed

Lines changed: 158 additions & 58 deletions

File tree

packages/ui/src/features/canvas/components/WebsiteLayout.tsx

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
ArrowClockwiseIcon,
23
DotsThreeIcon,
34
GitForkIcon,
45
LinkIcon,
@@ -18,6 +19,8 @@ import { ChannelBreadcrumb } from "@posthog/ui/features/canvas/components/Channe
1819
import { iconForTemplate } from "@posthog/ui/features/canvas/components/canvasTemplateIcon";
1920
import { NewCanvasMenu } from "@posthog/ui/features/canvas/components/NewCanvasMenu";
2021
import { CanvasFrameHost } from "@posthog/ui/features/canvas/freeform/CanvasFrameHost";
22+
import { useCanvasFrameStore } from "@posthog/ui/features/canvas/freeform/canvasFrameStore";
23+
import { CANVAS_QUERY_KEY } from "@posthog/ui/features/canvas/freeform/freeformDataBridge";
2124
import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels";
2225
import { useChannelTasks } from "@posthog/ui/features/canvas/hooks/useChannelTasks";
2326
import {
@@ -39,6 +42,7 @@ import { toast } from "@posthog/ui/primitives/toast";
3942
import { track } from "@posthog/ui/shell/analytics";
4043
import { useHeaderStore } from "@posthog/ui/shell/headerStore";
4144
import { Box, Flex } from "@radix-ui/themes";
45+
import { useQueryClient } from "@tanstack/react-query";
4246
import {
4347
Outlet,
4448
useNavigate,
@@ -106,6 +110,23 @@ function FreeformEditControls({
106110
const revert = useFreeformChatStore((s) => s.revert);
107111
const goToLatest = useFreeformChatStore((s) => s.goToLatest);
108112

113+
const queryClient = useQueryClient();
114+
const remountFrame = useCanvasFrameStore((s) => s.remount);
115+
// Fully remount the mounted canvas iframe: drop the host-side read cache so
116+
// queries re-run, then recreate the iframe element (not just reload its
117+
// document) so a refresh also recovers from a wedged frame.
118+
const onRefresh = () => {
119+
track(ANALYTICS_EVENTS.DASHBOARD_ACTION, {
120+
action_type: "refresh",
121+
surface: "canvas",
122+
channel_id: channelId,
123+
dashboard_id: dashboardId,
124+
kind: "freeform",
125+
});
126+
void queryClient.invalidateQueries({ queryKey: [CANVAS_QUERY_KEY] });
127+
remountFrame(dashboardId);
128+
};
129+
109130
const hasCode = code.length > 0;
110131
// Viewing the head version (or there's no history yet) → autosave is live.
111132
// Otherwise the user has undone to an older version and is browsing.
@@ -212,6 +233,10 @@ function FreeformEditControls({
212233
}
213234
/>
214235
<DropdownMenuContent align="end" side="bottom" sideOffset={4}>
236+
<DropdownMenuItem onClick={onRefresh}>
237+
<ArrowClockwiseIcon size={14} />
238+
Refresh
239+
</DropdownMenuItem>
215240
<DropdownMenuItem
216241
onClick={() =>
217242
void copyCanvasLink(channelId, dashboardId, "canvas")

packages/ui/src/features/canvas/freeform/CanvasFrameHost.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { FreeformCanvas } from "./FreeformCanvas";
1111
export function CanvasFrameHost() {
1212
const slots = useCanvasFrameStore((s) => s.slots);
1313
const activeDashboardId = useCanvasFrameStore((s) => s.activeDashboardId);
14+
const frameKeys = useCanvasFrameStore((s) => s.frameKeys);
1415

1516
return (
1617
// Fixed, viewport-filling, click-through layer. Children are positioned in
@@ -43,15 +44,16 @@ export function CanvasFrameHost() {
4344
// Keyed by the physical frame's identity (its slot index), NOT dashboardId:
4445
// reassigning a slot to a new canvas must reuse the same iframe (init
4546
// code-swap), not remount it — remounting re-parents the iframe = reload.
46-
const frameKey = `slot-${slotId}`;
47+
// The remount generation (bumped only by an explicit user Refresh) is
48+
// folded in so that action — and only that action — recreates the iframe.
49+
const frameKey = `slot-${slotId}-${frameKeys[slotId] ?? 0}`;
4750
return (
4851
<div key={frameKey} style={style}>
4952
<ErrorBoundary name="freeform-canvas" resetKey={slot.dashboardId}>
5053
<FreeformCanvas
5154
code={slot.inputs.code}
5255
mode="edit"
5356
analytics={slot.inputs.analytics}
54-
refreshKey={slot.inputs.refreshKey}
5557
onDataRequest={slot.inputs.onDataRequest}
5658
onError={slot.inputs.onError}
5759
onRendered={slot.inputs.onRendered}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { cleanup, render } from "@testing-library/react";
2+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
3+
import { CanvasFramePlaceholder } from "./CanvasFramePlaceholder";
4+
import { useCanvasFrameStore } from "./canvasFrameStore";
5+
6+
// The warm-frame host only shows a canvas once its slot has a measured rect
7+
// (`active = ... && !!slot.rect`). The placeholder must populate that rect from
8+
// its own synchronous measure on mount, WITHOUT relying on a later
9+
// ResizeObserver/scroll re-measure — on a settled layout no such re-measure
10+
// fires, so a canvas opened while the app has been running a while (e.g. via a
11+
// deep link) would otherwise stay hidden until a hard refresh. The jsdom
12+
// ResizeObserver stub never fires, so this test reproduces that settled-layout
13+
// condition: if the rect isn't captured synchronously, it never is.
14+
15+
const RECT = { top: 10, left: 20, width: 800, height: 600 };
16+
17+
function resetStore() {
18+
useCanvasFrameStore.setState({
19+
slots: [],
20+
activeDashboardId: null,
21+
maxWarmFrames: 2,
22+
});
23+
}
24+
25+
describe("CanvasFramePlaceholder", () => {
26+
beforeEach(() => {
27+
resetStore();
28+
vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue({
29+
...RECT,
30+
right: RECT.left + RECT.width,
31+
bottom: RECT.top + RECT.height,
32+
x: RECT.left,
33+
y: RECT.top,
34+
toJSON: () => "",
35+
} as DOMRect);
36+
});
37+
38+
afterEach(() => {
39+
cleanup();
40+
vi.restoreAllMocks();
41+
resetStore();
42+
});
43+
44+
it("captures the slot rect on mount without a follow-up re-measure", () => {
45+
render(
46+
<CanvasFramePlaceholder
47+
dashboardId="dash-1"
48+
code="export default () => null"
49+
onDataRequest={async () => null}
50+
/>,
51+
);
52+
53+
const slot = useCanvasFrameStore
54+
.getState()
55+
.slots.find((s) => s?.dashboardId === "dash-1");
56+
expect(slot).toBeTruthy();
57+
// Pre-fix this is null (the slot is registered in a passive effect that runs
58+
// after the layout-effect measure, so the first measure is dropped).
59+
expect(slot?.rect).toEqual(RECT);
60+
expect(useCanvasFrameStore.getState().activeDashboardId).toBe("dash-1");
61+
});
62+
});

packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.tsx

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import type {
22
CanvasAnalyticsConfig,
33
CanvasNavIntent,
44
} from "@posthog/core/canvas/freeformSchemas";
5-
import { useEffect, useLayoutEffect, useMemo, useRef } from "react";
6-
import { useCanvasRefreshNonce } from "../stores/canvasRefreshStore";
5+
import { useLayoutEffect, useMemo, useRef } from "react";
76
import { useCanvasFrameStore } from "./canvasFrameStore";
87

98
// Stands in for the canvas inside the route tree. It renders nothing visible —
@@ -29,7 +28,6 @@ export function CanvasFramePlaceholder({
2928
onNavigate?: (intent: CanvasNavIntent) => void;
3029
}) {
3130
const ref = useRef<HTMLDivElement>(null);
32-
const refreshKey = useCanvasRefreshNonce(`dashboard:${dashboardId}`);
3331

3432
const register = useCanvasFrameStore((s) => s.register);
3533
const setRect = useCanvasFrameStore((s) => s.setRect);
@@ -40,24 +38,21 @@ export function CanvasFramePlaceholder({
4038
() => ({
4139
code,
4240
analytics,
43-
refreshKey,
4441
onDataRequest,
4542
onError,
4643
onRendered,
4744
onNavigate,
4845
}),
49-
[
50-
code,
51-
analytics,
52-
refreshKey,
53-
onDataRequest,
54-
onError,
55-
onRendered,
56-
onNavigate,
57-
],
46+
[code, analytics, onDataRequest, onError, onRendered, onNavigate],
5847
);
5948

60-
useEffect(() => {
49+
// Layout effect (not passive) and declared first, so the slot exists before the
50+
// rect-measure effect below runs its initial synchronous measure. Otherwise the
51+
// slot is created too late (setRect no-ops with no slot) and the frame only
52+
// becomes visible once a later scroll/resize re-measures — which never happens
53+
// on a settled layout, so a canvas opened while the app has been running a while
54+
// (e.g. via a deep link) stays hidden until a hard refresh.
55+
useLayoutEffect(() => {
6156
register(dashboardId, inputs);
6257
}, [dashboardId, inputs, register]);
6358

packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,6 @@ export interface FreeformCanvasProps {
4545
* never crosses into the iframe.
4646
*/
4747
analytics?: CanvasAnalyticsConfig;
48-
/**
49-
* Bump to force a full iframe reload (a fresh boot re-runs the app's data
50-
* `useEffect`s = a real refresh). Folded into the srcDoc so changing it
51-
* reloads the frame via the existing reload path.
52-
*/
53-
refreshKey?: number;
5448
}
5549

5650
// Renders a freeform-React canvas inside a null-origin sandboxed iframe and
@@ -64,7 +58,6 @@ export function FreeformCanvas({
6458
onRendered,
6559
onNavigate,
6660
analytics,
67-
refreshKey = 0,
6861
}: FreeformCanvasProps) {
6962
const iframeRef = useRef<HTMLIFrameElement>(null);
7063
// The canvas mirrors the host's light/dark theme. Passed via `init` (not the
@@ -79,12 +72,10 @@ export function FreeformCanvas({
7972
// for posthog-js), not on code: code is injected via `init`, so changing it
8073
// never reloads the iframe — it re-renders in place.
8174
const analyticsHost = analytics?.apiHost;
82-
const srcDoc = useMemo(() => {
83-
const doc = buildSandboxDocument(mode, analyticsHost);
84-
// Append the refresh nonce as a comment so a bump changes the srcDoc string,
85-
// which reloads the iframe (re-announces "ready" → re-init → fresh run).
86-
return refreshKey > 0 ? `${doc}\n<!-- refresh:${refreshKey} -->` : doc;
87-
}, [mode, analyticsHost, refreshKey]);
75+
const srcDoc = useMemo(
76+
() => buildSandboxDocument(mode, analyticsHost),
77+
[mode, analyticsHost],
78+
);
8879

8980
// Latest props, read by the once-bound listener + the (stable) postInit.
9081
const latest = useRef({

packages/ui/src/features/canvas/freeform/canvasFrameStore.test.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,15 @@ import {
55
} from "./canvasFrameStore";
66

77
function inputs(code: string): CanvasFrameInputs {
8-
return { code, refreshKey: 0, onDataRequest: vi.fn() };
8+
return { code, onDataRequest: vi.fn() };
99
}
1010

1111
function reset() {
1212
useCanvasFrameStore.setState({
1313
slots: [],
1414
activeDashboardId: null,
1515
maxWarmFrames: 2,
16+
frameKeys: {},
1617
});
1718
}
1819

@@ -88,6 +89,38 @@ describe("canvasFrameStore", () => {
8889
expect(useCanvasFrameStore.getState().slots).toBe(after);
8990
});
9091

92+
it("remount bumps only the frame generation of the canvas's slot", () => {
93+
const { register, remount } = useCanvasFrameStore.getState();
94+
register("a", inputs("A"));
95+
register("b", inputs("B"));
96+
97+
remount("a");
98+
expect(useCanvasFrameStore.getState().frameKeys[0]).toBe(1);
99+
expect(useCanvasFrameStore.getState().frameKeys[1] ?? 0).toBe(0);
100+
});
101+
102+
it("remount is a no-op for a canvas with no slot", () => {
103+
const { remount } = useCanvasFrameStore.getState();
104+
const before = useCanvasFrameStore.getState().frameKeys;
105+
remount("missing");
106+
expect(useCanvasFrameStore.getState().frameKeys).toBe(before);
107+
});
108+
109+
it("keeps a slot's remount generation when it is reassigned (warm reuse)", () => {
110+
const { register, activate, remount } = useCanvasFrameStore.getState();
111+
register("a", inputs("A"));
112+
activate("a");
113+
remount("a"); // slot 0 generation -> 1
114+
register("b", inputs("B"));
115+
activate("b");
116+
register("c", inputs("C")); // evicts LRU "a", "c" takes slot 0
117+
118+
expect(slotIndexOf("c")).toBe(0);
119+
// Reassigning slot 0 to a different canvas must NOT change its key, so the
120+
// warm iframe is reused (code-swap) rather than remounted on navigation.
121+
expect(useCanvasFrameStore.getState().frameKeys[0]).toBe(1);
122+
});
123+
91124
it("deactivate clears the active id only when it matches", () => {
92125
const { register, activate, deactivate } = useCanvasFrameStore.getState();
93126
register("a", inputs("A"));

packages/ui/src/features/canvas/freeform/canvasFrameStore.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ export interface CanvasFrameRect {
3030
export interface CanvasFrameInputs {
3131
code: string;
3232
analytics?: CanvasAnalyticsConfig;
33-
refreshKey: number;
3433
onDataRequest: (method: string, payload: unknown) => Promise<unknown>;
3534
onError?: (message: string, stack?: string) => void;
3635
onRendered?: () => void;
@@ -49,11 +48,18 @@ interface CanvasFrameStore {
4948
slots: (CanvasFrameSlot | null)[];
5049
activeDashboardId: string | null;
5150
maxWarmFrames: number;
51+
// Per-slot remount generation. Folded into the frame's React key by the host,
52+
// so bumping it tears down and recreates that slot's iframe element (a true
53+
// remount, unlike a code-swap). Keyed by SLOT INDEX, not canvas, and only ever
54+
// bumped by an explicit user refresh — so reassigning a slot to another canvas
55+
// (navigation) leaves it untouched and still reuses the warm iframe.
56+
frameKeys: Record<number, number>;
5257

5358
register: (dashboardId: string, inputs: CanvasFrameInputs) => void;
5459
setRect: (dashboardId: string, rect: CanvasFrameRect) => void;
5560
activate: (dashboardId: string) => void;
5661
deactivate: (dashboardId: string) => void;
62+
remount: (dashboardId: string) => void;
5763
setMaxWarmFrames: (n: number) => void;
5864
}
5965

@@ -118,6 +124,7 @@ export const useCanvasFrameStore = create<CanvasFrameStore>()((set) => ({
118124
slots: [],
119125
activeDashboardId: null,
120126
maxWarmFrames: DEFAULT_MAX_WARM_FRAMES,
127+
frameKeys: {},
121128

122129
register: (dashboardId, inputs) =>
123130
set((s) => ({
@@ -168,6 +175,18 @@ export const useCanvasFrameStore = create<CanvasFrameStore>()((set) => ({
168175
s.activeDashboardId === dashboardId ? { activeDashboardId: null } : s,
169176
),
170177

178+
// Force a full remount of the canvas's mounted iframe (recreate the element +
179+
// its sandboxed document), for a manual "Refresh" that must recover from a
180+
// wedged frame, not just reload the document. No-op if the canvas has no slot.
181+
remount: (dashboardId) =>
182+
set((s) => {
183+
const idx = findSlot(s.slots, dashboardId);
184+
if (idx < 0) return s;
185+
return {
186+
frameKeys: { ...s.frameKeys, [idx]: (s.frameKeys[idx] ?? 0) + 1 },
187+
};
188+
}),
189+
171190
setMaxWarmFrames: (n) =>
172191
set((s) => ({
173192
maxWarmFrames: Math.max(1, n),

packages/ui/src/features/canvas/stores/canvasRefreshStore.ts

Lines changed: 0 additions & 27 deletions
This file was deleted.

0 commit comments

Comments
 (0)