diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index 505833d02..6c1d8d5bf 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/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 { parseRoamUploadResponse } 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", @@ -1732,7 +1666,7 @@ const InsideEditorAndUiContext = ({ }); const url = await window.roamAlphaAPI.file.upload({ file }); - const dataUrl = url.replace(/^!\[\]\(/, "").replace(/\)$/, ""); + const dataUrl = parseRoamUploadResponse(url); const assetId: TLAssetId = AssetRecordType.createId( getHashForString(dataUrl), diff --git a/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx b/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx index 3bddf46ae..8069f7857 100644 --- a/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx +++ b/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx @@ -2,7 +2,6 @@ import { useSync } from "@tldraw/sync"; import { TLAnyBindingUtilConstructor, TLAnyShapeUtilConstructor, - TLAssetStore, TLStoreWithStatus, defaultBindingUtils, defaultShapeUtils, @@ -10,6 +9,8 @@ import { } from "tldraw"; import { useMemo } from "react"; import { getCurrentRoamTldrawUserInfo } from "~/utils/roamTldrawUserInfo"; +import { createRoamAssetStore } from "~/utils/roamCanvasAssetStore"; +import { captureCanvasAssetUploaded } from "./canvasAssetTelemetry"; /** 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 +38,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, @@ -66,7 +53,10 @@ export const useCloudflareSyncStore = ({ customShapeTypes: string[]; customBindingTypes: string[]; }): CloudflareCanvasStoreAdapterResult => { - const assets = useMemo(() => createRoamAssetStore(), []); + const assets = useMemo( + () => createRoamAssetStore({ onUpload: captureCanvasAssetUploaded }), + [], + ); const shapeUtils = useMemo( () => [...defaultShapeUtils, ...customShapeUtils], [customShapeUtils], diff --git a/apps/roam/src/components/canvas/canvasAssetTelemetry.ts b/apps/roam/src/components/canvas/canvasAssetTelemetry.ts new file mode 100644 index 000000000..86fe8fccc --- /dev/null +++ b/apps/roam/src/components/canvas/canvasAssetTelemetry.ts @@ -0,0 +1,14 @@ +import posthog from "posthog-js"; + +/** + * Fired once per file uploaded to the canvas. Both canvas store adapters (local + * block-props and Cloudflare sync) route their uploads through the same asset + * store, so this is the single place assets are counted. The "file-drop" source + * covers drops and pastes alike, matching what this event has always reported. + */ +export const captureCanvasAssetUploaded = ({ file }: { file: File }): void => { + posthog.capture("Canvas: Asset Added", { + source: "file-drop", + mimeType: file.type, + }); +}; diff --git a/apps/roam/src/components/canvas/useRoamStore.ts b/apps/roam/src/components/canvas/useRoamStore.ts index 78989afaa..f00152704 100644 --- a/apps/roam/src/components/canvas/useRoamStore.ts +++ b/apps/roam/src/components/canvas/useRoamStore.ts @@ -26,6 +26,8 @@ import { import { AddPullWatch } from "roamjs-components/types"; import { LEGACY_SCHEMA } from "~/data/legacyTldrawSchema"; import internalError from "~/utils/internalError"; +import { createRoamAssetStore } from "~/utils/roamCanvasAssetStore"; +import { captureCanvasAssetUploaded } from "./canvasAssetTelemetry"; const THROTTLE = 350; @@ -93,6 +95,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({ onUpload: captureCanvasAssetUploaded }), }); const getPersistedRoamCanvasState = ({ diff --git a/apps/roam/src/utils/__tests__/roamCanvasAssetStore.test.ts b/apps/roam/src/utils/__tests__/roamCanvasAssetStore.test.ts new file mode 100644 index 000000000..3f229246e --- /dev/null +++ b/apps/roam/src/utils/__tests__/roamCanvasAssetStore.test.ts @@ -0,0 +1,171 @@ +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("reports each upload to the onUpload callback", async () => { + const upload = createUploadSpy(["![](https://example.com/a.png)"]); + setRoamAlphaAPI({ file: { upload } }); + + const onUpload = vi.fn(); + const store = createRoamAssetStore({ onUpload }); + const file = fakeFile("a.gif", "image/gif"); + + await store.upload({} as never, file); + + expect(onUpload).toHaveBeenCalledWith({ file }); + }); + + it("does not fail the upload when the telemetry callback throws", async () => { + const upload = createUploadSpy(["![](https://example.com/a.png)"]); + setRoamAlphaAPI({ file: { upload } }); + + const store = createRoamAssetStore({ + onUpload: () => { + throw new Error("posthog exploded"); + }, + }); + + await expect(store.upload({} as never, fakeFile("a.png"))).resolves.toBe( + "https://example.com/a.png", + ); + }); + + 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", + ); + }); +}); diff --git a/apps/roam/src/utils/roamCanvasAssetStore.ts b/apps/roam/src/utils/roamCanvasAssetStore.ts new file mode 100644 index 000000000..b5b55431b --- /dev/null +++ b/apps/roam/src/utils/roamCanvasAssetStore.ts @@ -0,0 +1,55 @@ +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(); +}; + +/** + * 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 = ({ + onUpload, +}: { + /** Fired after each successful upload. Used for telemetry; never throws. */ + onUpload?: (args: { file: File }) => void; +} = {}): TLAssetStore => ({ + upload: async (_asset, file) => { + const response = await window.roamAlphaAPI.file.upload({ file }); + const src = parseRoamUploadResponse(response); + + // A src that isn't a url fails the tldraw schema later, inside store.put, + // which is past the point where the file handler can catch it — the canvas + // dies with an error boundary. Failing here turns it into a toast for that + // one file, and the rest of a multi-file drop still lands. + if (!/^https?:\/\//.test(src)) { + throw new Error( + `Could not find a url in Roam's upload response for ${file.name}: ${response}`, + ); + } + + try { + onUpload?.({ file }); + } catch (error) { + console.error("Canvas asset upload telemetry failed", error); + } + return src; + }, + resolve: (asset) => asset.props.src, +});