From da0c7ec23b0d2a26f256a9379ede5ced6d86a3d5 Mon Sep 17 00:00:00 2001 From: mattakamatsu Date: Sat, 11 Jul 2026 20:28:45 -0700 Subject: [PATCH 1/9] Add {{dg-frame}} canvas frame embed (v0) with blank-page and remote-merge fixes Feature (per canvas-frame-embed spec v0): a dg-frame block embed that mounts the live canvas anchored to a named tldraw frame, referenced by shape id with name fallback. Frame-anchored embeds re-zoom to the frame on every mount (with a re-center button); frameless embeds remember a per-instance viewport via an instanceKey suffix on the canvas session storage key. Includes a two-step insert dialog (canvas page -> frame) wired to a slash command, and embeds default to tldraw focus mode (cmd+. restores the controls). Fixes found while testing the embed on real canvases: - tldrawStyles.ts hid ALL Roam blocks on any page hosting a tldraw container unless it was inside .dg-canvas-embed, so mounting a dg-frame embed blanked the whole page (silently - pure CSS). The exclusion list now covers every embed wrapper class. - useRoamStore's pull-watch handler applied the raw persisted store via applyDiff, which validates against the live schema without migrating, so any record predating MigrateNodeTypeToDiscourseNode (legacy node-uid shape types) threw an uncaught ValidationError inside mergeRemoteChanges and broke the canvas - for dg-canvas embeds too, triggered by any watched change on the canvas page (including its own first-mount State-block write). The merge logic now lives in canvasRemoteMerge.ts: incoming snapshots run through store.schema.migrateStoreSnapshot (same machinery as loadSnapshot), still-invalid records are dropped individually instead of aborting, failed merges are reported instead of thrown, bare legacy-format props no longer TypeError, and a stale merge timer can no longer outlive the stateId guard and clobber just-saved local edits. - refreshConfigTree now tolerates phantom null children in config-page pulls, which previously aborted the entire extension load. Verified live in plugin-testing-akamatsulab2 (repro pages seeded with a raw legacy-typed shape reproduced the exact ValidationError pre-fix; post-fix the same triggers merge cleanly, migrating records in flight). Unit tests cover parsing, session-key instancing, and the remote merge against real tldraw stores. Co-Authored-By: Claude Fable 5 --- .../components/canvas/CanvasFrameEmbed.tsx | 195 ++++++++++++++ .../canvas/CanvasFrameEmbedDialog.tsx | 207 +++++++++++++++ apps/roam/src/components/canvas/Tldraw.tsx | 67 ++++- .../components/canvas/canvasRemoteMerge.ts | 219 ++++++++++++++++ .../src/components/canvas/tldrawStyles.ts | 8 +- .../src/components/canvas/useRoamStore.ts | 182 +++---------- .../utils/__tests__/canvasRemoteMerge.test.ts | 241 ++++++++++++++++++ .../__tests__/canvasSessionState.test.ts | 26 ++ .../src/utils/__tests__/dgFrameEmbed.test.ts | 81 ++++++ apps/roam/src/utils/canvasSessionState.ts | 15 +- apps/roam/src/utils/dgFrameEmbed.ts | 56 ++++ .../utils/initializeObserversAndListeners.ts | 7 + apps/roam/src/utils/refreshConfigTree.ts | 41 ++- apps/roam/src/utils/registerSlashCommands.ts | 17 ++ 14 files changed, 1202 insertions(+), 160 deletions(-) create mode 100644 apps/roam/src/components/canvas/CanvasFrameEmbed.tsx create mode 100644 apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx create mode 100644 apps/roam/src/components/canvas/canvasRemoteMerge.ts create mode 100644 apps/roam/src/utils/__tests__/canvasRemoteMerge.test.ts create mode 100644 apps/roam/src/utils/__tests__/dgFrameEmbed.test.ts create mode 100644 apps/roam/src/utils/dgFrameEmbed.ts diff --git a/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx new file mode 100644 index 000000000..305e316db --- /dev/null +++ b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx @@ -0,0 +1,195 @@ +import React, { useCallback, useRef, useState } from "react"; +import ExtensionApiContextProvider from "roamjs-components/components/ExtensionApiContext"; +import { OnloadArgs } from "roamjs-components/types"; +import renderWithUnmount from "roamjs-components/util/renderWithUnmount"; +import getBlockUidFromTarget from "roamjs-components/dom/getBlockUidFromTarget"; +import getTextByBlockUid from "roamjs-components/queries/getTextByBlockUid"; +import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; +import type { Editor, TLFrameShape, TLShapeId } from "tldraw"; +import { TldrawCanvas, type CanvasEmbedOptions } from "./Tldraw"; +import { parseDgFrameEmbed } from "~/utils/dgFrameEmbed"; + +const FRAME_ZOOM_INSET = 16; + +type FrameRef = { name?: string; shapeId?: string }; + +// Resolve a frame by shape id first (stable across renames and moves), then fall +// back to a case-insensitive, trimmed name match. Duplicate names resolve to the +// first match with a warning. Returns null if neither path finds a frame. +const resolveFrameShape = ( + editor: Editor, + frame: FrameRef, +): TLFrameShape | null => { + if (frame.shapeId) { + const byId = editor.getShape(frame.shapeId as TLShapeId); + if (byId?.type === "frame") return byId; + } + + if (frame.name) { + const target = frame.name.trim().toLowerCase(); + const matches = editor + .getCurrentPageShapes() + .filter( + (shape): shape is TLFrameShape => + shape.type === "frame" && + ((shape as TLFrameShape).props.name ?? "").trim().toLowerCase() === + target, + ); + if (matches.length > 1) { + // eslint-disable-next-line no-console + console.warn( + `dg-frame: multiple frames named "${frame.name}"; using the first match.`, + ); + } + if (matches[0]) return matches[0]; + } + + return null; +}; + +// Frame always wins: zoom to the frame's current bounds so the embed tracks the +// frame even when it has been moved or resized on the canvas since last mount. +const zoomToFrame = (editor: Editor, frame: FrameRef): boolean => { + const shape = resolveFrameShape(editor, frame); + if (!shape) return false; + const bounds = editor.getShapePageBounds(shape); + if (!bounds) return false; + editor.zoomToBounds(bounds, { inset: FRAME_ZOOM_INSET }); + return true; +}; + +const CanvasEmbedPlaceholder = ({ message }: { message: string }) => ( +
+ {message} +
+); + +const CanvasFrameEmbed = ({ + title, + blockUid, + frame, +}: { + title: string; + blockUid: string; + frame: FrameRef | null; +}) => { + const editorRef = useRef(null); + const [frameMissing, setFrameMissing] = useState(false); + + const handleEditorMount = useCallback( + (editor: Editor) => { + editorRef.current = editor; + if (!frame) return; + // Defer one frame so tldraw has measured the embed's viewport before we + // compute a viewport-relative zoom on first mount. + requestAnimationFrame(() => { + setFrameMissing(!zoomToFrame(editor, frame)); + }); + }, + [frame], + ); + + const handleRecenter = useCallback(() => { + if (editorRef.current && frame) zoomToFrame(editorRef.current, frame); + }, [frame]); + + // Frame-anchored: manage the camera ourselves (no session persistence). + // Frameless: remember this embed's viewport independently, keyed by block uid. + // Both start in focus mode (chrome hidden; cmd+. brings the controls back). + const embedOptions: CanvasEmbedOptions = frame + ? { + disableSessionPersistence: true, + onEditorMount: handleEditorMount, + defaultFocusMode: true, + } + : { instanceKey: blockUid, defaultFocusMode: true }; + + return ( +
+ + {frame && ( + + )} + {frame && frameMissing && ( +
+ Frame {frame.name ? `“${frame.name}” ` : ""}not found on [[ + {title}]] +
+ )} +
+ ); +}; + +export const renderCanvasFrameEmbed = ( + button: HTMLElement, + onloadArgs: OnloadArgs, +) => { + button.hidden = true; + + if (!button.parentElement) return; + + const blockUid = getBlockUidFromTarget(button); + if (!blockUid) return; + + const blockText = getTextByBlockUid(blockUid); + const parsed = blockText ? parseDgFrameEmbed(blockText) : null; + if (!parsed) return; + + const { title, frameName, frameShapeId } = parsed; + + const pageUid = getPageUidByPageTitle(title); + if (!pageUid) { + const wrapper = document.createElement("div"); + button.parentElement.appendChild(wrapper); + renderWithUnmount( + , + wrapper, + ); + return; + } + + const frame: FrameRef | null = + frameName || frameShapeId + ? { name: frameName, shapeId: frameShapeId } + : null; + + const wrapper = document.createElement("div"); + wrapper.className = "dg-frame-embed my-2 w-full overflow-hidden rounded-md"; + wrapper.style.height = "400px"; + wrapper.onmousedown = (e: MouseEvent) => e.stopPropagation(); + button.parentElement.appendChild(wrapper); + + renderWithUnmount( + + + , + wrapper, + ); +}; diff --git a/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx b/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx new file mode 100644 index 000000000..72f3abcc0 --- /dev/null +++ b/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx @@ -0,0 +1,207 @@ +import React, { useCallback, useEffect, useMemo, useState } from "react"; +import { Button, Dialog, MenuItem } from "@blueprintjs/core"; +import AutocompleteInput from "roamjs-components/components/AutocompleteInput"; +import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; +import renderOverlay, { + RoamOverlayProps, +} from "roamjs-components/util/renderOverlay"; +import type { TLFrameShape, TLShape } from "tldraw"; +import { getCanvasPageTitles } from "~/utils/isCanvasPage"; +import { getRoamCanvasSnapshot } from "./useRoamStore"; + +export type DgFrameEmbedSelection = { + title: string; + frameName?: string; + frameShapeId?: string; +}; + +type CanvasFrameEmbedDialogProps = { + onSelect: (selection: DgFrameEmbedSelection) => void; +}; + +type FrameOption = { id: string; name: string; childCount: number }; + +// Enumerate frames on a canvas without mounting an editor, by reading the +// persisted store snapshot from block props and filtering for frame shapes. +// Frames are default tldraw shapes, so they survive even with no custom shape +// utils passed. Sync-mode canvases may show a slightly stale list — acceptable +// for a picker. Any read/upgrade failure degrades to "whole canvas only". +const getCanvasFrames = (pageUid: string): FrameOption[] => { + try { + const snapshot = getRoamCanvasSnapshot({ + pageUid, + migrations: [], + customShapeUtils: [], + customBindingUtils: [], + }); + if (!snapshot) return []; + + const records = Object.values(snapshot.store); + + const childCounts = new Map(); + for (const record of records) { + if (record.typeName !== "shape") continue; + const { parentId } = record; + childCounts.set(parentId, (childCounts.get(parentId) ?? 0) + 1); + } + + return records + .filter( + (record): record is TLFrameShape => + record.typeName === "shape" && (record as TLShape).type === "frame", + ) + .map((frame) => ({ + id: frame.id, + name: (frame.props.name ?? "").trim(), + childCount: childCounts.get(frame.id) ?? 0, + })); + } catch { + return []; + } +}; + +const CanvasFrameEmbedDialog = ({ + isOpen, + onClose, + onSelect, +}: RoamOverlayProps) => { + const [canvasPages, setCanvasPages] = useState(null); + const [selectedPage, setSelectedPage] = useState(null); + + useEffect(() => { + void getCanvasPageTitles().then(setCanvasPages); + }, []); + + const frames = useMemo( + () => + selectedPage ? getCanvasFrames(getPageUidByPageTitle(selectedPage)) : [], + [selectedPage], + ); + + const handlePickPage = useCallback( + (title: string) => { + if (canvasPages?.includes(title)) setSelectedPage(title); + }, + [canvasPages], + ); + + const handleSelectWholeCanvas = useCallback(() => { + if (!selectedPage) return; + onSelect({ title: selectedPage }); + onClose(); + }, [selectedPage, onSelect, onClose]); + + const handleSelectFrame = useCallback( + (frame: FrameOption) => { + if (!selectedPage) return; + onSelect({ + title: selectedPage, + frameName: frame.name || undefined, + frameShapeId: frame.id, + }); + onClose(); + }, + [selectedPage, onSelect, onClose], + ); + + const renderPageStep = () => { + if (canvasPages === null) + return ( +
Loading canvas pages...
+ ); + if (canvasPages.length === 0) + return ( +
No canvas pages found
+ ); + return ( + ( + { + event.preventDefault(); + handlePickPage(item); + }} + text={item} + /> + )} + /> + ); + }; + + const renderFrameStep = () => ( +
+
+ + {selectedPage} + + +
+
+ + {frames.length === 0 ? ( +
+ No frames found on this canvas. +
+ ) : ( + frames.map((frame) => ( + handleSelectFrame(frame)} + /> + )) + )} +
+
+ ); + + return ( + +
+ {selectedPage ? renderFrameStep() : renderPageStep()} +
+
+ ); +}; + +export const renderCanvasFrameEmbedDialog = ( + props: CanvasFrameEmbedDialogProps, +) => + renderOverlay({ + // eslint-disable-next-line @typescript-eslint/naming-convention + Overlay: CanvasFrameEmbedDialog, + props, + }); diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index c619370c4..420997dc9 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/apps/roam/src/components/canvas/Tldraw.tsx @@ -190,7 +190,33 @@ export const isPageUid = (uid: string) => ":node/title" ]; -export const TldrawCanvas = ({ title }: { title: string }) => { +// Options for mounting the canvas inside a block embed (`{{dg-frame: ...}}`). +// Left undefined for main-page and sidebar canvases, whose behavior is unchanged. +export type CanvasEmbedOptions = { + // When set (the embed's block uid), viewport/session state is remembered per + // embed instance instead of per canvas page — so two embeds of the same canvas + // hold independent viewports. Used by frameless embeds. + instanceKey?: string; + // Frame-anchored embeds manage their own camera (zoom-to-frame on every mount) + // and intentionally do not persist panning across remounts, so they opt out of + // the per-page/per-instance session persistence entirely. + disableSessionPersistence?: boolean; + // Start the editor in focus mode (what cmd+. toggles), hiding the tldraw + // menus/toolbar chrome — block embeds are for viewing first, so they default + // to a clean surface and the user can cmd+. the controls back. + defaultFocusMode?: boolean; + // Invoked once the embedded editor has mounted; frame resolution + zoom lives + // in the caller (CanvasFrameEmbed) so this component stays frame-agnostic. + onEditorMount?: (editor: Editor) => void; +}; + +export const TldrawCanvas = ({ + title, + embedOptions, +}: { + title: string; + embedOptions?: CanvasEmbedOptions; +}) => { // In Roam, canvas identity is currently keyed by the page UID. // Room sync is graph/page encoded as an opaque base64url token. const pageUid = useMemo(() => getPageUidByPageTitle(title), [title]); @@ -255,6 +281,7 @@ export const TldrawCanvas = ({ title }: { title: string }) => { pageUid={pageUid} canvasSyncMode={canvasSyncMode} onCanvasSyncModeChange={onCanvasSyncModeChange} + embedOptions={embedOptions} /> ); } @@ -265,6 +292,7 @@ export const TldrawCanvas = ({ title }: { title: string }) => { pageUid={pageUid} canvasSyncMode={canvasSyncMode} onCanvasSyncModeChange={onCanvasSyncModeChange} + embedOptions={embedOptions} /> ); }; @@ -329,11 +357,13 @@ const TldrawCanvasRoam = ({ pageUid, canvasSyncMode, onCanvasSyncModeChange, + embedOptions, }: { title: string; pageUid: string; canvasSyncMode: CanvasSyncMode; onCanvasSyncModeChange: (mode: CanvasSyncMode) => void; + embedOptions?: CanvasEmbedOptions; }) => { return ( ); }; @@ -352,11 +383,13 @@ const TldrawCanvasCloudflare = ({ pageUid, canvasSyncMode, onCanvasSyncModeChange, + embedOptions, }: { title: string; pageUid: string; canvasSyncMode: CanvasSyncMode; onCanvasSyncModeChange: (mode: CanvasSyncMode) => void; + embedOptions?: CanvasEmbedOptions; }) => { return ( ); }; @@ -377,6 +411,7 @@ const TldrawCanvasShared = ({ isCloudflareSync, canvasSyncMode, onCanvasSyncModeChange, + embedOptions, }: { title: string; pageUid: string; @@ -384,6 +419,7 @@ const TldrawCanvasShared = ({ isCloudflareSync: boolean; canvasSyncMode: CanvasSyncMode; onCanvasSyncModeChange: (mode: CanvasSyncMode) => void; + embedOptions?: CanvasEmbedOptions; }) => { const appRef = useRef(null); const lastInsertRef = useRef(); @@ -1041,13 +1077,18 @@ const TldrawCanvasShared = ({ components={editorComponents} store={store} onMount={(app) => { + // Frame-anchored embeds re-zoom to their frame on every mount and + // do not persist panning, so they opt out of session persistence. const unregisterCanvasSessionStatePersistence = - registerCanvasSessionStatePersistence({ - editor: app, - graphName: window.roamAlphaAPI.graph.name, - userUid: window.roamAlphaAPI.user.uid() || "", - pageUid, - }); + embedOptions?.disableSessionPersistence + ? () => {} + : registerCanvasSessionStatePersistence({ + editor: app, + graphName: window.roamAlphaAPI.graph.name, + userUid: window.roamAlphaAPI.user.uid() || "", + pageUid, + instanceKey: embedOptions?.instanceKey, + }); if (process.env.NODE_ENV !== "production") { if (!window.tldrawApps) window.tldrawApps = {}; @@ -1136,6 +1177,18 @@ const TldrawCanvasShared = ({ } }); + if ( + embedOptions?.defaultFocusMode && + !app.getInstanceState().isFocusMode + ) { + app.updateInstanceState({ isFocusMode: true }); + } + + // Hand the mounted editor back to an embed wrapper (frame + // resolution + zoom-to-frame). Runs after the store is loaded and + // shapes are present, so frame bounds are resolvable. + embedOptions?.onEditorMount?.(app); + return () => { unregisterCanvasSessionStatePersistence(); }; diff --git a/apps/roam/src/components/canvas/canvasRemoteMerge.ts b/apps/roam/src/components/canvas/canvasRemoteMerge.ts new file mode 100644 index 000000000..30f9406de --- /dev/null +++ b/apps/roam/src/components/canvas/canvasRemoteMerge.ts @@ -0,0 +1,219 @@ +// Merging persisted canvas state from Roam block props into a live tldraw +// store (the pull-watch "remote changes" path of useRoamStore). +// +// The persisted store is raw: it may predate the live schema (e.g. legacy +// node-uid shape types that MigrateNodeTypeToDiscourseNode retypes to +// "discourse-node" on load). `store.applyDiff` validates records against the +// live schema but never migrates them, so the incoming state must run through +// the same migration machinery `loadSnapshot` uses on mount before diffing — +// otherwise a single stale record throws a ValidationError inside +// `mergeRemoteChanges` and tears the canvas down. +import { TLRecord } from "@tldraw/tlschema"; +import { SerializedStore } from "@tldraw/store"; +import { + getIndices, + sortByIndex, + TLShape, + TLStore, + TLStoreSnapshot, +} from "tldraw"; +import { LEGACY_SCHEMA } from "~/data/legacyTldrawSchema"; + +export const isTLStoreSnapshot = (value: unknown): value is TLStoreSnapshot => { + return ( + typeof value === "object" && + value !== null && + "store" in value && + "schema" in value + ); +}; + +export const fixShapeIndices = ( + data: SerializedStore, +): SerializedStore => { + const shapes = Object.values(data).filter( + (record): record is TLShape => record.typeName === "shape", + ); + + const sortedShapes = shapes.sort((a, b) => { + if (a.index !== undefined && b.index !== undefined) { + return sortByIndex(a, b); + } + return a.id.localeCompare(b.id); + }); + + const newIndices = getIndices(shapes.length); + + const fixedShapes = sortedShapes.map((shape, i) => ({ + ...shape, + index: newIndices[i], + })); + + return Object.fromEntries( + Object.entries(data).map(([key, value]) => { + if (value.typeName === "shape") { + const updatedShape = fixedShapes.find((s) => s.id === value.id); + return [key, (updatedShape || value) as TLRecord]; + } + return [key, value]; + }), + ); +}; + +export const filterUserRecords = (data: SerializedStore) => { + return Object.fromEntries( + Object.entries(data).filter(([key]) => { + return !/^(user_presence|camera|instance|instance_page_state|user|user_document):/.test( + key, + ); + }), + ); +}; + +const personalRecordTypes = new Set([ + "camera", + "instance", + "instance_page_state", +]); + +const pruneState = (state: SerializedStore) => + Object.fromEntries( + Object.entries(state).filter( + ([_, record]) => !personalRecordTypes.has(record.typeName), + ), + ); + +const diffObjects = ( + oldRecord: Record, + newRecord: Record, +): Record => { + const allKeys = Array.from( + new Set(Object.keys(oldRecord).concat(Object.keys(newRecord))), + ); + return Object.fromEntries( + allKeys + .map((key) => { + const oldValue = oldRecord[key]; + const newValue = newRecord[key]; + if (typeof oldValue !== typeof newValue) { + return [key, newValue]; + } + if ( + typeof oldValue === "object" && + oldValue !== null && + newValue !== null + ) { + const diffed = diffObjects(oldValue, newValue); + if (Object.keys(diffed).length) { + return [key, diffed]; + } + return null; + } + if (oldValue !== newValue) { + return [key, newValue]; + } + return null; + }) + .filter((e): e is [string, any] => !!e), + ); +}; + +export const calculateDiff = ( + _newState: SerializedStore, + _oldState: SerializedStore, +) => { + const newState = pruneState(_newState); + const oldState = pruneState(_oldState); + return { + added: Object.fromEntries( + Object.keys(newState) + .filter((id) => !oldState[id]) + .map((id) => [id, newState[id]]), + ), + removed: Object.fromEntries( + Object.keys(oldState) + .filter((id) => !newState[id]) + .map((key) => [key, oldState[key]]), + ), + updated: Object.fromEntries( + Object.keys(newState) + .map((id) => { + const oldRecord = oldState[id]; + const newRecord = newState[id]; + if (!oldRecord || !newRecord) { + return null; + } + + const diffed = diffObjects(oldRecord, newRecord); + if (Object.keys(diffed).length) { + return [id, [oldRecord, newRecord]]; + } + return null; + }) + .filter((e): e is [string, any] => !!e), + ), + }; +}; + +export type MergeRemoteCanvasStateResult = + | { type: "applied"; droppedRecordIds: string[] } + | { type: "migration-failed" } + | { type: "apply-failed"; error: unknown }; + +export const mergeRemoteCanvasState = ({ + store, + remoteTldraw, +}: { + store: TLStore; + // The raw `props["roamjs-query-builder"].tldraw` value: a modern + // `{ store, schema }` snapshot, or a bare legacy serialized store. + remoteTldraw: TLStoreSnapshot | SerializedStore; +}): MergeRemoteCanvasStateResult => { + // Legacy-format data gets the same pre-processing the mount path + // (upgradeLegacyStoreSnapshot) applies before migrating. + const snapshot: TLStoreSnapshot = isTLStoreSnapshot(remoteTldraw) + ? remoteTldraw + : { + store: fixShapeIndices(filterUserRecords(remoteTldraw)), + schema: LEGACY_SCHEMA, + }; + + const migration = store.schema.migrateStoreSnapshot(snapshot); + if (migration.type !== "success") { + return { type: "migration-failed" }; + } + + // Keep only document-scoped records (what loadSnapshot keeps) and drop any + // record the live schema still rejects after migration — e.g. a shape whose + // node type is missing from this graph's config, which no migration can + // retype. A dropped record must not be treated as remotely deleted, so its + // id is also excluded from the diff's `removed` set below. + const droppedRecordIds: string[] = []; + const incoming: SerializedStore = Object.fromEntries( + Object.entries(migration.value).filter(([id, record]) => { + if (!store.scopedTypes.document.has(record.typeName)) return false; + try { + store.schema.validateRecord(store, record, "createRecord", null); + return true; + } catch { + droppedRecordIds.push(id); + return false; + } + }), + ); + + try { + store.mergeRemoteChanges(() => { + const currentState = store.getSnapshot(); + const diff = calculateDiff(incoming, currentState.store); + droppedRecordIds.forEach((id) => { + delete diff.removed[id]; + }); + store.applyDiff(diff); + }); + } catch (error) { + return { type: "apply-failed", error }; + } + + return { type: "applied", droppedRecordIds }; +}; diff --git a/apps/roam/src/components/canvas/tldrawStyles.ts b/apps/roam/src/components/canvas/tldrawStyles.ts index 03a5265e7..81e021d3e 100644 --- a/apps/roam/src/components/canvas/tldrawStyles.ts +++ b/apps/roam/src/components/canvas/tldrawStyles.ts @@ -1,12 +1,14 @@ // tldrawStyles.ts because some of these styles need to be inlined export default /* css */ ` - /* Hide Roam Blocks only when a full-page canvas is present (not embedded) */ - .roam-article:has(.roamjs-tldraw-canvas-container:not(.dg-canvas-embed *)) .rm-block-children { + /* Hide Roam Blocks only when a full-page canvas is present (not embedded). + Every block-embed wrapper class must be excluded here, or mounting that + embed hides ALL blocks on the host page. */ + .roam-article:has(.roamjs-tldraw-canvas-container:not(.dg-canvas-embed *, .dg-frame-embed *)) .rm-block-children { display: none; } /* Hide Roam Blocks in sidebar when a full-page canvas is present (not embedded) */ - .rm-sidebar-outline:has(.roamjs-tldraw-canvas-container:not(.dg-canvas-embed *)) .rm-block-children { + .rm-sidebar-outline:has(.roamjs-tldraw-canvas-container:not(.dg-canvas-embed *, .dg-frame-embed *)) .rm-block-children { display: none; } diff --git a/apps/roam/src/components/canvas/useRoamStore.ts b/apps/roam/src/components/canvas/useRoamStore.ts index 78989afaa..6753794b2 100644 --- a/apps/roam/src/components/canvas/useRoamStore.ts +++ b/apps/roam/src/components/canvas/useRoamStore.ts @@ -11,74 +11,28 @@ import { TLAnyBindingUtilConstructor, TLAnyShapeUtilConstructor, } from "@tldraw/editor"; -import { SerializedStore, StoreSnapshot } from "@tldraw/store"; +import { SerializedStore } from "@tldraw/store"; import { defaultBindingUtils, defaultShapeUtils, - getIndices, loadSnapshot, MigrationSequence, - sortByIndex, - TLShape, TLStoreSnapshot, TLStore, } from "tldraw"; import { AddPullWatch } from "roamjs-components/types"; import { LEGACY_SCHEMA } from "~/data/legacyTldrawSchema"; import internalError from "~/utils/internalError"; +import { + isTLStoreSnapshot, + filterUserRecords, + fixShapeIndices, + mergeRemoteCanvasState, +} from "./canvasRemoteMerge"; const THROTTLE = 350; -export const isTLStoreSnapshot = (value: unknown): value is TLStoreSnapshot => { - return ( - typeof value === "object" && - value !== null && - "store" in value && - "schema" in value - ); -}; - -const fixShapeIndices = ( - data: SerializedStore, -): SerializedStore => { - const shapes = Object.values(data).filter( - (record): record is TLShape => record.typeName === "shape", - ); - - const sortedShapes = shapes.sort((a, b) => { - if (a.index !== undefined && b.index !== undefined) { - return sortByIndex(a, b); - } - return a.id.localeCompare(b.id); - }); - - const newIndices = getIndices(shapes.length); - - const fixedShapes = sortedShapes.map((shape, i) => ({ - ...shape, - index: newIndices[i], - })); - - return Object.fromEntries( - Object.entries(data).map(([key, value]) => { - if (value.typeName === "shape") { - const updatedShape = fixedShapes.find((s) => s.id === value.id); - return [key, (updatedShape || value) as TLRecord]; - } - return [key, value]; - }), - ); -}; - -const filterUserRecords = (data: SerializedStore) => { - return Object.fromEntries( - Object.entries(data).filter(([key]) => { - return !/^(user_presence|camera|instance|instance_page_state|user|user_document):/.test( - key, - ); - }), - ); -}; +export { isTLStoreSnapshot } from "./canvasRemoteMerge"; const createCanvasStore = ({ migrations, @@ -349,12 +303,6 @@ export const useRoamStore = ({ pageUid, ]); - const personalRecordTypes = new Set([ - "camera", - "instance", - "instance_page_state", - ]); - const performUpgrade = () => { if (!oldData) return; try { @@ -404,84 +352,6 @@ export const useRoamStore = ({ } }; - const pruneState = (state: SerializedStore) => - Object.fromEntries( - Object.entries(state).filter( - ([_, record]) => !personalRecordTypes.has(record.typeName), - ), - ); - - const diffObjects = ( - oldRecord: Record, - newRecord: Record, - ): Record => { - const allKeys = Array.from( - new Set(Object.keys(oldRecord).concat(Object.keys(newRecord))), - ); - return Object.fromEntries( - allKeys - .map((key) => { - const oldValue = oldRecord[key]; - const newValue = newRecord[key]; - if (typeof oldValue !== typeof newValue) { - return [key, newValue]; - } - if ( - typeof oldValue === "object" && - oldValue !== null && - newValue !== null - ) { - const diffed = diffObjects(oldValue, newValue); - if (Object.keys(diffed).length) { - return [key, diffed]; - } - return null; - } - if (oldValue !== newValue) { - return [key, newValue]; - } - return null; - }) - .filter((e): e is [string, any] => !!e), - ); - }; - const calculateDiff = ( - _newState: SerializedStore, - _oldState: SerializedStore, - ) => { - const newState = pruneState(_newState); - const oldState = pruneState(_oldState); - return { - added: Object.fromEntries( - Object.keys(newState) - .filter((id) => !oldState[id]) - .map((id) => [id, newState[id]]), - ), - removed: Object.fromEntries( - Object.keys(oldState) - .filter((id) => !newState[id]) - .map((key) => [key, oldState[key]]), - ), - updated: Object.fromEntries( - Object.keys(newState) - .map((id) => { - const oldRecord = oldState[id]; - const newRecord = newState[id]; - if (!oldRecord || !newRecord) { - return null; - } - - const diffed = diffObjects(oldRecord, newRecord); - if (Object.keys(diffed).length) { - return [id, [oldRecord, newRecord]]; - } - return null; - }) - .filter((e): e is [string, any] => !!e), - ), - }; - }; - // Remote Changes useEffect(() => { const pullWatchProps: Parameters = [ @@ -492,18 +362,42 @@ export const useRoamStore = ({ (after?.[":block/props"] || {}) as json, ) as Record; const rjsqb = props["roamjs-query-builder"] as Record; + // Any pending merge was computed from an older props state than this + // invocation, so always cancel it — including when the newest state is + // our own save. (Returning early while a stale timer kept running used + // to let an outdated snapshot clobber just-saved local edits.) + clearTimeout(deserializeRef.current); const propsStateId = rjsqb?.stateId as string; if (localStateIds.current.some((s) => s === propsStateId)) return; - const newState = rjsqb?.tldraw as StoreSnapshot; + const newState = rjsqb?.tldraw as + | TLStoreSnapshot + | SerializedStore; if (!newState) return; - clearTimeout(deserializeRef.current); deserializeRef.current = window.setTimeout(() => { if (!store) return; - store.mergeRemoteChanges(() => { - const currentState = store.getSnapshot(); - const diff = calculateDiff(newState.store, currentState.store); - store.applyDiff(diff); + const result = mergeRemoteCanvasState({ + store, + remoteTldraw: newState, }); + if (result.type === "applied") { + if (result.droppedRecordIds.length) { + console.warn( + "Canvas remote merge skipped records the current schema rejects:", + result.droppedRecordIds, + ); + } + } else { + // Skipping a merge leaves the canvas slightly stale until the next + // save; that beats letting a ValidationError tear down the page. + internalError({ + error: + result.type === "apply-failed" + ? result.error + : new Error("Failed to migrate remote canvas snapshot"), + type: "Failed to merge remote canvas changes", + context: { pageUid }, + }); + } }, THROTTLE); }, ]; diff --git a/apps/roam/src/utils/__tests__/canvasRemoteMerge.test.ts b/apps/roam/src/utils/__tests__/canvasRemoteMerge.test.ts new file mode 100644 index 000000000..6896a7e7f --- /dev/null +++ b/apps/roam/src/utils/__tests__/canvasRemoteMerge.test.ts @@ -0,0 +1,241 @@ +import { describe, expect, it } from "vitest"; +import { + BaseBoxShapeUtil, + DocumentRecordType, + PageRecordType, + IndexKey, + T, + TLBaseShape, + TLDOCUMENT_ID, + TLStore, + TLStoreSnapshot, + createMigrationIds, + createMigrationSequence, + createTLStore, + defaultBindingUtils, + defaultShapeUtils, + loadSnapshot, +} from "tldraw"; +import type { SerializedStore } from "@tldraw/store"; +import type { TLRecord } from "@tldraw/tlschema"; +import { + calculateDiff, + mergeRemoteCanvasState, +} from "~/components/canvas/canvasRemoteMerge"; + +// Mirrors the production situation that produced the dg-frame blank-page bug: +// persisted canvas stores can hold shapes whose type is a legacy node uid +// (e.g. "lxCvhQ034") that a store-level migration retypes on load, but the +// live schema itself does not register. `loadSnapshot` migrates; a raw +// `applyDiff` of the persisted store does not, and throws a ValidationError. +const LEGACY_NODE_TYPE = "lxCvhQ034test"; +const MIGRATED_NODE_TYPE = "discourse-node-test"; + +type TestNodeShape = TLBaseShape< + typeof MIGRATED_NODE_TYPE, + { w: number; h: number; nodeTypeId: string } +>; + +class TestDiscourseNodeUtil extends BaseBoxShapeUtil { + static override type = MIGRATED_NODE_TYPE; + static override props = { + w: T.number, + h: T.number, + nodeTypeId: T.string, + }; + override getDefaultProps() { + return { w: 100, h: 50, nodeTypeId: "" }; + } + override component() { + return null; + } + override indicator() { + return null; + } +} + +const versions = createMigrationIds("com.test.discourse-node", { + MigrateNodeTypeToDiscourseNode: 1, +}); + +// Same shape as the real MigrateNodeTypeToDiscourseNode migration in +// discourseRelationMigrations.ts: retype a node-uid-typed shape to the +// registered discourse-node type, keeping the uid in props. +const testMigrations = createMigrationSequence({ + sequenceId: "com.test.discourse-node", + retroactive: true, + sequence: [ + { + id: versions.MigrateNodeTypeToDiscourseNode, + scope: "record", + filter: (r) => + r.typeName === "shape" && + (r as { type?: string }).type === LEGACY_NODE_TYPE, + up: (shape) => { + const s = shape as unknown as { + type: string; + props: Record; + }; + s.props.nodeTypeId = s.type; + s.type = MIGRATED_NODE_TYPE; + }, + }, + ], +}); + +const PAGE_ID = PageRecordType.createId("page"); +const LEGACY_SHAPE_ID = "shape:legacy1"; + +const makeLegacyShape = ( + overrides: Partial> = {}, +): Record => ({ + id: LEGACY_SHAPE_ID, + typeName: "shape", + type: LEGACY_NODE_TYPE, + x: 0, + y: 0, + rotation: 0, + index: "a2", + parentId: PAGE_ID, + isLocked: false, + opacity: 1, + meta: {}, + props: { w: 100, h: 50 }, + ...overrides, +}); + +// A persisted snapshot the way Roam block props hold it: records saved before +// the discourse migration sequence existed (schema without that sequence), so +// the legacy-typed shape is still present raw. +const makePersistedSnapshot = ( + extraRecords: Record[] = [], +): TLStoreSnapshot => { + const scratch = createTLStore({ + shapeUtils: defaultShapeUtils, + bindingUtils: defaultBindingUtils, + }); + scratch.put([ + DocumentRecordType.create({ id: TLDOCUMENT_ID }), + PageRecordType.create({ + id: PAGE_ID, + name: "Page 1", + index: "a1" as IndexKey, + }), + ]); + const snapshot = scratch.getStoreSnapshot(); + const store = { ...snapshot.store } as Record; + for (const record of [makeLegacyShape(), ...extraRecords]) { + store[record.id as string] = record; + } + return { + store: store as TLStoreSnapshot["store"], + schema: snapshot.schema, + }; +}; + +const makeLiveStore = (): TLStore => + createTLStore({ + shapeUtils: [...defaultShapeUtils, TestDiscourseNodeUtil], + bindingUtils: defaultBindingUtils, + migrations: [testMigrations], + }); + +const mountLiveStore = (snapshot: TLStoreSnapshot): TLStore => { + const store = makeLiveStore(); + loadSnapshot(store, snapshot); + return store; +}; + +const getShape = (store: TLStore, id: string) => + store.get(id as Parameters[0]) as unknown as + | { type: string; x: number; props: Record } + | undefined; + +describe("mergeRemoteCanvasState", () => { + it("mount path migrates the legacy-typed shape (sanity check)", () => { + const store = mountLiveStore(makePersistedSnapshot()); + const shape = getShape(store, LEGACY_SHAPE_ID); + expect(shape?.type).toBe(MIGRATED_NODE_TYPE); + expect(shape?.props.nodeTypeId).toBe(LEGACY_NODE_TYPE); + }); + + it("documents the original bug: raw applyDiff of the unmigrated store throws", () => { + const persisted = makePersistedSnapshot(); + const store = mountLiveStore(persisted); + expect(() => + store.mergeRemoteChanges(() => { + const diff = calculateDiff(persisted.store, store.getSnapshot().store); + store.applyDiff(diff); + }), + ).toThrow(); + }); + + it("applies an unmigrated remote snapshot by migrating it first", () => { + const persisted = makePersistedSnapshot(); + const store = mountLiveStore(persisted); + const result = mergeRemoteCanvasState({ store, remoteTldraw: persisted }); + expect(result).toEqual({ type: "applied", droppedRecordIds: [] }); + const shape = getShape(store, LEGACY_SHAPE_ID); + expect(shape?.type).toBe(MIGRATED_NODE_TYPE); + expect(shape?.props.nodeTypeId).toBe(LEGACY_NODE_TYPE); + }); + + it("propagates adds, moves, and removals from the remote snapshot", () => { + const store = mountLiveStore(makePersistedSnapshot()); + + const secondShape = makeLegacyShape({ id: "shape:legacy2", index: "a3" }); + const withChanges = makePersistedSnapshot([secondShape]); + (withChanges.store as unknown as Record>)[ + LEGACY_SHAPE_ID + ] = makeLegacyShape({ x: 250 }); + + const result = mergeRemoteCanvasState({ + store, + remoteTldraw: withChanges, + }); + expect(result.type).toBe("applied"); + expect(getShape(store, LEGACY_SHAPE_ID)?.x).toBe(250); + expect(getShape(store, "shape:legacy2")?.type).toBe(MIGRATED_NODE_TYPE); + + const withRemoval = makePersistedSnapshot(); + const removalResult = mergeRemoteCanvasState({ + store, + remoteTldraw: withRemoval, + }); + expect(removalResult.type).toBe("applied"); + expect(getShape(store, "shape:legacy2")).toBeUndefined(); + }); + + it("drops records the schema still rejects instead of failing the merge", () => { + const store = mountLiveStore(makePersistedSnapshot()); + + const hopeless = makeLegacyShape({ + id: "shape:unknown1", + type: "no-such-type-anywhere", + index: "a4", + }); + const moved = makeLegacyShape({ x: 99 }); + const snapshot = makePersistedSnapshot([hopeless]); + (snapshot.store as unknown as Record>)[ + LEGACY_SHAPE_ID + ] = moved; + + const result = mergeRemoteCanvasState({ store, remoteTldraw: snapshot }); + expect(result).toEqual({ + type: "applied", + droppedRecordIds: ["shape:unknown1"], + }); + // The good change still landed; the bad record never entered the store. + expect(getShape(store, LEGACY_SHAPE_ID)?.x).toBe(99); + expect(getShape(store, "shape:unknown1")).toBeUndefined(); + }); + + it("returns a result instead of throwing for bare legacy-format data", () => { + const store = mountLiveStore(makePersistedSnapshot()); + // Old code did `newState.store` on this and crashed with a TypeError. + const bareLegacyStore = {} as SerializedStore; + expect(() => + mergeRemoteCanvasState({ store, remoteTldraw: bareLegacyStore }), + ).not.toThrow(); + }); +}); diff --git a/apps/roam/src/utils/__tests__/canvasSessionState.test.ts b/apps/roam/src/utils/__tests__/canvasSessionState.test.ts index a1c2223ef..59034a0e2 100644 --- a/apps/roam/src/utils/__tests__/canvasSessionState.test.ts +++ b/apps/roam/src/utils/__tests__/canvasSessionState.test.ts @@ -78,6 +78,32 @@ describe("getCanvasSessionStorageKey", () => { }), ).toBe("dg:tldraw-session:v1:graph%2Fname:user%3A1:abc%20123"); }); + + it("appends an instance key so embeds of the same page stay independent", () => { + expect( + getCanvasSessionStorageKey({ + graphName: "graph/name", + userUid: "user:1", + pageUid: "abc 123", + instanceKey: "block-uid-1", + }), + ).toBe("dg:tldraw-session:v1:graph%2Fname:user%3A1:abc%20123:block-uid-1"); + }); + + it("is unchanged from the base key when no instance key is given", () => { + const base = getCanvasSessionStorageKey({ + graphName: "g", + userUid: "u", + pageUid: "p", + }); + const withUndefined = getCanvasSessionStorageKey({ + graphName: "g", + userUid: "u", + pageUid: "p", + instanceKey: undefined, + }); + expect(withUndefined).toBe(base); + }); }); describe("readCanvasSessionState", () => { diff --git a/apps/roam/src/utils/__tests__/dgFrameEmbed.test.ts b/apps/roam/src/utils/__tests__/dgFrameEmbed.test.ts new file mode 100644 index 000000000..8a995af13 --- /dev/null +++ b/apps/roam/src/utils/__tests__/dgFrameEmbed.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from "vitest"; +import { parseDgFrameEmbed, serializeDgFrameEmbed } from "~/utils/dgFrameEmbed"; + +describe("parseDgFrameEmbed", () => { + it("parses a frameless embed", () => { + expect(parseDgFrameEmbed("{{dg-frame: [[My Canvas]]}}")).toEqual({ + title: "My Canvas", + }); + }); + + it("parses a name-only embed", () => { + expect(parseDgFrameEmbed('{{dg-frame: [[My Canvas]] "Frame A"}}')).toEqual({ + title: "My Canvas", + frameName: "Frame A", + }); + }); + + it("parses a name + shape-id embed", () => { + expect( + parseDgFrameEmbed('{{dg-frame: [[My Canvas]] "Frame A" shape:aB1_c-2}}'), + ).toEqual({ + title: "My Canvas", + frameName: "Frame A", + frameShapeId: "shape:aB1_c-2", + }); + }); + + it("parses an id-only embed (name omitted)", () => { + expect( + parseDgFrameEmbed("{{dg-frame: [[My Canvas]] shape:aB1_c-2}}"), + ).toEqual({ + title: "My Canvas", + frameShapeId: "shape:aB1_c-2", + }); + }); + + it("is case-insensitive on the keyword and tolerates surrounding text", () => { + expect( + parseDgFrameEmbed('prefix {{DG-Frame: [[My Canvas]] "F"}} suffix'), + ).toMatchObject({ title: "My Canvas", frameName: "F" }); + }); + + it("returns null for a dg-canvas embed or unrelated text", () => { + expect(parseDgFrameEmbed("{{dg-canvas: [[My Canvas]]}}")).toBeNull(); + expect(parseDgFrameEmbed("just some text")).toBeNull(); + }); +}); + +describe("serializeDgFrameEmbed", () => { + it("round-trips a frameless embed", () => { + const text = serializeDgFrameEmbed({ title: "My Canvas" }); + expect(text).toBe("{{dg-frame: [[My Canvas]]}}"); + expect(parseDgFrameEmbed(text)).toEqual({ title: "My Canvas" }); + }); + + it("round-trips a fully-specified embed", () => { + const embed = { + title: "My Canvas", + frameName: "Frame A", + frameShapeId: "shape:aB1_c-2", + }; + const text = serializeDgFrameEmbed(embed); + expect(text).toBe('{{dg-frame: [[My Canvas]] "Frame A" shape:aB1_c-2}}'); + expect(parseDgFrameEmbed(text)).toEqual(embed); + }); + + it("collapses embedded double-quotes in the frame name so the token stays parseable", () => { + const text = serializeDgFrameEmbed({ + title: "My Canvas", + frameName: 'A "quoted" name', + frameShapeId: "shape:x", + }); + expect(text).toBe( + "{{dg-frame: [[My Canvas]] \"A 'quoted' name\" shape:x}}", + ); + expect(parseDgFrameEmbed(text)).toMatchObject({ + title: "My Canvas", + frameShapeId: "shape:x", + }); + }); +}); diff --git a/apps/roam/src/utils/canvasSessionState.ts b/apps/roam/src/utils/canvasSessionState.ts index 339c07eb8..2372e12e0 100644 --- a/apps/roam/src/utils/canvasSessionState.ts +++ b/apps/roam/src/utils/canvasSessionState.ts @@ -99,18 +99,26 @@ export const getCanvasSessionStorageKey = ({ graphName, userUid, pageUid, + instanceKey, }: { graphName: string; userUid: string; pageUid: string; + // Optional per-embed discriminator (the embed's block uid). When present, the + // viewport is remembered per embed instance instead of per canvas page, so two + // embeds of the same canvas hold independent viewports. Omitted for main-page + // and sidebar canvases, keeping their storage key (and behavior) unchanged. + instanceKey?: string; }): string => { - return [ + const parts = [ CANVAS_SESSION_STORAGE_PREFIX, `v${CANVAS_SESSION_STORAGE_VERSION}`, encodeStoragePart(graphName), encodeStoragePart(userUid || "anonymous"), encodeStoragePart(pageUid), - ].join(":"); + ]; + if (instanceKey) parts.push(encodeStoragePart(instanceKey)); + return parts.join(":"); }; export const readCanvasSessionState = ({ @@ -242,6 +250,7 @@ export const registerCanvasSessionStatePersistence = ({ graphName, userUid, pageUid, + instanceKey, storage = getCanvasSessionStorage(), saveDelayMs = CANVAS_SESSION_SAVE_DELAY_MS, }: { @@ -249,6 +258,7 @@ export const registerCanvasSessionStatePersistence = ({ graphName: string; userUid: string; pageUid: string; + instanceKey?: string; storage?: CanvasSessionStorage | null; saveDelayMs?: number; }): (() => void) => { @@ -258,6 +268,7 @@ export const registerCanvasSessionStatePersistence = ({ graphName, userUid, pageUid, + instanceKey, }); let saveTimeout: ReturnType | null = null; diff --git a/apps/roam/src/utils/dgFrameEmbed.ts b/apps/roam/src/utils/dgFrameEmbed.ts new file mode 100644 index 000000000..1fd69ed5c --- /dev/null +++ b/apps/roam/src/utils/dgFrameEmbed.ts @@ -0,0 +1,56 @@ +// Parsing/serialization for the `{{dg-frame: ...}}` block embed. +// +// Serialized forms (the frame argument is optional): +// {{dg-frame: [[Canvas Page Title]]}} frameless embed +// {{dg-frame: [[Canvas Page Title]] "Frame Name"}} name only (hand-written / self-healed) +// {{dg-frame: [[Canvas Page Title]] "Frame Name" shape:abc}} id-anchored (what the picker writes) +// {{dg-frame: [[Canvas Page Title]] shape:abc}} id only (name omitted) +// +// Frames are referenced by tldraw shape id (stable across renames and moves); +// the display name is carried alongside for human readability and as a +// resolution fallback. `shape:` is a plain token — we deliberately avoid Roam's +// `((...))` block-ref syntax so the embed round-trips through block text +// unambiguously. + +export type DgFrameEmbed = { + title: string; + frameName?: string; + // Full tldraw shape id, e.g. "shape:abc123" (tldraw ids use the nanoid + // alphabet: A-Z a-z 0-9 _ -). + frameShapeId?: string; +}; + +// Group 1: page title. Group 2: optional frame name. Group 3: optional shape id +// (captured with its `shape:` prefix, so it is the full tldraw id). +export const DG_FRAME_EMBED_REGEX = + /\{\{dg-frame:\s*\[\[(.+?)\]\](?:\s+"([^"]*)")?(?:\s+(shape:[A-Za-z0-9_-]+))?\s*\}\}/i; + +export const parseDgFrameEmbed = (blockText: string): DgFrameEmbed | null => { + const match = blockText.match(DG_FRAME_EMBED_REGEX); + if (!match) return null; + + const title = match[1].trim(); + if (!title) return null; + + const frameName = match[2]?.trim() || undefined; + const frameShapeId = match[3] || undefined; + + return { + title, + ...(frameName ? { frameName } : {}), + ...(frameShapeId ? { frameShapeId } : {}), + }; +}; + +export const serializeDgFrameEmbed = ({ + title, + frameName, + frameShapeId, +}: DgFrameEmbed): string => { + let inner = `dg-frame: [[${title}]]`; + // The name is only ever a readability/fallback hint (the id is authoritative), + // so collapse any embedded double-quote to keep the token unambiguous. + if (frameName) inner += ` "${frameName.replace(/"/g, "'")}"`; + if (frameShapeId) inner += ` ${frameShapeId}`; + return `{{${inner}}}`; +}; diff --git a/apps/roam/src/utils/initializeObserversAndListeners.ts b/apps/roam/src/utils/initializeObserversAndListeners.ts index 57804da57..f4e9e3249 100644 --- a/apps/roam/src/utils/initializeObserversAndListeners.ts +++ b/apps/roam/src/utils/initializeObserversAndListeners.ts @@ -50,6 +50,7 @@ import { getNodeTagStyles } from "~/utils/getDiscourseNodeColors"; import { renderPossibleDuplicates } from "~/components/VectorDuplicateMatches"; import { renderPublishNodeTitleButton } from "~/components/PublishNodeTitleButton"; import { renderCanvasEmbed } from "~/components/canvas/CanvasEmbed"; +import { renderCanvasFrameEmbed } from "~/components/canvas/CanvasFrameEmbed"; import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import getPageTitleByPageUid from "roamjs-components/queries/getPageTitleByPageUid"; import findDiscourseNode from "./findDiscourseNode"; @@ -170,6 +171,11 @@ export const initObservers = ({ render: (b) => renderCanvasEmbed(b, onloadArgs), }); + const canvasFrameEmbedObserver = createButtonObserver({ + attribute: "dg-frame", + render: (b) => renderCanvasFrameEmbed(b, onloadArgs), + }); + let batchedTagNodes: DiscourseNode[] | null = null; const getNodesForTagBatch = (): DiscourseNode[] => { if (batchedTagNodes === null) { @@ -469,6 +475,7 @@ export const initObservers = ({ pageTitleObserver, queryBlockObserver, canvasEmbedObserver, + canvasFrameEmbedObserver, graphOverviewExportObserver, nodeTagPopupButtonObserver, leftSidebarObserver, diff --git a/apps/roam/src/utils/refreshConfigTree.ts b/apps/roam/src/utils/refreshConfigTree.ts index 8d23401c8..8261eed94 100644 --- a/apps/roam/src/utils/refreshConfigTree.ts +++ b/apps/roam/src/utils/refreshConfigTree.ts @@ -1,13 +1,46 @@ -import getBasicTreeByParentUid from "roamjs-components/queries/getBasicTreeByParentUid"; import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import getDiscourseRelationLabels from "./getDiscourseRelationLabels"; import discourseConfigRef from "./discourseConfigRef"; import registerDiscourseDatalogTranslators from "./registerDiscourseDatalogTranslators"; import { unregisterDatalogTranslator } from "./conditionToDatalog"; -import type { PullBlock } from "roamjs-components/types/native"; +import type { PullBlock, RoamBasicNode } from "roamjs-components/types/native"; import { DISCOURSE_CONFIG_PAGE_TITLE } from "~/data/constants"; import type { SettingsSnapshot } from "~/components/settings/utils/accessors"; +// Null-safe replacement for roamjs-components' getBasicTreeByParentUid. +// +// Some graphs carry a `:block/children` edge from a page to a "phantom" block +// whose pull returns null (no :block/uid / :block/order / :block/string). The +// upstream helper sorts the raw pull rows by `:block/order` without filtering, +// so a single dangling child throws `Cannot read properties of null` — and, +// because refreshConfigTree runs first in onload, that aborts the entire +// extension load (no commands, menus, or UI register). Dropping the nulls +// preserves every real config block and lets the plugin load. +const sortBasicNodesSafe = (nodes: (PullBlock | null)[]): RoamBasicNode[] => + nodes + .filter((n): n is PullBlock => n !== null) + .sort( + (a, b) => + ((a[":block/order"] as number) || 0) - + ((b[":block/order"] as number) || 0), + ) + .map((node) => ({ + children: sortBasicNodesSafe( + (node[":block/children"] as (PullBlock | null)[]) || [], + ), + uid: node[":block/uid"] || "", + text: node[":block/string"] || "", + })); + +const getBasicTreeByParentUidSafe = (uid: string): RoamBasicNode[] => + sortBasicNodesSafe( + ( + window.roamAlphaAPI.data.fast.q( + `[:find (pull ?c [:block/string :block/uid :block/order {:block/children ...}]) :where [?b :block/uid "${uid}"] [?b :block/children ?c]]`, + ) as [PullBlock | null][] + ).map((a) => a[0]), + ); + const getPagesStartingWithPrefix = (prefix: string) => ( window.roamAlphaAPI.data.fast.q( @@ -22,7 +55,7 @@ const refreshConfigTree = (snapshot?: SettingsSnapshot) => { getDiscourseRelationLabels(undefined, snapshot).forEach((key) => unregisterDatalogTranslator({ key }), ); - discourseConfigRef.tree = getBasicTreeByParentUid( + discourseConfigRef.tree = getBasicTreeByParentUidSafe( getPageUidByPageTitle(DISCOURSE_CONFIG_PAGE_TITLE), ); const pages = getPagesStartingWithPrefix("discourse-graph/nodes"); @@ -32,7 +65,7 @@ const refreshConfigTree = (snapshot?: SettingsSnapshot) => { uid, { text: title.substring("discourse-graph/nodes/".length), - children: getBasicTreeByParentUid(uid), + children: getBasicTreeByParentUidSafe(uid), }, ]; }), diff --git a/apps/roam/src/utils/registerSlashCommands.ts b/apps/roam/src/utils/registerSlashCommands.ts index c47549818..77908a22b 100644 --- a/apps/roam/src/utils/registerSlashCommands.ts +++ b/apps/roam/src/utils/registerSlashCommands.ts @@ -1,5 +1,7 @@ import { updateBlock } from "roamjs-components/writes"; import { renderCanvasEmbedDialog } from "~/components/canvas/CanvasEmbedDialog"; +import { renderCanvasFrameEmbedDialog } from "~/components/canvas/CanvasFrameEmbedDialog"; +import { serializeDgFrameEmbed } from "~/utils/dgFrameEmbed"; type SlashCommandContext = { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -37,6 +39,21 @@ const SLASH_COMMANDS: { }); }, }, + { + label: "DG: Embed canvas frame", + callback: (context) => { + const uid = context["block-uid"]; + if (!uid) return; + renderCanvasFrameEmbedDialog({ + onSelect: ({ title, frameName, frameShapeId }) => { + void updateBlock({ + uid, + text: serializeDgFrameEmbed({ title, frameName, frameShapeId }), + }).then(() => document.body.click()); + }, + }); + }, + }, ]; export const registerSlashCommands = (): (() => void) => { From 146e6bc0da64719ca83e5d42a133058e8635fbbb Mon Sep 17 00:00:00 2001 From: mattakamatsu Date: Sat, 11 Jul 2026 20:52:31 -0700 Subject: [PATCH 2/9] Merge frame embed into {{dg-canvas}}; drop the separate {{dg-frame}} attribute Per product decision: one block syntax, {{dg-canvas: [[Canvas]]}}, drives both behaviors. A plain reference renders the original whole-canvas embed unchanged (the classic code path, .dg-canvas-embed wrapper). An optional frame argument - "Frame Name" and/or shape:ID - routes to the frame-anchored embed instead, but only when it maps to a real frame on the canvas. Degradation contract (a broken frame reference must never cost the user their canvas): the parser ignores any tail that is not a clean "name"/shape:id argument, and the renderer resolves the frame against the persisted store before mounting - an unmatched name or id silently falls back to the whole-canvas embed rather than showing a not-found notice. - dgFrameEmbed.ts -> dgCanvasEmbed.ts: parse/serialize {{dg-canvas}} with an optional frame argument; a malformed tail parses as whole-canvas. - CanvasFrameEmbed.tsx exports findCanvasFrameRef (editor-less frame existence check via getRoamCanvasSnapshot) and the CanvasFrameEmbed component; renderCanvasEmbed routes plain vs frame and picks the wrapper class. - Removed the dg-frame button observer and renderCanvasFrameEmbed; the frame picker slash command now writes {{dg-canvas: ... "Frame" shape:ID}}. Tests rewritten for the merged syntax including the degradation cases; verified live in plugin-testing-akamatsulab2 that plain / valid-frame / unknown-frame / malformed-tail each route to the expected embed with no errors. Co-Authored-By: Claude Fable 5 --- .../src/components/canvas/CanvasEmbed.tsx | 46 +++-- .../components/canvas/CanvasFrameEmbed.tsx | 180 ++++++++---------- .../canvas/CanvasFrameEmbedDialog.tsx | 4 +- .../src/utils/__tests__/dgCanvasEmbed.test.ts | 109 +++++++++++ .../src/utils/__tests__/dgFrameEmbed.test.ts | 81 -------- apps/roam/src/utils/dgCanvasEmbed.ts | 68 +++++++ apps/roam/src/utils/dgFrameEmbed.ts | 56 ------ .../utils/initializeObserversAndListeners.ts | 7 - apps/roam/src/utils/registerSlashCommands.ts | 4 +- 9 files changed, 288 insertions(+), 267 deletions(-) create mode 100644 apps/roam/src/utils/__tests__/dgCanvasEmbed.test.ts delete mode 100644 apps/roam/src/utils/__tests__/dgFrameEmbed.test.ts create mode 100644 apps/roam/src/utils/dgCanvasEmbed.ts delete mode 100644 apps/roam/src/utils/dgFrameEmbed.ts diff --git a/apps/roam/src/components/canvas/CanvasEmbed.tsx b/apps/roam/src/components/canvas/CanvasEmbed.tsx index f5b08f02b..e03780cdb 100644 --- a/apps/roam/src/components/canvas/CanvasEmbed.tsx +++ b/apps/roam/src/components/canvas/CanvasEmbed.tsx @@ -6,18 +6,8 @@ import getBlockUidFromTarget from "roamjs-components/dom/getBlockUidFromTarget"; import getTextByBlockUid from "roamjs-components/queries/getTextByBlockUid"; import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import { TldrawCanvas } from "./Tldraw"; - -const BLOCK_TEXT_REGEX = /\{\{dg-canvas:\s*\[\[(.+?)\]\]\s*\}\}/i; - -const extractCanvasTitle = (button: HTMLElement): string | null => { - const blockUid = getBlockUidFromTarget(button); - if (!blockUid) return null; - const blockText = getTextByBlockUid(blockUid); - if (!blockText) return null; - const match = blockText.match(BLOCK_TEXT_REGEX); - if (!match) return null; - return match[1].trim(); -}; +import { CanvasFrameEmbed, findCanvasFrameRef } from "./CanvasFrameEmbed"; +import { parseDgCanvasEmbed } from "~/utils/dgCanvasEmbed"; const CanvasEmbedPlaceholder = ({ message }: { message: string }) => (
(
); +// `{{dg-canvas: [[Title]]}}` renders the classic whole-canvas embed. An +// optional frame argument (`"Frame Name"` and/or `shape:ID`) routes to the +// frame-anchored embed instead — but only when it maps to a real frame on the +// canvas; malformed or unmatched frame arguments are ignored so the embed +// always degrades to just showing the canvas. export const renderCanvasEmbed = ( button: HTMLElement, onloadArgs: OnloadArgs, @@ -36,29 +31,44 @@ export const renderCanvasEmbed = ( if (!button.parentElement) return; - const title = extractCanvasTitle(button); - if (!title) return; + const blockUid = getBlockUidFromTarget(button); + if (!blockUid) return; + const blockText = getTextByBlockUid(blockUid); + const parsed = blockText ? parseDgCanvasEmbed(blockText) : null; + if (!parsed) return; - const pageUid = getPageUidByPageTitle(title); + const pageUid = getPageUidByPageTitle(parsed.title); if (!pageUid) { const wrapper = document.createElement("div"); button.parentElement.appendChild(wrapper); renderWithUnmount( - , + , wrapper, ); return; } + const frame = findCanvasFrameRef({ + pageUid, + frameName: parsed.frameName, + frameShapeId: parsed.frameShapeId, + }); + const wrapper = document.createElement("div"); - wrapper.className = "dg-canvas-embed my-2 w-full overflow-hidden rounded-md"; + wrapper.className = frame + ? "dg-frame-embed my-2 w-full overflow-hidden rounded-md" + : "dg-canvas-embed my-2 w-full overflow-hidden rounded-md"; wrapper.style.height = "400px"; wrapper.onmousedown = (e: MouseEvent) => e.stopPropagation(); button.parentElement.appendChild(wrapper); renderWithUnmount( - + {frame ? ( + + ) : ( + + )} , wrapper, ); diff --git a/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx index 305e316db..f8fde6349 100644 --- a/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx +++ b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx @@ -1,17 +1,57 @@ import React, { useCallback, useRef, useState } from "react"; -import ExtensionApiContextProvider from "roamjs-components/components/ExtensionApiContext"; -import { OnloadArgs } from "roamjs-components/types"; -import renderWithUnmount from "roamjs-components/util/renderWithUnmount"; -import getBlockUidFromTarget from "roamjs-components/dom/getBlockUidFromTarget"; -import getTextByBlockUid from "roamjs-components/queries/getTextByBlockUid"; -import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; -import type { Editor, TLFrameShape, TLShapeId } from "tldraw"; +import type { Editor, TLFrameShape, TLShape, TLShapeId } from "tldraw"; import { TldrawCanvas, type CanvasEmbedOptions } from "./Tldraw"; -import { parseDgFrameEmbed } from "~/utils/dgFrameEmbed"; +import { getRoamCanvasSnapshot } from "./useRoamStore"; const FRAME_ZOOM_INSET = 16; -type FrameRef = { name?: string; shapeId?: string }; +export type FrameRef = { name?: string; shapeId?: string }; + +// Decide whether a parsed frame argument actually maps to a frame on the +// canvas, by reading the persisted store snapshot without mounting an editor +// (frames are default tldraw shapes, so no custom utils are needed). The +// classic embed renderer uses this to route: no match -> the frame argument is +// ignored and the whole canvas renders. Sync-mode canvases may have a slightly +// stale snapshot; the worst case is falling back to the whole-canvas embed. +export const findCanvasFrameRef = ({ + pageUid, + frameName, + frameShapeId, +}: { + pageUid: string; + frameName?: string; + frameShapeId?: string; +}): FrameRef | null => { + if (!frameName && !frameShapeId) return null; + try { + const snapshot = getRoamCanvasSnapshot({ + pageUid, + migrations: [], + customShapeUtils: [], + customBindingUtils: [], + }); + if (!snapshot) return null; + + const frames = Object.values(snapshot.store).filter( + (record): record is TLFrameShape => + record.typeName === "shape" && (record as TLShape).type === "frame", + ); + const matchesId = + !!frameShapeId && frames.some((frame) => frame.id === frameShapeId); + const target = frameName?.trim().toLowerCase(); + const matchesName = + !!target && + frames.some( + (frame) => (frame.props.name ?? "").trim().toLowerCase() === target, + ); + + return matchesId || matchesName + ? { name: frameName, shapeId: frameShapeId } + : null; + } catch { + return null; + } +}; // Resolve a frame by shape id first (stable across renames and moves), then fall // back to a case-insensitive, trimmed name match. Duplicate names resolve to the @@ -38,7 +78,7 @@ const resolveFrameShape = ( if (matches.length > 1) { // eslint-disable-next-line no-console console.warn( - `dg-frame: multiple frames named "${frame.name}"; using the first match.`, + `dg-canvas: multiple frames named "${frame.name}"; using the first match.`, ); } if (matches[0]) return matches[0]; @@ -58,23 +98,14 @@ const zoomToFrame = (editor: Editor, frame: FrameRef): boolean => { return true; }; -const CanvasEmbedPlaceholder = ({ message }: { message: string }) => ( -
- {message} -
-); - -const CanvasFrameEmbed = ({ +// Frame-anchored variant of the canvas embed: mounted by renderCanvasEmbed only +// when the block's frame argument maps to a real frame on the canvas. +export const CanvasFrameEmbed = ({ title, - blockUid, frame, }: { title: string; - blockUid: string; - frame: FrameRef | null; + frame: FrameRef; }) => { const editorRef = useRef(null); const [frameMissing, setFrameMissing] = useState(false); @@ -82,7 +113,6 @@ const CanvasFrameEmbed = ({ const handleEditorMount = useCallback( (editor: Editor) => { editorRef.current = editor; - if (!frame) return; // Defer one frame so tldraw has measured the embed's viewport before we // compute a viewport-relative zoom on first mount. requestAnimationFrame(() => { @@ -93,42 +123,37 @@ const CanvasFrameEmbed = ({ ); const handleRecenter = useCallback(() => { - if (editorRef.current && frame) zoomToFrame(editorRef.current, frame); + if (editorRef.current) zoomToFrame(editorRef.current, frame); }, [frame]); - // Frame-anchored: manage the camera ourselves (no session persistence). - // Frameless: remember this embed's viewport independently, keyed by block uid. - // Both start in focus mode (chrome hidden; cmd+. brings the controls back). - const embedOptions: CanvasEmbedOptions = frame - ? { - disableSessionPersistence: true, - onEditorMount: handleEditorMount, - defaultFocusMode: true, - } - : { instanceKey: blockUid, defaultFocusMode: true }; + // Manage the camera ourselves (zoom-to-frame on every mount, no session + // persistence) and start with the tldraw chrome hidden (cmd+. restores it). + const embedOptions: CanvasEmbedOptions = { + disableSessionPersistence: true, + onEditorMount: handleEditorMount, + defaultFocusMode: true, + }; return (
- {frame && ( - - )} - {frame && frameMissing && ( + + {frameMissing && (
); }; - -export const renderCanvasFrameEmbed = ( - button: HTMLElement, - onloadArgs: OnloadArgs, -) => { - button.hidden = true; - - if (!button.parentElement) return; - - const blockUid = getBlockUidFromTarget(button); - if (!blockUid) return; - - const blockText = getTextByBlockUid(blockUid); - const parsed = blockText ? parseDgFrameEmbed(blockText) : null; - if (!parsed) return; - - const { title, frameName, frameShapeId } = parsed; - - const pageUid = getPageUidByPageTitle(title); - if (!pageUid) { - const wrapper = document.createElement("div"); - button.parentElement.appendChild(wrapper); - renderWithUnmount( - , - wrapper, - ); - return; - } - - const frame: FrameRef | null = - frameName || frameShapeId - ? { name: frameName, shapeId: frameShapeId } - : null; - - const wrapper = document.createElement("div"); - wrapper.className = "dg-frame-embed my-2 w-full overflow-hidden rounded-md"; - wrapper.style.height = "400px"; - wrapper.onmousedown = (e: MouseEvent) => e.stopPropagation(); - button.parentElement.appendChild(wrapper); - - renderWithUnmount( - - - , - wrapper, - ); -}; diff --git a/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx b/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx index 72f3abcc0..44ba4242a 100644 --- a/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx +++ b/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx @@ -9,14 +9,14 @@ import type { TLFrameShape, TLShape } from "tldraw"; import { getCanvasPageTitles } from "~/utils/isCanvasPage"; import { getRoamCanvasSnapshot } from "./useRoamStore"; -export type DgFrameEmbedSelection = { +export type DgCanvasFrameSelection = { title: string; frameName?: string; frameShapeId?: string; }; type CanvasFrameEmbedDialogProps = { - onSelect: (selection: DgFrameEmbedSelection) => void; + onSelect: (selection: DgCanvasFrameSelection) => void; }; type FrameOption = { id: string; name: string; childCount: number }; diff --git a/apps/roam/src/utils/__tests__/dgCanvasEmbed.test.ts b/apps/roam/src/utils/__tests__/dgCanvasEmbed.test.ts new file mode 100644 index 000000000..c545660ac --- /dev/null +++ b/apps/roam/src/utils/__tests__/dgCanvasEmbed.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from "vitest"; +import { + parseDgCanvasEmbed, + serializeDgCanvasEmbed, +} from "~/utils/dgCanvasEmbed"; + +describe("parseDgCanvasEmbed", () => { + it("parses a whole-canvas embed (no frame argument)", () => { + expect(parseDgCanvasEmbed("{{dg-canvas: [[My Canvas]]}}")).toEqual({ + title: "My Canvas", + }); + }); + + it("parses a name-only frame argument", () => { + expect( + parseDgCanvasEmbed('{{dg-canvas: [[My Canvas]] "Frame A"}}'), + ).toEqual({ + title: "My Canvas", + frameName: "Frame A", + }); + }); + + it("parses a name + shape-id frame argument", () => { + expect( + parseDgCanvasEmbed( + '{{dg-canvas: [[My Canvas]] "Frame A" shape:aB1_c-2}}', + ), + ).toEqual({ + title: "My Canvas", + frameName: "Frame A", + frameShapeId: "shape:aB1_c-2", + }); + }); + + it("parses an id-only frame argument (name omitted)", () => { + expect( + parseDgCanvasEmbed("{{dg-canvas: [[My Canvas]] shape:aB1_c-2}}"), + ).toEqual({ + title: "My Canvas", + frameShapeId: "shape:aB1_c-2", + }); + }); + + it("is case-insensitive on the keyword and tolerates surrounding text", () => { + expect( + parseDgCanvasEmbed('prefix {{DG-Canvas: [[My Canvas]] "F"}} suffix'), + ).toMatchObject({ title: "My Canvas", frameName: "F" }); + }); + + // Degradation contract: a tail that is not a clean `"name"`/`shape:id` + // argument is ignored, so the block still resolves to a whole-canvas embed + // instead of failing to render. + it("ignores a malformed frame argument and degrades to whole canvas", () => { + expect( + parseDgCanvasEmbed("{{dg-canvas: [[My Canvas]] not a frame arg}}"), + ).toEqual({ title: "My Canvas" }); + expect( + parseDgCanvasEmbed('{{dg-canvas: [[My Canvas]] "unclosed}}'), + ).toEqual({ title: "My Canvas" }); + expect( + parseDgCanvasEmbed('{{dg-canvas: [[My Canvas]] "Frame A" extra junk}}'), + ).toEqual({ title: "My Canvas" }); + }); + + it("tolerates extra whitespace around a valid frame argument", () => { + expect( + parseDgCanvasEmbed('{{dg-canvas: [[My Canvas]] "Frame A" }}'), + ).toEqual({ title: "My Canvas", frameName: "Frame A" }); + }); + + it("returns null for a non-dg-canvas embed or unrelated text", () => { + expect(parseDgCanvasEmbed("{{dg-query: [[My Canvas]]}}")).toBeNull(); + expect(parseDgCanvasEmbed("just some text")).toBeNull(); + }); +}); + +describe("serializeDgCanvasEmbed", () => { + it("round-trips a whole-canvas embed", () => { + const text = serializeDgCanvasEmbed({ title: "My Canvas" }); + expect(text).toBe("{{dg-canvas: [[My Canvas]]}}"); + expect(parseDgCanvasEmbed(text)).toEqual({ title: "My Canvas" }); + }); + + it("round-trips a fully-specified frame embed", () => { + const embed = { + title: "My Canvas", + frameName: "Frame A", + frameShapeId: "shape:aB1_c-2", + }; + const text = serializeDgCanvasEmbed(embed); + expect(text).toBe('{{dg-canvas: [[My Canvas]] "Frame A" shape:aB1_c-2}}'); + expect(parseDgCanvasEmbed(text)).toEqual(embed); + }); + + it("collapses embedded double-quotes in the frame name so the token stays parseable", () => { + const text = serializeDgCanvasEmbed({ + title: "My Canvas", + frameName: 'A "quoted" name', + frameShapeId: "shape:x", + }); + expect(text).toBe( + "{{dg-canvas: [[My Canvas]] \"A 'quoted' name\" shape:x}}", + ); + expect(parseDgCanvasEmbed(text)).toMatchObject({ + title: "My Canvas", + frameShapeId: "shape:x", + }); + }); +}); diff --git a/apps/roam/src/utils/__tests__/dgFrameEmbed.test.ts b/apps/roam/src/utils/__tests__/dgFrameEmbed.test.ts deleted file mode 100644 index 8a995af13..000000000 --- a/apps/roam/src/utils/__tests__/dgFrameEmbed.test.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { parseDgFrameEmbed, serializeDgFrameEmbed } from "~/utils/dgFrameEmbed"; - -describe("parseDgFrameEmbed", () => { - it("parses a frameless embed", () => { - expect(parseDgFrameEmbed("{{dg-frame: [[My Canvas]]}}")).toEqual({ - title: "My Canvas", - }); - }); - - it("parses a name-only embed", () => { - expect(parseDgFrameEmbed('{{dg-frame: [[My Canvas]] "Frame A"}}')).toEqual({ - title: "My Canvas", - frameName: "Frame A", - }); - }); - - it("parses a name + shape-id embed", () => { - expect( - parseDgFrameEmbed('{{dg-frame: [[My Canvas]] "Frame A" shape:aB1_c-2}}'), - ).toEqual({ - title: "My Canvas", - frameName: "Frame A", - frameShapeId: "shape:aB1_c-2", - }); - }); - - it("parses an id-only embed (name omitted)", () => { - expect( - parseDgFrameEmbed("{{dg-frame: [[My Canvas]] shape:aB1_c-2}}"), - ).toEqual({ - title: "My Canvas", - frameShapeId: "shape:aB1_c-2", - }); - }); - - it("is case-insensitive on the keyword and tolerates surrounding text", () => { - expect( - parseDgFrameEmbed('prefix {{DG-Frame: [[My Canvas]] "F"}} suffix'), - ).toMatchObject({ title: "My Canvas", frameName: "F" }); - }); - - it("returns null for a dg-canvas embed or unrelated text", () => { - expect(parseDgFrameEmbed("{{dg-canvas: [[My Canvas]]}}")).toBeNull(); - expect(parseDgFrameEmbed("just some text")).toBeNull(); - }); -}); - -describe("serializeDgFrameEmbed", () => { - it("round-trips a frameless embed", () => { - const text = serializeDgFrameEmbed({ title: "My Canvas" }); - expect(text).toBe("{{dg-frame: [[My Canvas]]}}"); - expect(parseDgFrameEmbed(text)).toEqual({ title: "My Canvas" }); - }); - - it("round-trips a fully-specified embed", () => { - const embed = { - title: "My Canvas", - frameName: "Frame A", - frameShapeId: "shape:aB1_c-2", - }; - const text = serializeDgFrameEmbed(embed); - expect(text).toBe('{{dg-frame: [[My Canvas]] "Frame A" shape:aB1_c-2}}'); - expect(parseDgFrameEmbed(text)).toEqual(embed); - }); - - it("collapses embedded double-quotes in the frame name so the token stays parseable", () => { - const text = serializeDgFrameEmbed({ - title: "My Canvas", - frameName: 'A "quoted" name', - frameShapeId: "shape:x", - }); - expect(text).toBe( - "{{dg-frame: [[My Canvas]] \"A 'quoted' name\" shape:x}}", - ); - expect(parseDgFrameEmbed(text)).toMatchObject({ - title: "My Canvas", - frameShapeId: "shape:x", - }); - }); -}); diff --git a/apps/roam/src/utils/dgCanvasEmbed.ts b/apps/roam/src/utils/dgCanvasEmbed.ts new file mode 100644 index 000000000..49a4a5900 --- /dev/null +++ b/apps/roam/src/utils/dgCanvasEmbed.ts @@ -0,0 +1,68 @@ +// Parsing/serialization for the `{{dg-canvas: ...}}` block embed. +// +// One syntax covers both the classic whole-canvas embed and the frame-anchored +// embed (the frame argument is optional): +// {{dg-canvas: [[Canvas Page Title]]}} whole canvas (classic embed) +// {{dg-canvas: [[Canvas Page Title]] "Frame Name"}} frame by name (hand-written) +// {{dg-canvas: [[Canvas Page Title]] "Frame Name" shape:abc}} frame by id (what the picker writes) +// {{dg-canvas: [[Canvas Page Title]] shape:abc}} frame by id (name omitted) +// +// Frames are referenced by tldraw shape id (stable across renames and moves); +// the display name is carried alongside for human readability and as a +// resolution fallback. `shape:` is a plain token — we deliberately avoid Roam's +// `((...))` block-ref syntax so the embed round-trips through block text +// unambiguously. +// +// Degradation contract: a tail that does not parse as `"name"`/`shape:id` is +// ignored (whole-canvas embed), and a frame argument that matches no frame on +// the canvas is likewise ignored by the renderer — a broken frame reference +// must never cost the user their canvas. + +export type DgCanvasEmbed = { + title: string; + frameName?: string; + // Full tldraw shape id, e.g. "shape:abc123" (tldraw ids use the nanoid + // alphabet: A-Z a-z 0-9 _ -). + frameShapeId?: string; +}; + +// Group 1: page title. Group 2: everything between the closing `]]` and `}}`, +// parsed separately so a malformed frame argument degrades to a whole-canvas +// embed instead of a dead button. +export const DG_CANVAS_EMBED_REGEX = + /\{\{dg-canvas:\s*\[\[(.+?)\]\]([^}]*)\}\}/i; + +// The tail only counts as a frame reference if it is exactly an optional +// quoted frame name followed by an optional shape-id token. +const FRAME_ARGS_REGEX = /^\s*(?:"([^"]*)"\s*)?(shape:[A-Za-z0-9_-]+)?\s*$/; + +export const parseDgCanvasEmbed = (blockText: string): DgCanvasEmbed | null => { + const match = blockText.match(DG_CANVAS_EMBED_REGEX); + if (!match) return null; + + const title = match[1].trim(); + if (!title) return null; + + const frameArgs = match[2].match(FRAME_ARGS_REGEX); + const frameName = frameArgs?.[1]?.trim() || undefined; + const frameShapeId = frameArgs?.[2] || undefined; + + return { + title, + ...(frameName ? { frameName } : {}), + ...(frameShapeId ? { frameShapeId } : {}), + }; +}; + +export const serializeDgCanvasEmbed = ({ + title, + frameName, + frameShapeId, +}: DgCanvasEmbed): string => { + let inner = `dg-canvas: [[${title}]]`; + // The name is only ever a readability/fallback hint (the id is authoritative), + // so collapse any embedded double-quote to keep the token unambiguous. + if (frameName) inner += ` "${frameName.replace(/"/g, "'")}"`; + if (frameShapeId) inner += ` ${frameShapeId}`; + return `{{${inner}}}`; +}; diff --git a/apps/roam/src/utils/dgFrameEmbed.ts b/apps/roam/src/utils/dgFrameEmbed.ts deleted file mode 100644 index 1fd69ed5c..000000000 --- a/apps/roam/src/utils/dgFrameEmbed.ts +++ /dev/null @@ -1,56 +0,0 @@ -// Parsing/serialization for the `{{dg-frame: ...}}` block embed. -// -// Serialized forms (the frame argument is optional): -// {{dg-frame: [[Canvas Page Title]]}} frameless embed -// {{dg-frame: [[Canvas Page Title]] "Frame Name"}} name only (hand-written / self-healed) -// {{dg-frame: [[Canvas Page Title]] "Frame Name" shape:abc}} id-anchored (what the picker writes) -// {{dg-frame: [[Canvas Page Title]] shape:abc}} id only (name omitted) -// -// Frames are referenced by tldraw shape id (stable across renames and moves); -// the display name is carried alongside for human readability and as a -// resolution fallback. `shape:` is a plain token — we deliberately avoid Roam's -// `((...))` block-ref syntax so the embed round-trips through block text -// unambiguously. - -export type DgFrameEmbed = { - title: string; - frameName?: string; - // Full tldraw shape id, e.g. "shape:abc123" (tldraw ids use the nanoid - // alphabet: A-Z a-z 0-9 _ -). - frameShapeId?: string; -}; - -// Group 1: page title. Group 2: optional frame name. Group 3: optional shape id -// (captured with its `shape:` prefix, so it is the full tldraw id). -export const DG_FRAME_EMBED_REGEX = - /\{\{dg-frame:\s*\[\[(.+?)\]\](?:\s+"([^"]*)")?(?:\s+(shape:[A-Za-z0-9_-]+))?\s*\}\}/i; - -export const parseDgFrameEmbed = (blockText: string): DgFrameEmbed | null => { - const match = blockText.match(DG_FRAME_EMBED_REGEX); - if (!match) return null; - - const title = match[1].trim(); - if (!title) return null; - - const frameName = match[2]?.trim() || undefined; - const frameShapeId = match[3] || undefined; - - return { - title, - ...(frameName ? { frameName } : {}), - ...(frameShapeId ? { frameShapeId } : {}), - }; -}; - -export const serializeDgFrameEmbed = ({ - title, - frameName, - frameShapeId, -}: DgFrameEmbed): string => { - let inner = `dg-frame: [[${title}]]`; - // The name is only ever a readability/fallback hint (the id is authoritative), - // so collapse any embedded double-quote to keep the token unambiguous. - if (frameName) inner += ` "${frameName.replace(/"/g, "'")}"`; - if (frameShapeId) inner += ` ${frameShapeId}`; - return `{{${inner}}}`; -}; diff --git a/apps/roam/src/utils/initializeObserversAndListeners.ts b/apps/roam/src/utils/initializeObserversAndListeners.ts index f4e9e3249..57804da57 100644 --- a/apps/roam/src/utils/initializeObserversAndListeners.ts +++ b/apps/roam/src/utils/initializeObserversAndListeners.ts @@ -50,7 +50,6 @@ import { getNodeTagStyles } from "~/utils/getDiscourseNodeColors"; import { renderPossibleDuplicates } from "~/components/VectorDuplicateMatches"; import { renderPublishNodeTitleButton } from "~/components/PublishNodeTitleButton"; import { renderCanvasEmbed } from "~/components/canvas/CanvasEmbed"; -import { renderCanvasFrameEmbed } from "~/components/canvas/CanvasFrameEmbed"; import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import getPageTitleByPageUid from "roamjs-components/queries/getPageTitleByPageUid"; import findDiscourseNode from "./findDiscourseNode"; @@ -171,11 +170,6 @@ export const initObservers = ({ render: (b) => renderCanvasEmbed(b, onloadArgs), }); - const canvasFrameEmbedObserver = createButtonObserver({ - attribute: "dg-frame", - render: (b) => renderCanvasFrameEmbed(b, onloadArgs), - }); - let batchedTagNodes: DiscourseNode[] | null = null; const getNodesForTagBatch = (): DiscourseNode[] => { if (batchedTagNodes === null) { @@ -475,7 +469,6 @@ export const initObservers = ({ pageTitleObserver, queryBlockObserver, canvasEmbedObserver, - canvasFrameEmbedObserver, graphOverviewExportObserver, nodeTagPopupButtonObserver, leftSidebarObserver, diff --git a/apps/roam/src/utils/registerSlashCommands.ts b/apps/roam/src/utils/registerSlashCommands.ts index 77908a22b..5b4f874bd 100644 --- a/apps/roam/src/utils/registerSlashCommands.ts +++ b/apps/roam/src/utils/registerSlashCommands.ts @@ -1,7 +1,7 @@ import { updateBlock } from "roamjs-components/writes"; import { renderCanvasEmbedDialog } from "~/components/canvas/CanvasEmbedDialog"; import { renderCanvasFrameEmbedDialog } from "~/components/canvas/CanvasFrameEmbedDialog"; -import { serializeDgFrameEmbed } from "~/utils/dgFrameEmbed"; +import { serializeDgCanvasEmbed } from "~/utils/dgCanvasEmbed"; type SlashCommandContext = { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -48,7 +48,7 @@ const SLASH_COMMANDS: { onSelect: ({ title, frameName, frameShapeId }) => { void updateBlock({ uid, - text: serializeDgFrameEmbed({ title, frameName, frameShapeId }), + text: serializeDgCanvasEmbed({ title, frameName, frameShapeId }), }).then(() => document.body.click()); }, }); From 257544c697bccb43df8c6b2152cfdd8be0666296 Mon Sep 17 00:00:00 2001 From: mattakamatsu Date: Sat, 11 Jul 2026 21:23:12 -0700 Subject: [PATCH 3/9] fix(canvas): keep a } in a frame name from blanking the embed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `}` inside a serialized frame name aborted the whole-embed regex match, so parseDgCanvasEmbed returned null and renderCanvasEmbed rendered nothing — violating the contract that a bad frame reference always degrades to the whole canvas. Make the tail lazy (`[\s\S]*?` to the `}}` terminator) so a stray `}` is handled by FRAME_ARGS_REGEX instead, and strip curly braces in the serializer as defense-in-depth for picker-written blocks. Co-Authored-By: Claude Fable 5 --- .../src/utils/__tests__/dgCanvasEmbed.test.ts | 28 +++++++++++++++++++ apps/roam/src/utils/dgCanvasEmbed.ts | 19 +++++++++---- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/apps/roam/src/utils/__tests__/dgCanvasEmbed.test.ts b/apps/roam/src/utils/__tests__/dgCanvasEmbed.test.ts index c545660ac..7172a9c2c 100644 --- a/apps/roam/src/utils/__tests__/dgCanvasEmbed.test.ts +++ b/apps/roam/src/utils/__tests__/dgCanvasEmbed.test.ts @@ -68,6 +68,19 @@ describe("parseDgCanvasEmbed", () => { ).toEqual({ title: "My Canvas", frameName: "Frame A" }); }); + // A lone `}` inside a frame name must not stop the match at the wrong place and + // blank the whole block — the embed still parses (title + id survive) rather + // than returning null. + it("keeps a `}` inside a frame name from blanking the embed", () => { + expect( + parseDgCanvasEmbed('{{dg-canvas: [[My Canvas]] "Fig }2" shape:aB1_c-2}}'), + ).toEqual({ + title: "My Canvas", + frameName: "Fig }2", + frameShapeId: "shape:aB1_c-2", + }); + }); + it("returns null for a non-dg-canvas embed or unrelated text", () => { expect(parseDgCanvasEmbed("{{dg-query: [[My Canvas]]}}")).toBeNull(); expect(parseDgCanvasEmbed("just some text")).toBeNull(); @@ -92,6 +105,21 @@ describe("serializeDgCanvasEmbed", () => { expect(parseDgCanvasEmbed(text)).toEqual(embed); }); + it("strips curly braces from the frame name so the serialized token round-trips", () => { + const text = serializeDgCanvasEmbed({ + title: "My Canvas", + frameName: "Draft }v2{", + frameShapeId: "shape:x", + }); + // Braces gone; the token parses back to a whole, non-null embed. + expect(text).toBe('{{dg-canvas: [[My Canvas]] "Draft v2" shape:x}}'); + expect(parseDgCanvasEmbed(text)).toEqual({ + title: "My Canvas", + frameName: "Draft v2", + frameShapeId: "shape:x", + }); + }); + it("collapses embedded double-quotes in the frame name so the token stays parseable", () => { const text = serializeDgCanvasEmbed({ title: "My Canvas", diff --git a/apps/roam/src/utils/dgCanvasEmbed.ts b/apps/roam/src/utils/dgCanvasEmbed.ts index 49a4a5900..4e5c63b5f 100644 --- a/apps/roam/src/utils/dgCanvasEmbed.ts +++ b/apps/roam/src/utils/dgCanvasEmbed.ts @@ -26,11 +26,15 @@ export type DgCanvasEmbed = { frameShapeId?: string; }; -// Group 1: page title. Group 2: everything between the closing `]]` and `}}`, -// parsed separately so a malformed frame argument degrades to a whole-canvas -// embed instead of a dead button. +// Group 1: page title. Group 2: everything between the closing `]]` and the +// `}}` terminator, parsed separately so a malformed frame argument degrades to a +// whole-canvas embed instead of a dead button. The tail is lazy (`[\s\S]*?`, not +// `[^}]*`) on purpose: a lone `}` inside a frame name must not stop the match at +// the wrong place and make the whole embed fail to parse (which would blank the +// block rather than degrade) — the tail runs to the first `}}` and a stray `}` +// in the name is then handled by FRAME_ARGS_REGEX. export const DG_CANVAS_EMBED_REGEX = - /\{\{dg-canvas:\s*\[\[(.+?)\]\]([^}]*)\}\}/i; + /\{\{dg-canvas:\s*\[\[(.+?)\]\]([\s\S]*?)\}\}/i; // The tail only counts as a frame reference if it is exactly an optional // quoted frame name followed by an optional shape-id token. @@ -61,8 +65,11 @@ export const serializeDgCanvasEmbed = ({ }: DgCanvasEmbed): string => { let inner = `dg-canvas: [[${title}]]`; // The name is only ever a readability/fallback hint (the id is authoritative), - // so collapse any embedded double-quote to keep the token unambiguous. - if (frameName) inner += ` "${frameName.replace(/"/g, "'")}"`; + // so collapse characters that would break the token: double-quotes (which + // delimit the name) and curly braces (which would collide with Roam's + // `{{ }}` component syntax and the parser's `}}` terminator). + const safeName = frameName?.replace(/"/g, "'").replace(/[{}]/g, ""); + if (safeName) inner += ` "${safeName}"`; if (frameShapeId) inner += ` ${frameShapeId}`; return `{{${inner}}}`; }; From 8c47d5bfb7bebc2a80d4587c3fa8c3f699588461 Mon Sep 17 00:00:00 2001 From: mattakamatsu Date: Sat, 11 Jul 2026 21:23:53 -0700 Subject: [PATCH 4/9] fix(canvas): guard the zoom-on-mount rAF and refresh re-center state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The requestAnimationFrame scheduled at editor mount was never cancelled, so if the embed unmounted within a frame the callback ran zoomToFrame against a disposed tldraw editor and set state on an unmounted component. Track the handle and cancel it in an unmount effect. Also have the re-center (⌖) handler re-resolve the frame so a delete/rename after mount clears or raises the "not found" notice instead of leaving it stale. Co-Authored-By: Claude Fable 5 --- .../components/canvas/CanvasFrameEmbed.tsx | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx index f8fde6349..313712fc2 100644 --- a/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx +++ b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useRef, useState } from "react"; +import React, { useCallback, useEffect, useRef, useState } from "react"; import type { Editor, TLFrameShape, TLShape, TLShapeId } from "tldraw"; import { TldrawCanvas, type CanvasEmbedOptions } from "./Tldraw"; import { getRoamCanvasSnapshot } from "./useRoamStore"; @@ -108,22 +108,36 @@ export const CanvasFrameEmbed = ({ frame: FrameRef; }) => { const editorRef = useRef(null); + const rafRef = useRef(null); const [frameMissing, setFrameMissing] = useState(false); const handleEditorMount = useCallback( (editor: Editor) => { editorRef.current = editor; // Defer one frame so tldraw has measured the embed's viewport before we - // compute a viewport-relative zoom on first mount. - requestAnimationFrame(() => { + // compute a viewport-relative zoom on first mount. Track the handle so the + // cleanup effect can cancel it if the embed unmounts within the frame — + // otherwise the callback runs against a disposed editor. + rafRef.current = requestAnimationFrame(() => { + rafRef.current = null; setFrameMissing(!zoomToFrame(editor, frame)); }); }, [frame], ); + useEffect( + () => () => { + if (rafRef.current !== null) cancelAnimationFrame(rafRef.current); + }, + [], + ); + const handleRecenter = useCallback(() => { - if (editorRef.current) zoomToFrame(editorRef.current, frame); + // Re-resolve on click so a frame deleted/renamed since mount clears (or + // raises) the not-found notice instead of leaving stale state. + if (editorRef.current) + setFrameMissing(!zoomToFrame(editorRef.current, frame)); }, [frame]); // Manage the camera ourselves (zoom-to-frame on every mount, no session From 81a80a103fd43e0e1918c9b538ebeaff929bdfc8 Mon Sep 17 00:00:00 2001 From: mattakamatsu Date: Sat, 11 Jul 2026 21:26:25 -0700 Subject: [PATCH 5/9] perf(canvas): resolve frames from the raw persisted store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findCanvasFrameRef and the frame picker both called getRoamCanvasSnapshot, which builds and migrates a full throwaway TLStore just to list frame shapes — duplicating the parse the editor mount does moments later, and throwing (then swallowing) on legacy canvases whose node shapes need the discourse migration. Add getPersistedCanvasStore / getCanvasFrameShapes that read frame records straight from block props (frames are default shapes, present in both formats and untouched by migration), and route both callers through them. One source of frame enumeration, no throwaway editor, and legacy canvases list frames. Co-Authored-By: Claude Fable 5 --- .../components/canvas/CanvasFrameEmbed.tsx | 28 ++++++------------- .../canvas/CanvasFrameEmbedDialog.tsx | 21 ++++---------- .../src/components/canvas/useRoamStore.ts | 23 +++++++++++++++ 3 files changed, 37 insertions(+), 35 deletions(-) diff --git a/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx index 313712fc2..453113883 100644 --- a/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx +++ b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx @@ -1,18 +1,17 @@ import React, { useCallback, useEffect, useRef, useState } from "react"; -import type { Editor, TLFrameShape, TLShape, TLShapeId } from "tldraw"; +import type { Editor, TLFrameShape, TLShapeId } from "tldraw"; import { TldrawCanvas, type CanvasEmbedOptions } from "./Tldraw"; -import { getRoamCanvasSnapshot } from "./useRoamStore"; +import { getCanvasFrameShapes } from "./useRoamStore"; const FRAME_ZOOM_INSET = 16; export type FrameRef = { name?: string; shapeId?: string }; -// Decide whether a parsed frame argument actually maps to a frame on the -// canvas, by reading the persisted store snapshot without mounting an editor -// (frames are default tldraw shapes, so no custom utils are needed). The -// classic embed renderer uses this to route: no match -> the frame argument is -// ignored and the whole canvas renders. Sync-mode canvases may have a slightly -// stale snapshot; the worst case is falling back to the whole-canvas embed. +// Decide whether a parsed frame argument actually maps to a frame on the canvas +// by scanning the persisted frame shapes (no editor mounted). The classic embed +// renderer uses this to route: no match -> the frame argument is ignored and the +// whole canvas renders. Sync-mode canvases may have a slightly stale snapshot; +// the worst case is falling back to the whole-canvas embed. export const findCanvasFrameRef = ({ pageUid, frameName, @@ -24,18 +23,7 @@ export const findCanvasFrameRef = ({ }): FrameRef | null => { if (!frameName && !frameShapeId) return null; try { - const snapshot = getRoamCanvasSnapshot({ - pageUid, - migrations: [], - customShapeUtils: [], - customBindingUtils: [], - }); - if (!snapshot) return null; - - const frames = Object.values(snapshot.store).filter( - (record): record is TLFrameShape => - record.typeName === "shape" && (record as TLShape).type === "frame", - ); + const frames = getCanvasFrameShapes(pageUid); const matchesId = !!frameShapeId && frames.some((frame) => frame.id === frameShapeId); const target = frameName?.trim().toLowerCase(); diff --git a/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx b/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx index 44ba4242a..1dab06342 100644 --- a/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx +++ b/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx @@ -7,7 +7,7 @@ import renderOverlay, { } from "roamjs-components/util/renderOverlay"; import type { TLFrameShape, TLShape } from "tldraw"; import { getCanvasPageTitles } from "~/utils/isCanvasPage"; -import { getRoamCanvasSnapshot } from "./useRoamStore"; +import { getPersistedCanvasStore } from "./useRoamStore"; export type DgCanvasFrameSelection = { title: string; @@ -21,22 +21,13 @@ type CanvasFrameEmbedDialogProps = { type FrameOption = { id: string; name: string; childCount: number }; -// Enumerate frames on a canvas without mounting an editor, by reading the -// persisted store snapshot from block props and filtering for frame shapes. -// Frames are default tldraw shapes, so they survive even with no custom shape -// utils passed. Sync-mode canvases may show a slightly stale list — acceptable -// for a picker. Any read/upgrade failure degrades to "whole canvas only". +// Enumerate frames (with a child-shape count as a disambiguator) from the raw +// persisted store — no editor mounted, no migration. Sync-mode canvases may +// show a slightly stale list, acceptable for a picker; any read failure +// degrades to "whole canvas only". const getCanvasFrames = (pageUid: string): FrameOption[] => { try { - const snapshot = getRoamCanvasSnapshot({ - pageUid, - migrations: [], - customShapeUtils: [], - customBindingUtils: [], - }); - if (!snapshot) return []; - - const records = Object.values(snapshot.store); + const records = Object.values(getPersistedCanvasStore(pageUid)); const childCounts = new Map(); for (const record of records) { diff --git a/apps/roam/src/components/canvas/useRoamStore.ts b/apps/roam/src/components/canvas/useRoamStore.ts index 6753794b2..91a26c7ae 100644 --- a/apps/roam/src/components/canvas/useRoamStore.ts +++ b/apps/roam/src/components/canvas/useRoamStore.ts @@ -17,6 +17,8 @@ import { defaultShapeUtils, loadSnapshot, MigrationSequence, + TLFrameShape, + TLShape, TLStoreSnapshot, TLStore, } from "tldraw"; @@ -136,6 +138,27 @@ export const getRoamCanvasSnapshot = ({ }; }; +// The raw persisted record map from block props, without constructing or +// migrating a TLStore (modern `{ store, schema }` store, or the legacy raw +// store, whichever is present). Cheap read for callers that only need to +// inspect records — no throwaway editor, and no ValidationError on an +// un-migrated legacy node shape the way loadSnapshot-without-utils would throw. +export const getPersistedCanvasStore = ( + pageUid: string, +): SerializedStore => { + const { initialSnapshot, oldData } = getPersistedRoamCanvasState({ pageUid }); + return initialSnapshot ? initialSnapshot.store : (oldData ?? {}); +}; + +// Frame shapes on a canvas. Frames are default tldraw shapes — present verbatim +// in both persisted formats and untouched by discourse-node migrations — so +// this raw scan needs no migration or shape utils. +export const getCanvasFrameShapes = (pageUid: string): TLFrameShape[] => + Object.values(getPersistedCanvasStore(pageUid)).filter( + (record): record is TLFrameShape => + record.typeName === "shape" && (record as TLShape).type === "frame", + ); + export const useRoamStore = ({ customShapeUtils, customBindingUtils, From fc90ca1ef71d93d4fd264d1b7e497001c5d5a3fe Mon Sep 17 00:00:00 2001 From: mattakamatsu Date: Sat, 11 Jul 2026 21:28:03 -0700 Subject: [PATCH 6/9] chore(canvas): drop unused instanceKey session-key plumbing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit instanceKey was threaded through CanvasEmbedOptions, the session storage key, and the persistence registration to give frameless embeds an independent viewport — but merging the frame embed into {{dg-canvas}} left the plain path on the original TldrawCanvas with no embedOptions, so no caller ever set it. Remove the dead parameter, its storage-key branch, and its tests, and correct the CanvasEmbedOptions doc comment that still referenced the removed {{dg-frame}} syntax. (Two same-canvas embeds share one viewport — the original {{dg-canvas}} behavior, intentionally unchanged.) Co-Authored-By: Claude Fable 5 --- apps/roam/src/components/canvas/Tldraw.tsx | 9 ++----- .../__tests__/canvasSessionState.test.ts | 26 ------------------- apps/roam/src/utils/canvasSessionState.ts | 15 ++--------- 3 files changed, 4 insertions(+), 46 deletions(-) diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index 420997dc9..2f22fcabd 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/apps/roam/src/components/canvas/Tldraw.tsx @@ -190,16 +190,12 @@ export const isPageUid = (uid: string) => ":node/title" ]; -// Options for mounting the canvas inside a block embed (`{{dg-frame: ...}}`). +// Options for mounting the canvas inside a block embed (`{{dg-canvas: ...}}`). // Left undefined for main-page and sidebar canvases, whose behavior is unchanged. export type CanvasEmbedOptions = { - // When set (the embed's block uid), viewport/session state is remembered per - // embed instance instead of per canvas page — so two embeds of the same canvas - // hold independent viewports. Used by frameless embeds. - instanceKey?: string; // Frame-anchored embeds manage their own camera (zoom-to-frame on every mount) // and intentionally do not persist panning across remounts, so they opt out of - // the per-page/per-instance session persistence entirely. + // the per-page session persistence entirely. disableSessionPersistence?: boolean; // Start the editor in focus mode (what cmd+. toggles), hiding the tldraw // menus/toolbar chrome — block embeds are for viewing first, so they default @@ -1087,7 +1083,6 @@ const TldrawCanvasShared = ({ graphName: window.roamAlphaAPI.graph.name, userUid: window.roamAlphaAPI.user.uid() || "", pageUid, - instanceKey: embedOptions?.instanceKey, }); if (process.env.NODE_ENV !== "production") { diff --git a/apps/roam/src/utils/__tests__/canvasSessionState.test.ts b/apps/roam/src/utils/__tests__/canvasSessionState.test.ts index 59034a0e2..a1c2223ef 100644 --- a/apps/roam/src/utils/__tests__/canvasSessionState.test.ts +++ b/apps/roam/src/utils/__tests__/canvasSessionState.test.ts @@ -78,32 +78,6 @@ describe("getCanvasSessionStorageKey", () => { }), ).toBe("dg:tldraw-session:v1:graph%2Fname:user%3A1:abc%20123"); }); - - it("appends an instance key so embeds of the same page stay independent", () => { - expect( - getCanvasSessionStorageKey({ - graphName: "graph/name", - userUid: "user:1", - pageUid: "abc 123", - instanceKey: "block-uid-1", - }), - ).toBe("dg:tldraw-session:v1:graph%2Fname:user%3A1:abc%20123:block-uid-1"); - }); - - it("is unchanged from the base key when no instance key is given", () => { - const base = getCanvasSessionStorageKey({ - graphName: "g", - userUid: "u", - pageUid: "p", - }); - const withUndefined = getCanvasSessionStorageKey({ - graphName: "g", - userUid: "u", - pageUid: "p", - instanceKey: undefined, - }); - expect(withUndefined).toBe(base); - }); }); describe("readCanvasSessionState", () => { diff --git a/apps/roam/src/utils/canvasSessionState.ts b/apps/roam/src/utils/canvasSessionState.ts index 2372e12e0..339c07eb8 100644 --- a/apps/roam/src/utils/canvasSessionState.ts +++ b/apps/roam/src/utils/canvasSessionState.ts @@ -99,26 +99,18 @@ export const getCanvasSessionStorageKey = ({ graphName, userUid, pageUid, - instanceKey, }: { graphName: string; userUid: string; pageUid: string; - // Optional per-embed discriminator (the embed's block uid). When present, the - // viewport is remembered per embed instance instead of per canvas page, so two - // embeds of the same canvas hold independent viewports. Omitted for main-page - // and sidebar canvases, keeping their storage key (and behavior) unchanged. - instanceKey?: string; }): string => { - const parts = [ + return [ CANVAS_SESSION_STORAGE_PREFIX, `v${CANVAS_SESSION_STORAGE_VERSION}`, encodeStoragePart(graphName), encodeStoragePart(userUid || "anonymous"), encodeStoragePart(pageUid), - ]; - if (instanceKey) parts.push(encodeStoragePart(instanceKey)); - return parts.join(":"); + ].join(":"); }; export const readCanvasSessionState = ({ @@ -250,7 +242,6 @@ export const registerCanvasSessionStatePersistence = ({ graphName, userUid, pageUid, - instanceKey, storage = getCanvasSessionStorage(), saveDelayMs = CANVAS_SESSION_SAVE_DELAY_MS, }: { @@ -258,7 +249,6 @@ export const registerCanvasSessionStatePersistence = ({ graphName: string; userUid: string; pageUid: string; - instanceKey?: string; storage?: CanvasSessionStorage | null; saveDelayMs?: number; }): (() => void) => { @@ -268,7 +258,6 @@ export const registerCanvasSessionStatePersistence = ({ graphName, userUid, pageUid, - instanceKey, }); let saveTimeout: ReturnType | null = null; From 093a27f0ed872a0b56d78a773262c71babe98c9f Mon Sep 17 00:00:00 2001 From: mattakamatsu Date: Sat, 11 Jul 2026 21:29:41 -0700 Subject: [PATCH 7/9] style(canvas): use Blueprint surfaces for embed chrome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ⌖ re-center button and the frame-not-found notice backed their readability with an inline rgba(255,255,255,0.9) — a new hardcoded shading color (AGENTS.md: don't introduce new shading colors). Use a default (non-minimal) Blueprint Button and a Blueprint Card, whose themed surfaces provide the background and elevation in both light and dark themes. Co-Authored-By: Claude Fable 5 --- .../components/canvas/CanvasFrameEmbed.tsx | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx index 453113883..994950d6a 100644 --- a/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx +++ b/apps/roam/src/components/canvas/CanvasFrameEmbed.tsx @@ -1,4 +1,5 @@ import React, { useCallback, useEffect, useRef, useState } from "react"; +import { Button, Card, Elevation } from "@blueprintjs/core"; import type { Editor, TLFrameShape, TLShapeId } from "tldraw"; import { TldrawCanvas, type CanvasEmbedOptions } from "./Tldraw"; import { getCanvasFrameShapes } from "./useRoamStore"; @@ -139,36 +140,30 @@ export const CanvasFrameEmbed = ({ return (
- + {frameMissing && ( -
Frame {frame.name ? `“${frame.name}” ` : ""}not found on [[ {title}]] -
+ )}
); From ce449eeda29b2bde50d4758499a39f016bfc0774 Mon Sep 17 00:00:00 2001 From: mattakamatsu Date: Sat, 11 Jul 2026 21:32:47 -0700 Subject: [PATCH 8/9] chore(canvas): add explicit return types on new canvas exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AGENTS.md asks for explicit return types. Annotate the new exports that were missing them: renderCanvasFrameEmbedDialog and filterUserRecords. (calculateDiff is relocated code with any-typed internals that resist a precise return type without a rewrite — left as-is.) Co-Authored-By: Claude Fable 5 --- apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx | 2 +- apps/roam/src/components/canvas/canvasRemoteMerge.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx b/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx index 1dab06342..9029459d3 100644 --- a/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx +++ b/apps/roam/src/components/canvas/CanvasFrameEmbedDialog.tsx @@ -190,7 +190,7 @@ const CanvasFrameEmbedDialog = ({ export const renderCanvasFrameEmbedDialog = ( props: CanvasFrameEmbedDialogProps, -) => +): (() => void) | undefined => renderOverlay({ // eslint-disable-next-line @typescript-eslint/naming-convention Overlay: CanvasFrameEmbedDialog, diff --git a/apps/roam/src/components/canvas/canvasRemoteMerge.ts b/apps/roam/src/components/canvas/canvasRemoteMerge.ts index 30f9406de..d3b5a85e7 100644 --- a/apps/roam/src/components/canvas/canvasRemoteMerge.ts +++ b/apps/roam/src/components/canvas/canvasRemoteMerge.ts @@ -60,7 +60,9 @@ export const fixShapeIndices = ( ); }; -export const filterUserRecords = (data: SerializedStore) => { +export const filterUserRecords = ( + data: SerializedStore, +): SerializedStore => { return Object.fromEntries( Object.entries(data).filter(([key]) => { return !/^(user_presence|camera|instance|instance_page_state|user|user_document):/.test( From c170ce3461bcf587092dd34669c9a6d292b7a65e Mon Sep 17 00:00:00 2001 From: mattakamatsu Date: Sat, 11 Jul 2026 22:38:19 -0700 Subject: [PATCH 9/9] chore(canvas): type the relocated diff helpers with unknown instead of any Clears the 12 lint warnings CI flagged on canvasRemoteMerge.ts (the file reads as newly added, so the diff-helper code relocated from useRoamStore.ts is re-linted). diffObjects/calculateDiff now use `unknown` with narrowed casts and typed filter predicates; pruneState drops the unused destructured key. Behavior unchanged. Co-Authored-By: Claude Fable 5 --- .../components/canvas/canvasRemoteMerge.ts | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/apps/roam/src/components/canvas/canvasRemoteMerge.ts b/apps/roam/src/components/canvas/canvasRemoteMerge.ts index d3b5a85e7..007959a3e 100644 --- a/apps/roam/src/components/canvas/canvasRemoteMerge.ts +++ b/apps/roam/src/components/canvas/canvasRemoteMerge.ts @@ -81,20 +81,20 @@ const personalRecordTypes = new Set([ const pruneState = (state: SerializedStore) => Object.fromEntries( Object.entries(state).filter( - ([_, record]) => !personalRecordTypes.has(record.typeName), + ([, record]) => !personalRecordTypes.has(record.typeName), ), ); const diffObjects = ( - oldRecord: Record, - newRecord: Record, -): Record => { + oldRecord: Record, + newRecord: Record, +): Record => { const allKeys = Array.from( new Set(Object.keys(oldRecord).concat(Object.keys(newRecord))), ); return Object.fromEntries( allKeys - .map((key) => { + .map((key): [string, unknown] | null => { const oldValue = oldRecord[key]; const newValue = newRecord[key]; if (typeof oldValue !== typeof newValue) { @@ -105,7 +105,11 @@ const diffObjects = ( oldValue !== null && newValue !== null ) { - const diffed = diffObjects(oldValue, newValue); + // Both branches are non-null objects (their `typeof` matched above). + const diffed = diffObjects( + oldValue as Record, + newValue as Record, + ); if (Object.keys(diffed).length) { return [key, diffed]; } @@ -116,7 +120,7 @@ const diffObjects = ( } return null; }) - .filter((e): e is [string, any] => !!e), + .filter((e): e is [string, unknown] => !!e), ); }; @@ -139,20 +143,23 @@ export const calculateDiff = ( ), updated: Object.fromEntries( Object.keys(newState) - .map((id) => { + .map((id): [string, [TLRecord, TLRecord]] | null => { const oldRecord = oldState[id]; const newRecord = newState[id]; if (!oldRecord || !newRecord) { return null; } - const diffed = diffObjects(oldRecord, newRecord); + const diffed = diffObjects( + oldRecord as unknown as Record, + newRecord as unknown as Record, + ); if (Object.keys(diffed).length) { return [id, [oldRecord, newRecord]]; } return null; }) - .filter((e): e is [string, any] => !!e), + .filter((e): e is [string, [TLRecord, TLRecord]] => !!e), ), }; };