From f372c86039bd38d1e2f561cf688f8143dd68d535 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Thu, 2 Apr 2026 16:21:03 -0400 Subject: [PATCH 1/6] changes --- apps/roam/src/components/canvas/Tldraw.tsx | 15 ++-- ...tlesOnLoad.ts => syncCanvasNodesOnLoad.ts} | 79 ++++++++++++++++++- 2 files changed, 85 insertions(+), 9 deletions(-) rename apps/roam/src/utils/{syncCanvasNodeTitlesOnLoad.ts => syncCanvasNodesOnLoad.ts} (64%) diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index d19bfea5e..fce4abd2b 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/apps/roam/src/components/canvas/Tldraw.tsx @@ -93,7 +93,6 @@ import ToastListener, { dispatchToastEvent } from "./ToastListener"; import { CanvasDrawerPanel } from "./CanvasDrawer"; import { ClipboardPanel, ClipboardProvider } from "./Clipboard"; import internalError from "~/utils/internalError"; -import { syncCanvasNodeTitlesOnLoad } from "~/utils/syncCanvasNodeTitlesOnLoad"; import { AUTO_CANVAS_RELATIONS_KEY } from "~/data/userSettings"; import { getSetting } from "~/utils/extensionSettings"; import { isPluginTimerReady, waitForPluginTimer } from "~/utils/pluginTimer"; @@ -117,6 +116,7 @@ import { } from "./useCanvasStoreAdapterArgs"; import posthog from "posthog-js"; import { json, normalizeProps } from "~/utils/getBlockProps"; +import { syncCanvasNodesOnLoad } from "~/utils/syncCanvasNodesOnLoad"; declare global { // eslint-disable-next-line @typescript-eslint/consistent-type-definitions @@ -1003,14 +1003,15 @@ const TldrawCanvasShared = ({ appRef.current = app; - void syncCanvasNodeTitlesOnLoad( - app, - allNodes.map((n) => n.type), - allRelationIds, - ).catch((error) => { + void syncCanvasNodesOnLoad({ + editor: app, + nodeTypeIds: allNodes.map((n) => n.type), + relationShapeTypeIds: allRelationIds, + extensionAPI, + }).catch((error) => { internalError({ error, - type: "Canvas: Sync node titles on load", + type: "Canvas: Sync nodes on load", }); }); diff --git a/apps/roam/src/utils/syncCanvasNodeTitlesOnLoad.ts b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts similarity index 64% rename from apps/roam/src/utils/syncCanvasNodeTitlesOnLoad.ts rename to apps/roam/src/utils/syncCanvasNodesOnLoad.ts index acd74df5d..3ae395e91 100644 --- a/apps/roam/src/utils/syncCanvasNodeTitlesOnLoad.ts +++ b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts @@ -1,5 +1,7 @@ import type { Editor } from "tldraw"; +import type { OnloadArgs } from "roamjs-components/types"; import type { DiscourseNodeShape } from "~/components/canvas/DiscourseNodeUtil"; +import calcCanvasNodeSizeAndImg from "./calcCanvasNodeSizeAndImg"; /** * Query Roam for current :node/title or :block/string for each uid. @@ -56,11 +58,37 @@ const deleteNodeShapeAndRelations = ( * - Updates shapes whose title changed * - Removes shapes whose uid no longer exists in the graph */ +export const syncCanvasNodesOnLoad = async ({ + editor, + nodeTypeIds, + relationShapeTypeIds, + extensionAPI, +}: { + editor: Editor; + nodeTypeIds: string[]; + relationShapeTypeIds: string[]; + extensionAPI: OnloadArgs["extensionAPI"]; +}): Promise => { + const { discourseNodeShapes, uidToTitle } = await syncCanvasNodeTitlesOnLoad( + editor, + nodeTypeIds, + relationShapeTypeIds, + ); + await syncCanvasKeyImagesOnLoad({ + editor, + discourseNodeShapes, + uidToTitle, + extensionAPI, + }); +}; export const syncCanvasNodeTitlesOnLoad = async ( editor: Editor, nodeTypeIds: string[], relationShapeTypeIds: string[], -): Promise => { +): Promise<{ + discourseNodeShapes: DiscourseNodeShape[]; + uidToTitle: Map; +}> => { const nodeTypeSet = new Set(nodeTypeIds); const relationIds = new Set(relationShapeTypeIds); const allRecords = editor.store.allRecords(); @@ -72,7 +100,8 @@ export const syncCanvasNodeTitlesOnLoad = async ( ) as DiscourseNodeShape[]; const uids = [...new Set(discourseNodeShapes.map((s) => s.props.uid))]; - if (uids.length === 0) return; + if (uids.length === 0) + return { discourseNodeShapes: [], uidToTitle: new Map() }; const uidToTitle = await queryTitlesByUids(uids); @@ -104,4 +133,50 @@ export const syncCanvasNodeTitlesOnLoad = async ( })), ); } + + return { discourseNodeShapes, uidToTitle }; +}; + +const syncCanvasKeyImagesOnLoad = async ({ + editor, + discourseNodeShapes, + uidToTitle, + extensionAPI, +}: { + editor: Editor; + discourseNodeShapes: DiscourseNodeShape[]; + uidToTitle: Map; + extensionAPI: OnloadArgs["extensionAPI"]; +}): Promise => { + const survivingShapes = discourseNodeShapes.filter((s) => + uidToTitle.has(s.props.uid), + ); + const imageUpdates: { + id: DiscourseNodeShape["id"]; + type: string; + props: { imageUrl: string; w: number; h: number }; + }[] = []; + + await Promise.all( + survivingShapes.map(async (shape) => { + const title = uidToTitle.get(shape.props.uid) ?? shape.props.title ?? ""; + const { w, h, imageUrl } = await calcCanvasNodeSizeAndImg({ + nodeText: title, + uid: shape.props.uid, + nodeType: shape.type, + extensionAPI, + }); + if ((shape.props.imageUrl ?? "") !== imageUrl) { + imageUpdates.push({ + id: shape.id, + type: shape.type, + props: { imageUrl, w, h }, + }); + } + }), + ); + + if (imageUpdates.length > 0) { + editor.updateShapes(imageUpdates); + } }; From cc64ec77090cb4baa29646b937677303867838c6 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 6 Apr 2026 13:29:21 -0400 Subject: [PATCH 2/6] address PR comment --- .../src/utils/calcCanvasNodeSizeAndImg.ts | 64 ++++++++++++++----- apps/roam/src/utils/syncCanvasNodesOnLoad.ts | 36 ++++++++--- 2 files changed, 74 insertions(+), 26 deletions(-) diff --git a/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts b/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts index fe37643e3..d6ab58f42 100644 --- a/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts +++ b/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts @@ -76,7 +76,15 @@ const getFirstImageByUid = (uid: string): string | null => { return findFirstImage(tree); }; -const calcCanvasNodeSizeAndImg = async ({ +const getNodeCanvasSettings = (nodeType: string) => { + const allNodes = getDiscourseNodes(); + const canvasSettings = Object.fromEntries( + allNodes.map((n) => [n.type, { ...n.canvasSettings }]), + ); + return canvasSettings[nodeType] || {}; +}; + +export const getCanvasNodeKeyImageUrl = async ({ nodeText, uid, nodeType, @@ -86,26 +94,16 @@ const calcCanvasNodeSizeAndImg = async ({ uid: string; nodeType: string; extensionAPI: OnloadArgs["extensionAPI"]; -}) => { - const allNodes = getDiscourseNodes(); - const canvasSettings = Object.fromEntries( - allNodes.map((n) => [n.type, { ...n.canvasSettings }]), - ); +}): Promise => { const { "query-builder-alias": qbAlias = "", "key-image": isKeyImage = "", "key-image-option": keyImageOption = "", - } = canvasSettings[nodeType] || {}; - - const { w, h } = measureCanvasNodeText({ - ...DEFAULT_STYLE_PROPS, - maxWidth: MAX_WIDTH, - text: nodeText, - }); + } = getNodeCanvasSettings(nodeType); - if (!isKeyImage) return { w, h, imageUrl: "" }; + if (!isKeyImage) return ""; - let imageUrl; + let imageUrl: string | null; if (keyImageOption === "query-builder") { const parentUid = resolveQueryBuilderRef({ queryRef: qbAlias, @@ -122,14 +120,46 @@ const calcCanvasNodeSizeAndImg = async ({ } else { imageUrl = getFirstImageByUid(uid); } + return imageUrl ?? ""; +}; + +const calcCanvasNodeSizeAndImg = async ({ + nodeText, + uid, + nodeType, + extensionAPI, +}: { + nodeText: string; + uid: string; + nodeType: string; + extensionAPI: OnloadArgs["extensionAPI"]; +}) => { + const { w, h } = measureCanvasNodeText({ + ...DEFAULT_STYLE_PROPS, + maxWidth: MAX_WIDTH, + text: nodeText, + }); + + const imageUrl = await getCanvasNodeKeyImageUrl({ + nodeText, + uid, + nodeType, + extensionAPI, + }); + if (!imageUrl) return { w, h, imageUrl: "" }; try { const { width, height } = await loadImage(imageUrl); - if (!width || !height || !Number.isFinite(width) || !Number.isFinite(height)) { + if ( + !width || + !height || + !Number.isFinite(width) || + !Number.isFinite(height) + ) { return { w, h, imageUrl: "" }; } - + const aspectRatio = width / height; const nodeImageHeight = w / aspectRatio; const newHeight = h + nodeImageHeight; diff --git a/apps/roam/src/utils/syncCanvasNodesOnLoad.ts b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts index 3ae395e91..05bfe099c 100644 --- a/apps/roam/src/utils/syncCanvasNodesOnLoad.ts +++ b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts @@ -1,7 +1,9 @@ import type { Editor } from "tldraw"; import type { OnloadArgs } from "roamjs-components/types"; import type { DiscourseNodeShape } from "~/components/canvas/DiscourseNodeUtil"; -import calcCanvasNodeSizeAndImg from "./calcCanvasNodeSizeAndImg"; +import calcCanvasNodeSizeAndImg, { + getCanvasNodeKeyImageUrl, +} from "./calcCanvasNodeSizeAndImg"; /** * Query Roam for current :node/title or :block/string for each uid. @@ -157,22 +159,38 @@ const syncCanvasKeyImagesOnLoad = async ({ props: { imageUrl: string; w: number; h: number }; }[] = []; - await Promise.all( + // First pass: cheaply fetch imageUrls (no image loading) to find which shapes changed. + const urlResults = await Promise.all( survivingShapes.map(async (shape) => { const title = uidToTitle.get(shape.props.uid) ?? shape.props.title ?? ""; + const imageUrl = await getCanvasNodeKeyImageUrl({ + nodeText: title, + uid: shape.props.uid, + nodeType: shape.type, + extensionAPI, + }); + return { shape, title, imageUrl }; + }), + ); + + const changedShapes = urlResults.filter( + ({ shape, imageUrl }) => (shape.props.imageUrl ?? "") !== imageUrl, + ); + + // Second pass: load images only for shapes whose URL changed, to compute new dimensions. + await Promise.all( + changedShapes.map(async ({ shape, title }) => { const { w, h, imageUrl } = await calcCanvasNodeSizeAndImg({ nodeText: title, uid: shape.props.uid, nodeType: shape.type, extensionAPI, }); - if ((shape.props.imageUrl ?? "") !== imageUrl) { - imageUpdates.push({ - id: shape.id, - type: shape.type, - props: { imageUrl, w, h }, - }); - } + imageUpdates.push({ + id: shape.id, + type: shape.type, + props: { imageUrl, w, h }, + }); }), ); From 9f99bcb47937dd4ec57dafdab91c3778717cb2fb Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 6 Apr 2026 17:59:29 -0400 Subject: [PATCH 3/6] address PR comment --- apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts b/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts index d6ab58f42..8f9aa24cb 100644 --- a/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts +++ b/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts @@ -76,7 +76,7 @@ const getFirstImageByUid = (uid: string): string | null => { return findFirstImage(tree); }; -const getNodeCanvasSettings = (nodeType: string) => { +const getNodeCanvasSettings = (nodeType: string): Record => { const allNodes = getDiscourseNodes(); const canvasSettings = Object.fromEntries( allNodes.map((n) => [n.type, { ...n.canvasSettings }]), From 9515eba6f3ffb544f581809d6e1fa188a1e2a0ea Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 15 Apr 2026 15:47:58 -0400 Subject: [PATCH 4/6] address PR comments --- .../src/utils/calcCanvasNodeSizeAndImg.ts | 45 +++- apps/roam/src/utils/syncCanvasNodesOnLoad.ts | 238 +++++++++++++++--- 2 files changed, 252 insertions(+), 31 deletions(-) diff --git a/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts b/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts index 8f9aa24cb..fd4fee42a 100644 --- a/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts +++ b/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts @@ -10,7 +10,7 @@ import { render as renderToast } from "roamjs-components/components/Toast"; import { loadImage } from "./loadImage"; import { DEFAULT_STYLE_PROPS } from "~/components/canvas/DiscourseNodeUtil"; -const extractFirstImageUrl = (text: string): string | null => { +export const extractFirstImageUrl = (text: string): string | null => { const regex = /!\[.*?\]\((https:\/\/[^)]+)\)/; const result = text.match(regex) || resolveRefs(text).match(regex); return result ? result[1] : null; @@ -71,7 +71,7 @@ const findFirstImage = ( return null; }; -const getFirstImageByUid = (uid: string): string | null => { +export const getFirstImageByUid = (uid: string): string | null => { const tree = getFullTreeByParentUid(uid); return findFirstImage(tree); }; @@ -123,6 +123,47 @@ export const getCanvasNodeKeyImageUrl = async ({ return imageUrl ?? ""; }; +/** + * Compute canvas node dimensions using a pre-fetched imageUrl. + * Skips querying for the imageUrl — caller must supply it. + */ +export const calcCanvasNodeDimensionsWithUrl = async ({ + nodeText, + imageUrl, +}: { + nodeText: string; + imageUrl: string; +}): Promise<{ w: number; h: number }> => { + const { w, h } = measureCanvasNodeText({ + ...DEFAULT_STYLE_PROPS, + maxWidth: MAX_WIDTH, + text: nodeText, + }); + + if (!imageUrl) return { w, h }; + + try { + const { width, height } = await loadImage(imageUrl); + if ( + !width || + !height || + !Number.isFinite(width) || + !Number.isFinite(height) + ) { + return { w, h }; + } + const aspectRatio = width / height; + return { w, h: h + w / aspectRatio }; + } catch { + renderToast({ + id: "tldraw-image-load-fail", + content: "Failed to load image", + intent: "warning", + }); + return { w, h }; + } +}; + const calcCanvasNodeSizeAndImg = async ({ nodeText, uid, diff --git a/apps/roam/src/utils/syncCanvasNodesOnLoad.ts b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts index 05bfe099c..947319260 100644 --- a/apps/roam/src/utils/syncCanvasNodesOnLoad.ts +++ b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts @@ -1,9 +1,17 @@ import type { Editor } from "tldraw"; import type { OnloadArgs } from "roamjs-components/types"; import type { DiscourseNodeShape } from "~/components/canvas/DiscourseNodeUtil"; -import calcCanvasNodeSizeAndImg, { - getCanvasNodeKeyImageUrl, +import { DEFAULT_STYLE_PROPS } from "~/components/canvas/DiscourseNodeUtil"; +import { MAX_WIDTH } from "~/components/canvas/Tldraw"; +import { + extractFirstImageUrl, + getFirstImageByUid, + calcCanvasNodeDimensionsWithUrl, } from "./calcCanvasNodeSizeAndImg"; +import getDiscourseNodes from "./getDiscourseNodes"; +import resolveQueryBuilderRef from "./resolveQueryBuilderRef"; +import runQuery from "./runQuery"; +import { measureCanvasNodeText } from "./measureCanvasNodeText"; /** * Query Roam for current :node/title or :block/string for each uid. @@ -55,10 +63,13 @@ const deleteNodeShapeAndRelations = ( }; /** - * On canvas load: sync discourse node shape titles with Roam and remove shapes whose nodes no longer exist. + * On canvas load: sync discourse node shape titles and key images with Roam, + * and remove shapes whose nodes no longer exist. * - Queries Roam for current title per uid via async.fast.q * - Updates shapes whose title changed * - Removes shapes whose uid no longer exists in the graph + * - Updates key images for surviving shapes, recomputing dimensions only when + * a shape gains or loses an image */ export const syncCanvasNodesOnLoad = async ({ editor, @@ -139,6 +150,91 @@ export const syncCanvasNodeTitlesOnLoad = async ( return { discourseNodeShapes, uidToTitle }; }; +type BlockNode = { + uid: string; + string: string; + order: number; + parentUid: string; + children: BlockNode[]; +}; + +const findFirstImageDfs = (blocks: BlockNode[]): string | null => { + for (const block of blocks) { + const imageUrl = extractFirstImageUrl(block.string); + if (imageUrl) return imageUrl; + const nested = findFirstImageDfs(block.children); + if (nested) return nested; + } + return null; +}; + +/** + * Single Roam query to find the first markdown image URL across multiple pages. + * Fetches all blocks with their parent UIDs, reconstructs the tree in JS, + * then DFS-traverses in page order to find the first image. + */ +const batchGetFirstImageUrlsByUids = async ( + uids: string[], +): Promise> => { + if (uids.length === 0) return new Map(); + + // Each row: [pageUid, blockUid, string, order, parentUid] + // parentUid === pageUid for top-level blocks, a block uid for nested ones. + const results = (await window.roamAlphaAPI.data.async.fast.q( + `[:find ?pageUid ?blockUid ?string ?order ?parentUid + :in $ [?pageUid ...] + :where [?page :block/uid ?pageUid] + [?block :block/page ?page] + [?block :block/uid ?blockUid] + [?block :block/string ?string] + [?block :block/order ?order] + [?parent :block/children ?block] + [?parent :block/uid ?parentUid]]`, + uids, + )) as [string, string, string, number, string][]; + + // Build a flat block map per page. + const pageToBlockMap = new Map>(); + for (const [pageUid, blockUid, str, order, parentUid] of results) { + if (!pageToBlockMap.has(pageUid)) pageToBlockMap.set(pageUid, new Map()); + pageToBlockMap + .get(pageUid)! + .set(blockUid, { + uid: blockUid, + string: str, + order, + parentUid, + children: [], + }); + } + + // Reconstruct the tree and collect root blocks (direct children of the page). + const pageToRootBlocks = new Map(); + for (const [pageUid, blockMap] of pageToBlockMap) { + const rootBlocks: BlockNode[] = []; + for (const block of blockMap.values()) { + if (block.parentUid === pageUid) { + rootBlocks.push(block); + } else { + blockMap.get(block.parentUid)?.children.push(block); + } + } + for (const block of blockMap.values()) { + block.children.sort((a, b) => a.order - b.order); + } + rootBlocks.sort((a, b) => a.order - b.order); + pageToRootBlocks.set(pageUid, rootBlocks); + } + + const uidToImageUrl = new Map(); + for (const uid of uids) { + const rootBlocks = pageToRootBlocks.get(uid) ?? []; + uidToImageUrl.set(uid, findFirstImageDfs(rootBlocks) ?? ""); + } + + return uidToImageUrl; +}; + const syncCanvasKeyImagesOnLoad = async ({ editor, discourseNodeShapes, @@ -153,44 +249,128 @@ const syncCanvasKeyImagesOnLoad = async ({ const survivingShapes = discourseNodeShapes.filter((s) => uidToTitle.has(s.props.uid), ); - const imageUpdates: { - id: DiscourseNodeShape["id"]; - type: string; - props: { imageUrl: string; w: number; h: number }; - }[] = []; + if (survivingShapes.length === 0) return; + + // Compute canvas settings once to avoid calling getDiscourseNodes() per shape. + const allNodes = getDiscourseNodes(); + const nodeTypeToCanvasSettings = Object.fromEntries( + allNodes.map((n) => [n.type, n.canvasSettings as Record]), + ); + + // Separate shapes by key-image fetch strategy. + const firstImageShapes: DiscourseNodeShape[] = []; + const queryBuilderShapes: DiscourseNodeShape[] = []; + for (const shape of survivingShapes) { + const settings = nodeTypeToCanvasSettings[shape.type] ?? {}; + if (!settings["key-image"]) continue; + if (settings["key-image-option"] === "query-builder") { + queryBuilderShapes.push(shape); + } else { + firstImageShapes.push(shape); + } + } + + console.log("nodeTypeToCanvasSettings", nodeTypeToCanvasSettings); + console.log( + "firstImageShapes", + firstImageShapes.map((s) => ({ type: s.type, uid: s.props.uid })), + ); + console.log( + "queryBuilderShapes", + queryBuilderShapes.map((s) => ({ type: s.type, uid: s.props.uid })), + ); + + // Batch query for "First image on page" shapes — one Roam query for all. + const uidToFirstImage = await batchGetFirstImageUrlsByUids( + firstImageShapes.map((s) => s.props.uid), + ); + console.log("uidToFirstImage", Object.fromEntries(uidToFirstImage)); - // First pass: cheaply fetch imageUrls (no image loading) to find which shapes changed. - const urlResults = await Promise.all( - survivingShapes.map(async (shape) => { + // Per-shape queries for "Query builder" shapes (inherently per-node). + const qbResults = await Promise.all( + queryBuilderShapes.map(async (shape) => { const title = uidToTitle.get(shape.props.uid) ?? shape.props.title ?? ""; - const imageUrl = await getCanvasNodeKeyImageUrl({ - nodeText: title, - uid: shape.props.uid, - nodeType: shape.type, + const settings = nodeTypeToCanvasSettings[shape.type] ?? {}; + const qbAlias = settings["query-builder-alias"] ?? ""; + const parentUid = resolveQueryBuilderRef({ + queryRef: qbAlias, extensionAPI, }); - return { shape, title, imageUrl }; + const results = await runQuery({ + extensionAPI, + parentUid, + // eslint-disable-next-line @typescript-eslint/naming-convention + inputs: { NODETEXT: title, NODEUID: shape.props.uid }, + }); + const resultUid = results.allProcessedResults[0]?.uid ?? ""; + const imageUrl = getFirstImageByUid(resultUid) ?? ""; + return { shape, imageUrl }; }), ); + const urlResults: { shape: DiscourseNodeShape; imageUrl: string }[] = [ + // Shapes with no key-image configured always get an empty imageUrl. + ...survivingShapes + .filter((s) => !nodeTypeToCanvasSettings[s.type]?.["key-image"]) + .map((shape) => ({ shape, imageUrl: "" })), + ...firstImageShapes.map((shape) => ({ + shape, + imageUrl: uidToFirstImage.get(shape.props.uid) ?? "", + })), + ...qbResults, + ]; + console.log("urlResults", urlResults); + const changedShapes = urlResults.filter( ({ shape, imageUrl }) => (shape.props.imageUrl ?? "") !== imageUrl, ); - // Second pass: load images only for shapes whose URL changed, to compute new dimensions. + console.log("changedShapes", changedShapes); + + // Only load images when imageUrl transitions between empty and non-empty, + // since those are the only cases that require recomputing dimensions. + const imageUpdates: { + id: DiscourseNodeShape["id"]; + type: string; + props: { imageUrl: string; w?: number; h?: number }; + }[] = []; + await Promise.all( - changedShapes.map(async ({ shape, title }) => { - const { w, h, imageUrl } = await calcCanvasNodeSizeAndImg({ - nodeText: title, - uid: shape.props.uid, - nodeType: shape.type, - extensionAPI, - }); - imageUpdates.push({ - id: shape.id, - type: shape.type, - props: { imageUrl, w, h }, - }); + changedShapes.map(async ({ shape, imageUrl }) => { + const prevImageUrl = shape.props.imageUrl ?? ""; + const title = uidToTitle.get(shape.props.uid) ?? shape.props.title ?? ""; + + if (prevImageUrl === "" && imageUrl !== "") { + // Image newly added: compute dimensions including image height. + const { w, h } = await calcCanvasNodeDimensionsWithUrl({ + nodeText: title, + imageUrl, + }); + imageUpdates.push({ + id: shape.id, + type: shape.type, + props: { imageUrl, w, h }, + }); + } else if (prevImageUrl !== "" && imageUrl === "") { + // Image removed: recompute as text-only dimensions. + const { w, h } = measureCanvasNodeText({ + ...DEFAULT_STYLE_PROPS, + maxWidth: MAX_WIDTH, + text: title, + }); + imageUpdates.push({ + id: shape.id, + type: shape.type, + props: { imageUrl: "", w, h }, + }); + } else { + // URL changed but both are non-empty: update imageUrl only, leave w/h. + imageUpdates.push({ + id: shape.id, + type: shape.type, + props: { imageUrl }, + }); + } }), ); From 7c541cd388738a421dd94089fb50ab2d2a566f55 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 15 Apr 2026 15:49:31 -0400 Subject: [PATCH 5/6] remove console log --- apps/roam/src/utils/syncCanvasNodesOnLoad.ts | 30 +++++--------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/apps/roam/src/utils/syncCanvasNodesOnLoad.ts b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts index 947319260..b5d30d474 100644 --- a/apps/roam/src/utils/syncCanvasNodesOnLoad.ts +++ b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts @@ -197,15 +197,13 @@ const batchGetFirstImageUrlsByUids = async ( const pageToBlockMap = new Map>(); for (const [pageUid, blockUid, str, order, parentUid] of results) { if (!pageToBlockMap.has(pageUid)) pageToBlockMap.set(pageUid, new Map()); - pageToBlockMap - .get(pageUid)! - .set(blockUid, { - uid: blockUid, - string: str, - order, - parentUid, - children: [], - }); + pageToBlockMap.get(pageUid)!.set(blockUid, { + uid: blockUid, + string: str, + order, + parentUid, + children: [], + }); } // Reconstruct the tree and collect root blocks (direct children of the page). @@ -270,21 +268,10 @@ const syncCanvasKeyImagesOnLoad = async ({ } } - console.log("nodeTypeToCanvasSettings", nodeTypeToCanvasSettings); - console.log( - "firstImageShapes", - firstImageShapes.map((s) => ({ type: s.type, uid: s.props.uid })), - ); - console.log( - "queryBuilderShapes", - queryBuilderShapes.map((s) => ({ type: s.type, uid: s.props.uid })), - ); - // Batch query for "First image on page" shapes — one Roam query for all. const uidToFirstImage = await batchGetFirstImageUrlsByUids( firstImageShapes.map((s) => s.props.uid), ); - console.log("uidToFirstImage", Object.fromEntries(uidToFirstImage)); // Per-shape queries for "Query builder" shapes (inherently per-node). const qbResults = await Promise.all( @@ -319,14 +306,11 @@ const syncCanvasKeyImagesOnLoad = async ({ })), ...qbResults, ]; - console.log("urlResults", urlResults); const changedShapes = urlResults.filter( ({ shape, imageUrl }) => (shape.props.imageUrl ?? "") !== imageUrl, ); - console.log("changedShapes", changedShapes); - // Only load images when imageUrl transitions between empty and non-empty, // since those are the only cases that require recomputing dimensions. const imageUpdates: { From e4c8a026b0f11b225ac60df7631c2e10408f859d Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 15 Apr 2026 17:47:39 -0400 Subject: [PATCH 6/6] address PR comments --- .../src/utils/calcCanvasNodeSizeAndImg.ts | 68 ++----- apps/roam/src/utils/syncCanvasNodesOnLoad.ts | 175 +++++++++++------- 2 files changed, 121 insertions(+), 122 deletions(-) diff --git a/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts b/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts index fd4fee42a..ee0914204 100644 --- a/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts +++ b/apps/roam/src/utils/calcCanvasNodeSizeAndImg.ts @@ -19,7 +19,7 @@ export const extractFirstImageUrl = (text: string): string | null => { // Matches embed, embed-path, and embed-children syntax: // {{[[embed]]: ((block-uid)) }}, {{[[embed-path]]: ((block-uid)) }}, {{[[embed-children]]: ((block-uid)) }} // Also handles multiple parentheses: {{[[embed]]: ((((block-uid)))) }} -const EMBED_REGEX = +export const EMBED_REGEX = /{{\[\[(?:embed|embed-path|embed-children)\]\]:\s*\(\(+([^)]+?)\)+\)\s*}}/i; const getBlockReferences = ( @@ -37,7 +37,7 @@ const getBlockReferences = ( return result[":block/refs"] || []; }; -const findFirstImage = ( +export const findFirstImage = ( node: TreeNode, visited = new Set(), ): string | null => { @@ -123,57 +123,19 @@ export const getCanvasNodeKeyImageUrl = async ({ return imageUrl ?? ""; }; -/** - * Compute canvas node dimensions using a pre-fetched imageUrl. - * Skips querying for the imageUrl — caller must supply it. - */ -export const calcCanvasNodeDimensionsWithUrl = async ({ - nodeText, - imageUrl, -}: { - nodeText: string; - imageUrl: string; -}): Promise<{ w: number; h: number }> => { - const { w, h } = measureCanvasNodeText({ - ...DEFAULT_STYLE_PROPS, - maxWidth: MAX_WIDTH, - text: nodeText, - }); - - if (!imageUrl) return { w, h }; - - try { - const { width, height } = await loadImage(imageUrl); - if ( - !width || - !height || - !Number.isFinite(width) || - !Number.isFinite(height) - ) { - return { w, h }; - } - const aspectRatio = width / height; - return { w, h: h + w / aspectRatio }; - } catch { - renderToast({ - id: "tldraw-image-load-fail", - content: "Failed to load image", - intent: "warning", - }); - return { w, h }; - } -}; - const calcCanvasNodeSizeAndImg = async ({ nodeText, uid, nodeType, extensionAPI, + imageUrl: preloadedImageUrl, }: { nodeText: string; uid: string; nodeType: string; extensionAPI: OnloadArgs["extensionAPI"]; + /** Pre-fetched image URL. When provided, skips calling getCanvasNodeKeyImageUrl. */ + imageUrl?: string; }) => { const { w, h } = measureCanvasNodeText({ ...DEFAULT_STYLE_PROPS, @@ -181,12 +143,15 @@ const calcCanvasNodeSizeAndImg = async ({ text: nodeText, }); - const imageUrl = await getCanvasNodeKeyImageUrl({ - nodeText, - uid, - nodeType, - extensionAPI, - }); + const imageUrl = + preloadedImageUrl !== undefined + ? preloadedImageUrl + : await getCanvasNodeKeyImageUrl({ + nodeText, + uid, + nodeType, + extensionAPI, + }); if (!imageUrl) return { w, h, imageUrl: "" }; @@ -202,10 +167,7 @@ const calcCanvasNodeSizeAndImg = async ({ } const aspectRatio = width / height; - const nodeImageHeight = w / aspectRatio; - const newHeight = h + nodeImageHeight; - - return { w, h: newHeight, imageUrl }; + return { w, h: h + w / aspectRatio, imageUrl }; } catch { renderToast({ id: "tldraw-image-load-fail", diff --git a/apps/roam/src/utils/syncCanvasNodesOnLoad.ts b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts index b5d30d474..de208a04f 100644 --- a/apps/roam/src/utils/syncCanvasNodesOnLoad.ts +++ b/apps/roam/src/utils/syncCanvasNodesOnLoad.ts @@ -3,10 +3,10 @@ import type { OnloadArgs } from "roamjs-components/types"; import type { DiscourseNodeShape } from "~/components/canvas/DiscourseNodeUtil"; import { DEFAULT_STYLE_PROPS } from "~/components/canvas/DiscourseNodeUtil"; import { MAX_WIDTH } from "~/components/canvas/Tldraw"; -import { - extractFirstImageUrl, +import type { TreeNode } from "roamjs-components/types"; +import calcCanvasNodeSizeAndImg, { + findFirstImage, getFirstImageByUid, - calcCanvasNodeDimensionsWithUrl, } from "./calcCanvasNodeSizeAndImg"; import getDiscourseNodes from "./getDiscourseNodes"; import resolveQueryBuilderRef from "./resolveQueryBuilderRef"; @@ -82,11 +82,11 @@ export const syncCanvasNodesOnLoad = async ({ relationShapeTypeIds: string[]; extensionAPI: OnloadArgs["extensionAPI"]; }): Promise => { - const { discourseNodeShapes, uidToTitle } = await syncCanvasNodeTitlesOnLoad( + const { discourseNodeShapes, uidToTitle } = await syncCanvasNodeTitlesOnLoad({ editor, nodeTypeIds, relationShapeTypeIds, - ); + }); await syncCanvasKeyImagesOnLoad({ editor, discourseNodeShapes, @@ -94,11 +94,15 @@ export const syncCanvasNodesOnLoad = async ({ extensionAPI, }); }; -export const syncCanvasNodeTitlesOnLoad = async ( - editor: Editor, - nodeTypeIds: string[], - relationShapeTypeIds: string[], -): Promise<{ +export const syncCanvasNodeTitlesOnLoad = async ({ + editor, + nodeTypeIds, + relationShapeTypeIds, +}: { + editor: Editor; + nodeTypeIds: string[]; + relationShapeTypeIds: string[]; +}): Promise<{ discourseNodeShapes: DiscourseNodeShape[]; uidToTitle: Map; }> => { @@ -152,82 +156,111 @@ export const syncCanvasNodeTitlesOnLoad = async ( type BlockNode = { uid: string; - string: string; + text: string; order: number; parentUid: string; children: BlockNode[]; }; -const findFirstImageDfs = (blocks: BlockNode[]): string | null => { - for (const block of blocks) { - const imageUrl = extractFirstImageUrl(block.string); - if (imageUrl) return imageUrl; - const nested = findFirstImageDfs(block.children); - if (nested) return nested; - } - return null; -}; - /** - * Single Roam query to find the first markdown image URL across multiple pages. - * Fetches all blocks with their parent UIDs, reconstructs the tree in JS, - * then DFS-traverses in page order to find the first image. + * Batch-fetch the first markdown image URL for multiple UIDs. + * + * The naive alternative — calling getFirstImageByUid() per shape — uses + * window.roamAlphaAPI.pull, which is synchronous and blocks the main thread. + * For a canvas with many shapes this causes a noticeable UI freeze on load. + * + * Instead, we use a single async Datalog query (data.async.fast.q) that fetches + * all block strings for all page UIDs off the main thread, reconstruct the tree + * in JS, then DFS with findFirstImage (handling embeds and block refs). + * + * Trade-off: unlike getFirstImageByUid, this fetches all blocks on the page with + * no early termination. For very large pages this may transfer more data, but the + * async execution avoids blocking the UI entirely. + * + * Block UIDs (discourse nodes that are blocks, not pages) are partitioned out + * and handled with getFirstImageByUid individually since the page-scoped Datalog + * query does not apply to them. */ const batchGetFirstImageUrlsByUids = async ( uids: string[], ): Promise> => { if (uids.length === 0) return new Map(); - // Each row: [pageUid, blockUid, string, order, parentUid] - // parentUid === pageUid for top-level blocks, a block uid for nested ones. - const results = (await window.roamAlphaAPI.data.async.fast.q( - `[:find ?pageUid ?blockUid ?string ?order ?parentUid - :in $ [?pageUid ...] - :where [?page :block/uid ?pageUid] - [?block :block/page ?page] - [?block :block/uid ?blockUid] - [?block :block/string ?string] - [?block :block/order ?order] - [?parent :block/children ?block] - [?parent :block/uid ?parentUid]]`, + // Identify which UIDs are page UIDs — the batch tree query only works for pages. + const pageUidRows = (await window.roamAlphaAPI.data.async.fast.q( + `[:find ?uid + :in $ [?uid ...] + :where [?e :block/uid ?uid] + [?e :node/title]]`, uids, - )) as [string, string, string, number, string][]; - - // Build a flat block map per page. - const pageToBlockMap = new Map>(); - for (const [pageUid, blockUid, str, order, parentUid] of results) { - if (!pageToBlockMap.has(pageUid)) pageToBlockMap.set(pageUid, new Map()); - pageToBlockMap.get(pageUid)!.set(blockUid, { - uid: blockUid, - string: str, - order, - parentUid, - children: [], - }); - } + )) as [string][]; - // Reconstruct the tree and collect root blocks (direct children of the page). - const pageToRootBlocks = new Map(); - for (const [pageUid, blockMap] of pageToBlockMap) { - const rootBlocks: BlockNode[] = []; - for (const block of blockMap.values()) { - if (block.parentUid === pageUid) { - rootBlocks.push(block); - } else { - blockMap.get(block.parentUid)?.children.push(block); - } + const pageUidSet = new Set(pageUidRows.map(([uid]) => uid)); + const pageUids = [...pageUidSet]; + const blockUids = uids.filter((uid) => !pageUidSet.has(uid)); + + const uidToImageUrl = new Map(); + + // Batch tree query for page UIDs. + if (pageUids.length > 0) { + // Each row: [pageUid, blockUid, string, order, parentUid] + // parentUid === pageUid for top-level blocks, a block uid for nested ones. + const results = (await window.roamAlphaAPI.data.async.fast.q( + `[:find ?pageUid ?blockUid ?string ?order ?parentUid + :in $ [?pageUid ...] + :where [?page :block/uid ?pageUid] + [?block :block/page ?page] + [?block :block/uid ?blockUid] + [?block :block/string ?string] + [?block :block/order ?order] + [?parent :block/children ?block] + [?parent :block/uid ?parentUid]]`, + pageUids, + )) as [string, string, string, number, string][]; + + const pageToBlockMap = new Map>(); + for (const [pageUid, blockUid, text, order, parentUid] of results) { + if (!pageToBlockMap.has(pageUid)) pageToBlockMap.set(pageUid, new Map()); + pageToBlockMap.get(pageUid)!.set(blockUid, { + uid: blockUid, + text, + order, + parentUid, + children: [], + }); } - for (const block of blockMap.values()) { - block.children.sort((a, b) => a.order - b.order); + + for (const pageUid of pageUids) { + const blockMap = pageToBlockMap.get(pageUid); + if (!blockMap) { + uidToImageUrl.set(pageUid, ""); + continue; + } + const rootBlocks: BlockNode[] = []; + for (const block of blockMap.values()) { + if (block.parentUid === pageUid) { + rootBlocks.push(block); + } else { + blockMap.get(block.parentUid)?.children.push(block); + } + } + for (const block of blockMap.values()) { + block.children.sort((a, b) => a.order - b.order); + } + rootBlocks.sort((a, b) => a.order - b.order); + // Use a synthetic root so findFirstImage can traverse all top-level blocks. + const syntheticRoot = { + uid: pageUid, + text: "", + children: rootBlocks as unknown as TreeNode[], + } as TreeNode; + uidToImageUrl.set(pageUid, findFirstImage(syntheticRoot) ?? ""); } - rootBlocks.sort((a, b) => a.order - b.order); - pageToRootBlocks.set(pageUid, rootBlocks); } - const uidToImageUrl = new Map(); - for (const uid of uids) { - const rootBlocks = pageToRootBlocks.get(uid) ?? []; - uidToImageUrl.set(uid, findFirstImageDfs(rootBlocks) ?? ""); + // Block UIDs: getFirstImageByUid handles both pages and blocks correctly. + for (const uid of blockUids) { + uidToImageUrl.set(uid, getFirstImageByUid(uid) ?? ""); } return uidToImageUrl; @@ -326,8 +359,12 @@ const syncCanvasKeyImagesOnLoad = async ({ if (prevImageUrl === "" && imageUrl !== "") { // Image newly added: compute dimensions including image height. - const { w, h } = await calcCanvasNodeDimensionsWithUrl({ + // Pass the pre-fetched imageUrl to skip re-fetching it. + const { w, h } = await calcCanvasNodeSizeAndImg({ nodeText: title, + uid: shape.props.uid, + nodeType: shape.type, + extensionAPI, imageUrl, }); imageUpdates.push({