[ENG-2149] Support dropping multiple images onto the canvas

apps/roam/src/components/canvas/Tldraw.tsx

@@ -31,7 +31,6 @@ import {
   createShapeId,
   TLPointerEventInfo,
   TLExternalContent,
-  MediaHelpers,
   AssetRecordType,
   TLAsset,
   TLAssetId,
@@ -117,6 +116,7 @@ import {
 } from "./useCanvasStoreAdapterArgs";
 import { shouldCreateAutoCanvasRelations } from "./autoCanvasRelationsSuppression";
 import posthog from "posthog-js";
+import { uploadCanvasFileToRoam } from "~/utils/roamCanvasAssetStore";
 import { getPersonalSetting } from "~/components/settings/utils/accessors";
 import { PERSONAL_KEYS } from "~/components/settings/utils/settingKeys";
 import { json, normalizeProps } from "~/utils/getBlockProps";
@@ -1541,16 +1541,6 @@ const InsideEditorAndUiContext = ({
   );
 
   useEffect(() => {
-    // https://tldraw.dev/examples/data/assets/hosted-images
-    const ACCEPTED_IMG_TYPE = [
-      "image/jpeg",
-      "image/png",
-      "image/gif",
-      "image/svg+xml",
-      "image/webp",
-    ];
-    const isImage = (ext: string) => ACCEPTED_IMG_TYPE.includes(ext);
-
     // Register default handlers for images and videos
     registerDefaultExternalContentHandlers(
       editor,
@@ -1638,62 +1628,6 @@ const InsideEditorAndUiContext = ({
     };
 
     editor.registerExternalContentHandler("text", textHandler);
-    editor.registerExternalContentHandler(
-      "files",
-      // eslint-disable-next-line @typescript-eslint/no-misused-promises
-      async (content: TLExternalContent) => {
-        if (content.type !== "files") {
-          console.error("Expected files, received:", content.type);
-          return;
-        }
-        const file = content.files[0];
-
-        const url = await window.roamAlphaAPI.file.upload({ file });
-        const dataUrl = url.replace(/^!\[\]\(/, "").replace(/\)$/, "");
-        // TODO add video support
-        const isImageType = isImage(file.type);
-        if (!isImageType) {
-          console.error("Unsupported file type:", file.type);
-          return;
-        }
-        const size = await MediaHelpers.getImageSize(file);
-        const isAnimated = await MediaHelpers.isAnimated(file);
-        const assetId: TLAssetId = AssetRecordType.createId(
-          getHashForString(dataUrl),
-        );
-        const shapeType = isImageType ? "image" : "video";
-        const asset: TLAsset = AssetRecordType.create({
-          id: assetId,
-          type: shapeType,
-          typeName: "asset",
-          props: {
-            name: file.name,
-            src: dataUrl,
-            w: size.w,
-            h: size.h,
-            ...fileSizeProps(getValidFileSize(file)),
-            mimeType: file.type,
-            isAnimated,
-          },
-        });
-        editor.createAssets([asset]);
-
-        const position = editor.getViewportPageBounds().center;
-
-        editor.createShape({
-          type: "image",
-          x: position.x - size.w / 2,
-          y: position.y - size.h / 2,
-          props: { assetId, w: size.w, h: size.h },
-        });
-        posthog.capture("Canvas: Asset Added", {
-          source: "file-drop",
-          mimeType: file.type,
-        });
-
-        return asset;
-      },
-    );
     //https://github.com/tldraw/tldraw/blob/v2.3.x/packages/tldraw/src/lib/defaultExternalContentHandlers.ts#L183
     editor.registerExternalContentHandler(
       "svg-text",
@@ -1731,8 +1665,17 @@ const InsideEditorAndUiContext = ({
           type: "image/svg+xml",
         });
 
-        const url = await window.roamAlphaAPI.file.upload({ file });
-        const dataUrl = url.replace(/^!\[\]\(/, "").replace(/\)$/, "");
+        let dataUrl: string;
+        try {
+          dataUrl = await uploadCanvasFileToRoam(file, "svg-paste");
+        } catch (error) {
+          toasts.addToast({
+            title: msg("assets.files.upload-failed"),
+            severity: "error",
+          });
+          console.error(error);
+          return;
+        }
 
         const assetId: TLAssetId = AssetRecordType.createId(
           getHashForString(dataUrl),
@@ -1766,11 +1709,6 @@ const InsideEditorAndUiContext = ({
           y: position.y - height / 2,
           props: { assetId, w: width, h: height },
         });
-        posthog.capture("Canvas: Asset Added", {
-          source: "svg-paste",
-          mimeType: "image/svg+xml",
-        });
-
         return asset;
       },
     );

apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx

@@ -2,14 +2,14 @@ import { useSync } from "@tldraw/sync";
 import {
   TLAnyBindingUtilConstructor,
   TLAnyShapeUtilConstructor,
-  TLAssetStore,
   TLStoreWithStatus,
   defaultBindingUtils,
   defaultShapeUtils,
   MigrationSequence,
 } from "tldraw";
 import { useMemo } from "react";
 import { getCurrentRoamTldrawUserInfo } from "~/utils/roamTldrawUserInfo";
+import { createRoamAssetStore } from "~/utils/roamCanvasAssetStore";
 
 /** Base URL for tldraw-sync-cloudflare worker. Use https (not wss) - useSync upgrades to WebSocket. */
 export const TLDRAW_CLOUDFLARE_SYNC_WS_BASE_URL =
@@ -37,20 +37,6 @@ export const getSyncRoomId = ({ pageUid }: { pageUid: string }): string => {
     .replace(/=+$/g, "");
 };
 
-const parseRoamUploadResponse = (value: string): string => {
-  return value.replace(/^!\[\]\(/, "").replace(/\)$/, "");
-};
-
-const createRoamAssetStore = (): TLAssetStore => {
-  return {
-    upload: async (_asset, file) => {
-      const response = await window.roamAlphaAPI.file.upload({ file });
-      return parseRoamUploadResponse(response);
-    },
-    resolve: (asset) => asset.props.src,
-  };
-};
-
 export const useCloudflareSyncStore = ({
   pageUid,
   migrations,

apps/roam/src/components/canvas/useRoamStore.ts

@@ -26,6 +26,7 @@ import {
 import { AddPullWatch } from "roamjs-components/types";
 import { LEGACY_SCHEMA } from "~/data/legacyTldrawSchema";
 import internalError from "~/utils/internalError";
+import { createRoamAssetStore } from "~/utils/roamCanvasAssetStore";
 
 const THROTTLE = 350;
 
@@ -93,6 +94,9 @@ const createCanvasStore = ({
     migrations,
     shapeUtils: [...defaultShapeUtils, ...customShapeUtils],
     bindingUtils: [...defaultBindingUtils, ...customBindingUtils],
+    // Without this, tldraw inlines dropped media as base64 into the shape's
+    // asset, which we then persist into the page's block props.
+    assets: createRoamAssetStore(),
   });
 
 const getPersistedRoamCanvasState = ({

apps/roam/src/utils/__tests__/roamCanvasAssetStore.test.ts

@@ -0,0 +1,143 @@
+import { describe, expect, it, vi } from "vitest";
+import {
+  createRoamAssetStore,
+  parseRoamUploadResponse,
+} from "~/utils/roamCanvasAssetStore";
+
+const setRoamAlphaAPI = (roamAlphaAPI: unknown): void => {
+  (globalThis as { window: unknown }).window = { roamAlphaAPI };
+};
+
+const createUploadSpy = (urls: string[]) => {
+  let call = 0;
+  return vi.fn(() => Promise.resolve(urls[call++] ?? urls[urls.length - 1]));
+};
+
+const fakeFile = (name: string, type = "image/png"): File =>
+  ({ name, type, size: 1024 }) as unknown as File;
+
+describe("parseRoamUploadResponse", () => {
+  it("unwraps the markdown image Roam returns from file.upload", () => {
+    expect(
+      parseRoamUploadResponse(
+        "![](https://firebasestorage.googleapis.com/v0/b/x/o/imgs%2Fapp%2Fg%2Fa.png?alt=media)",
+      ),
+    ).toBe(
+      "https://firebasestorage.googleapis.com/v0/b/x/o/imgs%2Fapp%2Fg%2Fa.png?alt=media",
+    );
+  });
+
+  // Roam picks the wrapper by file type, not one wrapper for everything.
+  // A video comes back as a {{[[video]]}} render component, and treating that
+  // as an image left "{{[[video]]: " glued to the front of the url, which the
+  // tldraw schema rejected and which crashed the whole canvas.
+  it("unwraps the video render component Roam returns for a video", () => {
+    expect(
+      parseRoamUploadResponse(
+        "{{[[video]]: https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2Fg%2Fo0geRItw_H.mp4?alt=media&token=c254db91-ec06-4519-b43d-1beeef402758}}",
+      ),
+    ).toBe(
+      "https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2Fg%2Fo0geRItw_H.mp4?alt=media&token=c254db91-ec06-4519-b43d-1beeef402758",
+    );
+  });
+
+  it("unwraps the other render components Roam uses per file type", () => {
+    expect(parseRoamUploadResponse("{{[[audio]]: https://x.test/a.mp3}}")).toBe(
+      "https://x.test/a.mp3",
+    );
+    expect(parseRoamUploadResponse("{{[[pdf]]: https://x.test/a.pdf}}")).toBe(
+      "https://x.test/a.pdf",
+    );
+    expect(parseRoamUploadResponse("[a.zip](https://x.test/a.zip)")).toBe(
+      "https://x.test/a.zip",
+    );
+  });
+
+  it("leaves a bare url untouched", () => {
+    expect(parseRoamUploadResponse("https://example.com/a.png")).toBe(
+      "https://example.com/a.png",
+    );
+  });
+});
+
+describe("createRoamAssetStore upload validation", () => {
+  // A src that isn't a url fails tldraw's schema inside store.put, which is
+  // outside the file handler's try/catch and takes the canvas down with an
+  // error boundary. Fail here instead, where it becomes a toast.
+  it("throws instead of returning a src the canvas schema will reject", async () => {
+    setRoamAlphaAPI({
+      file: { upload: () => Promise.resolve("upload failed: quota exceeded") },
+    });
+
+    await expect(
+      createRoamAssetStore().upload({} as never, fakeFile("a.png")),
+    ).rejects.toThrow(/could not find a url/i);
+  });
+
+  it("accepts the url out of any wrapper Roam used", async () => {
+    setRoamAlphaAPI({
+      file: {
+        upload: () => Promise.resolve("{{[[video]]: https://x.test/a.mp4}}"),
+      },
+    });
+
+    await expect(
+      createRoamAssetStore().upload(
+        {} as never,
+        fakeFile("a.mp4", "video/mp4"),
+      ),
+    ).resolves.toBe("https://x.test/a.mp4");
+  });
+});
+
+describe("createRoamAssetStore", () => {
+  it("uploads a file to Roam and returns the bare url", async () => {
+    const upload = createUploadSpy(["![](https://example.com/a.png)"]);
+    setRoamAlphaAPI({ file: { upload } });
+
+    const store = createRoamAssetStore();
+    const file = fakeFile("a.png");
+
+    await expect(store.upload({} as never, file)).resolves.toBe(
+      "https://example.com/a.png",
+    );
+    expect(upload).toHaveBeenCalledWith({ file });
+  });
+
+  // ENG-2149: dropping several images at once must upload every one of them.
+  // tldraw's default "files" content handler calls the asset store once per
+  // file, so the store has to stay stateless and per-file.
+  it("uploads every file of a multi-file drop to its own url", async () => {
+    const upload = createUploadSpy([
+      "![](https://example.com/a.png)",
+      "![](https://example.com/b.png)",
+      "![](https://example.com/c.png)",
+    ]);
+    setRoamAlphaAPI({ file: { upload } });
+
+    const store = createRoamAssetStore();
+    const files = [fakeFile("a.png"), fakeFile("b.png"), fakeFile("c.png")];
+
+    const srcs = await Promise.all(
+      files.map((file) => store.upload({} as never, file)),
+    );
+
+    expect(srcs).toEqual([
+      "https://example.com/a.png",
+      "https://example.com/b.png",
+      "https://example.com/c.png",
+    ]);
+    expect(upload).toHaveBeenCalledTimes(3);
+  });
+
+  it("resolves an asset to the url stored in its props", () => {
+    const store = createRoamAssetStore();
+    const asset = {
+      props: { src: "https://example.com/a.png" },
+    } as never;
+
+    expect(store.resolve?.(asset, {} as never)).toBe(
+      "https://example.com/a.png",
+    );
+  });
+});

apps/roam/src/utils/roamCanvasAssetStore.ts

@@ -0,0 +1,58 @@
+import posthog from "posthog-js";
+import type { TLAssetStore } from "tldraw";
+
+/**
+ * `roamAlphaAPI.file.upload` doesn't resolve to a bare url. It resolves to
+ * whatever Roam markup renders that file, and the markup depends on the file
+ * type: `![](url)` for an image, `{{[[video]]: url}}` for a video,
+ * `{{[[audio]]: url}}`, `{{[[pdf]]: url}}`, `[name](url)` for anything else.
+ * The canvas wants the url on its own, so pull it back out of the wrapper
+ * rather than stripping any one wrapper's punctuation.
+ */
+export const parseRoamUploadResponse = (value: string): string => {
+  const url = value.match(/https?:\/\/[^\s)}\]]+/)?.[0];
+  return url ?? value.trim();
+};
+
+/**
+ * Upload one canvas file to Roam's file store and return the url to put in an
+ * asset's `src`. Every canvas upload goes through here, whether it came from a
+ * drop, a paste, or the asset store.
+ *
+ * Throws when Roam's response has no url in it. A src that isn't a url only
+ * fails later, inside `store.put`, which is past the point the caller can catch
+ * it — the canvas dies with an error boundary. Failing here keeps the blast
+ * radius to the one file.
+ */
+export const uploadCanvasFileToRoam = async (
+  file: File,
+  source: "file-drop" | "svg-paste" = "file-drop",
+): Promise<string> => {
+  const response = await window.roamAlphaAPI.file.upload({ file });
+  const src = parseRoamUploadResponse(response);
+
+  if (!/^https?:\/\//.test(src)) {
+    throw new Error(
+      `Could not find a url in Roam's upload response for ${file.name}: ${response}`,
+    );
+  }
+
+  posthog.capture("Canvas: Asset Added", { source, mimeType: file.type });
+  return src;
+};
+
+/**
+ * The canvas's asset store: uploads canvas media to Roam's file store instead of
+ * inlining it as base64 (tldraw's default), which would bloat the page's block
+ * props.
+ *
+ * This is deliberately the *only* thing we customize about asset handling. Every
+ * caller of `editor.uploadAsset` funnels through here — tldraw's own external
+ * content handlers own the rest: iterating a multi-file drop, enforcing size and
+ * mime-type limits, and placing the resulting shapes. Overriding a layer above
+ * this is what made multi-image drops drop all but the first image (ENG-2149).
+ */
+export const createRoamAssetStore = (): TLAssetStore => ({
+  upload: (_asset, file) => uploadCanvasFileToRoam(file),
+  resolve: (asset) => asset.props.src,
+});