[ENG-1287] On canvas load, update key image

[trangdoan982]

https://www.loom.com/share/1dc9445cdf5a4712af0864c579b99d10

---

[linear]

ENG-1287 On canvas load, update key image

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

[mdroidian]

Summary

Functionally this direction makes sense, but the main risk is how much Roam and the browser do on canvas open. With 20+ discourse nodes that use key images, the current flow can trigger a large number of independent resolutions (tree walks, query-builder runs, pulls, and image loads), not a small fixed cost. The highest-impact follow-ups here should be performance-focused before we lean on this in real graphs.

Why this matters

• syncCanvasKeyImagesOnLoad runs getCanvasNodeKeyImageUrl for every surviving node in parallel (Promise.all over all shapes). Each call can be quite heavy.
• For nodes whose stored URL differs from the resolved URL, calcCanvasNodeSizeAndImg calls getCanvasNodeKeyImageUrl again, so changed nodes pay for URL resolution twice before dimensions are computed. That’s an easy 2× on the hottest path when many URLs update at once (e.g. after settings or data changes).
• The second pass also loadImages in parallel for every changed shape. That can be fine for a few images but rough on large canvases (many concurrent network decodes, main-thread work).

So the concern isn’t only “many shapes” — it’s many shapes × (possibly expensive per-node Roam work + duplicate resolution + parallel image loads).

Testing / acceptance (please block merge on this)

After the performance work lands, verify on a large canvas, not a 3-node fixture:

1. Prefer an existing heavy canvas in graph plugin-testing-akamatsulab2: open a page known to host a big tldraw canvas. Aim for ≥20 discourse node shapes with distinct key images if possible.
2. If no suitable page exists, create a test page in that graph with 25+ nodes, each with a different key image (exercises URL resolution + image load paths).
3. Report concrete signals in the PR : time-to-interactive or wall time until sync finishes, any UI jank, and whether Roam feels responsive. A before/after comparison on the same canvas is ideal.

[mdroidian]

Need to update the comment to include images

[mdroidian]

This is also running an individual query for each shape. This could be hundreds of queries.

We'll have to write a custom query for any nodes that have the key-image option of First image on page. So we can run one single query rather than a query for each node. Those with Query builder reference will probably still be a Promise.all, unfortunately.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 10 additional findings in Devin Review.

[devin-ai-integration]

🔴 Discarded imageUrl return value from calcCanvasNodeSizeAndImg causes inconsistent shape state on image load failure

When an image URL is newly added to a shape (line 360: prevImageUrl === "" && imageUrl !== ""), calcCanvasNodeSizeAndImg is called and only w and h are destructured from its result. The imageUrl used in the update (line 373) comes from the outer closure, not from the function's return value. If calcCanvasNodeSizeAndImg fails to load the image (catch block at calcCanvasNodeSizeAndImg.ts:171), it returns { w: textW, h: textH, imageUrl: "" } — text-only dimensions with an empty imageUrl. But the caller ignores the returned empty imageUrl and writes the original (broken) URL to the shape along with text-only dimensions. This leaves the shape in a persistently inconsistent state: it has a non-empty imageUrl that can't be loaded, paired with dimensions that don't account for any image height.

Suggested change

	const { w, h } = await calcCanvasNodeSizeAndImg({
	nodeText: title,
	uid: shape.props.uid,
	nodeType: shape.type,
	extensionAPI,
	imageUrl,
	});
	imageUpdates.push({
	id: shape.id,
	type: shape.type,
	props: { imageUrl, w, h },
	});
	const { w, h, imageUrl: resolvedImageUrl } = await calcCanvasNodeSizeAndImg({
	nodeText: title,
	uid: shape.props.uid,
	nodeType: shape.type,
	extensionAPI,
	imageUrl,
	});
	imageUpdates.push({
	id: shape.id,
	type: shape.type,
	props: { imageUrl: resolvedImageUrl, w, h },
	});

---

Was this helpful? React with 👍 or 👎 to provide feedback.