From d9cca44c597283c44e62a851d89975ebb32ae0c1 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 14 Sep 2026 10:59:17 -0400 Subject: [PATCH 1/3] ENG-2227 Add link support to tldraw text shapes in Roam Text shapes could not carry a link, so a canvas narrative could not point at its detailed source. tldraw gates its built-in Edit link action on `'url' in shape.props`, and the stock text shape has no such prop, so the fix is schema-level rather than UI wiring. Subclass TextShapeUtil to add a `url` prop and render the hyperlink button, and replace the stock util through a shared list so all four stores agree. Existing text shapes are backfilled by migration v6 of the repo's own sequence; without it they fail validation and the canvas will not open. The sync worker validates `text` against stock props, so the client now declares it alongside the custom shape types. Co-Authored-By: Claude Opus 5 --- apps/roam/src/components/Export.tsx | 4 +- .../discourseRelationMigrations.ts | 13 ++ .../canvas/TextShapeWithLinkUtil.tsx | 75 +++++++++++ apps/roam/src/components/canvas/Tldraw.tsx | 4 +- .../canvas/TldrawCanvasCloudflareSync.tsx | 8 +- .../src/components/canvas/baseShapeUtils.ts | 13 ++ .../src/components/canvas/useRoamStore.ts | 4 +- .../src/utils/__tests__/textShapeLink.test.ts | 121 ++++++++++++++++++ apps/roam/src/utils/textShapeLink.ts | 27 ++++ 9 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx create mode 100644 apps/roam/src/components/canvas/baseShapeUtils.ts create mode 100644 apps/roam/src/utils/__tests__/textShapeLink.test.ts create mode 100644 apps/roam/src/utils/textShapeLink.ts diff --git a/apps/roam/src/components/Export.tsx b/apps/roam/src/components/Export.tsx index cb0567d71e..fe6925d7fc 100644 --- a/apps/roam/src/components/Export.tsx +++ b/apps/roam/src/components/Export.tsx @@ -16,6 +16,7 @@ import { Radio, FormGroup, } from "@blueprintjs/core"; +import { baseShapeUtils } from "~/components/canvas/baseShapeUtils"; import React, { useState, useEffect, useMemo, FormEvent } from "react"; import MenuItemSelect from "roamjs-components/components/MenuItemSelect"; import { saveAs } from "file-saver"; @@ -50,7 +51,6 @@ import { TLParentId, getIndexAbove, TLShape, - defaultShapeUtils, defaultBindingUtils, } from "tldraw"; import { @@ -433,7 +433,7 @@ const ExportDialog: ExportDialogComponent = ({ const tlStore = createTLStore({ migrations, - shapeUtils: [...defaultShapeUtils, ...customShapeUtils], + shapeUtils: [...baseShapeUtils, ...customShapeUtils], bindingUtils: [...defaultBindingUtils, ...customBindingUtils], }); diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts b/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts index 9e49a075ac..860ba76fad 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts @@ -18,6 +18,7 @@ import { createMigrationIds } from "tldraw"; import { RelationBinding } from "./DiscourseRelationBindings"; import { getRelationColor } from "./DiscourseRelationUtil"; import { DISCOURSE_NODE_SHAPE_TYPE } from "~/components/canvas/DiscourseNodeUtil"; +import { backfillTextShapeUrl, isTextShapeRecord } from "~/utils/textShapeLink"; const SEQUENCE_ID_BASE = "com.roam-research.discourse-graphs"; @@ -52,6 +53,7 @@ export const createMigrations = ({ AddSizeAndFontFamily: 3, RemoveNullAssetFileSize: 4, MigrateNodeTypeToDiscourseNode: 5, + AddTextShapeUrl: 6, }); return createMigrationSequence({ sequenceId: `${SEQUENCE_ID_BASE}`, @@ -203,6 +205,17 @@ export const createMigrations = ({ shape.type = DISCOURSE_NODE_SHAPE_TYPE; }, }, + { + id: versions["AddTextShapeUrl"], + scope: "record", + filter: (r: any) => isTextShapeRecord(r), + up: (shape: any) => { + backfillTextShapeUrl(shape); + }, + down: (shape: any) => { + delete shape.props.url; + }, + }, ], }); }; diff --git a/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx new file mode 100644 index 0000000000..208352e750 --- /dev/null +++ b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx @@ -0,0 +1,75 @@ +import React from "react"; +import { + T, + TextShapeUtil, + TLTextShape, + stopEventPropagation, + textShapeProps, +} from "tldraw"; + +export type TextShapeWithLinkProps = TLTextShape["props"] & { url: string }; + +const LINK_ICON = + "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='30' height='30' fill='none'%3E%3Cpath stroke='%23000' stroke-linecap='round' stroke-linejoin='round' stroke-width='2' d='M13 5H7a2 2 0 0 0-2 2v16a2 2 0 0 0 2 2h16a2 2 0 0 0 2-2v-6M19 5h6m0 0v6m0-6L13 17'/%3E%3C/svg%3E"; + +// tldraw does not export HyperlinkButton, so this mirrors its markup to keep +// text links visually identical to geo links. +const HyperlinkButton = ({ + url, + zoomLevel, +}: { + url: string; + zoomLevel: number; +}): JSX.Element => ( + +
+ +); + +export const getTextShapeUrl = (shape: TLTextShape): string => + (shape.props as Partial).url ?? ""; + +const textShapeWithLinkProps = { ...textShapeProps, url: T.linkUrl }; + +// Adds the `url` prop that tldraw's built-in Edit link action gates on +// (`'url' in shape.props`), so text shapes reuse the geo link UI unchanged. +export class TextShapeWithLinkUtil extends TextShapeUtil { + static override props = textShapeWithLinkProps; + + override getDefaultProps(): TextShapeWithLinkProps { + return { ...super.getDefaultProps(), url: "" }; + } + + // The cast bridges two React type copies in the dependency tree, which make + // the base signature's JSX.Element nominally distinct from ours. + override component( + shape: TLTextShape, + ): ReturnType { + const url = getTextShapeUrl(shape); + return ( + <> + {super.component(shape)} + {url && ( + + )} + + ) as ReturnType; + } +} diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index 505833d02b..21d02b4695 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/apps/roam/src/components/canvas/Tldraw.tsx @@ -5,6 +5,7 @@ import React, { useEffect, useCallback, } from "react"; +import { baseShapeUtils } from "./baseShapeUtils"; import { Icon } from "@blueprintjs/core"; import ExtensionApiContextProvider, { useExtensionAPI, @@ -24,7 +25,6 @@ import { TldrawUi, defaultBindingUtils, defaultShapeTools, - defaultShapeUtils, defaultTools, useEditor, VecModel, @@ -1339,7 +1339,7 @@ const TldrawCanvasShared = ({ // instanceId={initialState.instanceId} autoFocus={false} initialState="select" - shapeUtils={[...defaultShapeUtils, ...customShapeUtils]} + shapeUtils={[...baseShapeUtils, ...customShapeUtils]} tools={[...defaultTools, ...defaultShapeTools, ...customTools]} bindingUtils={[...defaultBindingUtils, ...customBindingUtils]} components={editorComponents} diff --git a/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx b/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx index 3bddf46ae8..67dbd8e944 100644 --- a/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx +++ b/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx @@ -1,11 +1,11 @@ import { useSync } from "@tldraw/sync"; +import { baseShapeUtils } from "./baseShapeUtils"; import { TLAnyBindingUtilConstructor, TLAnyShapeUtilConstructor, TLAssetStore, TLStoreWithStatus, defaultBindingUtils, - defaultShapeUtils, MigrationSequence, } from "tldraw"; import { useMemo } from "react"; @@ -68,7 +68,7 @@ export const useCloudflareSyncStore = ({ }): CloudflareCanvasStoreAdapterResult => { const assets = useMemo(() => createRoamAssetStore(), []); const shapeUtils = useMemo( - () => [...defaultShapeUtils, ...customShapeUtils], + () => [...baseShapeUtils, ...customShapeUtils], [customShapeUtils], ); const bindingUtils = useMemo( @@ -80,7 +80,9 @@ export const useCloudflareSyncStore = ({ const uri = useMemo(() => { const roomId = getSyncRoomId({ pageUid }); const query = new URLSearchParams(); - for (const shapeType of customShapeTypes) { + // The worker validates `text` against stock tldraw props unless it is + // declared here, which would reject the url prop text links add. + for (const shapeType of [...customShapeTypes, "text"]) { query.append("shapeType", shapeType); } for (const bindingType of customBindingTypes) { diff --git a/apps/roam/src/components/canvas/baseShapeUtils.ts b/apps/roam/src/components/canvas/baseShapeUtils.ts new file mode 100644 index 0000000000..7992764925 --- /dev/null +++ b/apps/roam/src/components/canvas/baseShapeUtils.ts @@ -0,0 +1,13 @@ +import { + defaultShapeUtils, + TextShapeUtil, + TLAnyShapeUtilConstructor, +} from "tldraw"; +import { TextShapeWithLinkUtil } from "./TextShapeWithLinkUtil"; + +// tldraw throws when a shape type is registered twice, so the stock text util +// has to be replaced rather than appended. Every store must use this same list. +export const baseShapeUtils: TLAnyShapeUtilConstructor[] = + defaultShapeUtils.map((util) => + util === TextShapeUtil ? TextShapeWithLinkUtil : util, + ); diff --git a/apps/roam/src/components/canvas/useRoamStore.ts b/apps/roam/src/components/canvas/useRoamStore.ts index 78989afaa6..724c6caa65 100644 --- a/apps/roam/src/components/canvas/useRoamStore.ts +++ b/apps/roam/src/components/canvas/useRoamStore.ts @@ -1,4 +1,5 @@ import { TLRecord } from "@tldraw/tlschema"; +import { baseShapeUtils } from "./baseShapeUtils"; import nanoid from "nanoid"; import { useRef, useMemo, useEffect, useState } from "react"; import getBasicTreeByParentUid from "roamjs-components/queries/getBasicTreeByParentUid"; @@ -14,7 +15,6 @@ import { import { SerializedStore, StoreSnapshot } from "@tldraw/store"; import { defaultBindingUtils, - defaultShapeUtils, getIndices, loadSnapshot, MigrationSequence, @@ -91,7 +91,7 @@ const createCanvasStore = ({ }): TLStore => createTLStore({ migrations, - shapeUtils: [...defaultShapeUtils, ...customShapeUtils], + shapeUtils: [...baseShapeUtils, ...customShapeUtils], bindingUtils: [...defaultBindingUtils, ...customBindingUtils], }); diff --git a/apps/roam/src/utils/__tests__/textShapeLink.test.ts b/apps/roam/src/utils/__tests__/textShapeLink.test.ts new file mode 100644 index 0000000000..f5f70d1b1c --- /dev/null +++ b/apps/roam/src/utils/__tests__/textShapeLink.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from "vitest"; +import { createTLSchema, defaultShapeSchemas } from "tldraw"; +import { + backfillTextShapeUrl, + hasLinkUrlProp, + isTextShapeRecord, +} from "~/utils/textShapeLink"; + +const makeTextShape = ( + props: Record = {}, +): { + id: string; + typeName: string; + type: string; + props: Record; +} & Record => ({ + id: "shape:t1", + typeName: "shape", + type: "text", + x: 0, + y: 0, + rotation: 0, + index: "a1", + parentId: "page:page", + isLocked: false, + opacity: 1, + meta: {}, + props: { + color: "black", + size: "m", + font: "draw", + textAlign: "middle", + w: 100, + text: "hello", + scale: 1, + autoSize: true, + ...props, + }, +}); + +describe("isTextShapeRecord", () => { + it("matches only text shape records", () => { + expect(isTextShapeRecord(makeTextShape())).toBe(true); + expect(isTextShapeRecord({ ...makeTextShape(), type: "geo" })).toBe(false); + expect(isTextShapeRecord({ typeName: "asset", type: "text" })).toBe(false); + expect(isTextShapeRecord(null)).toBe(false); + }); +}); + +describe("backfillTextShapeUrl", () => { + it("adds an empty url to a shape created before link support", () => { + const shape = makeTextShape(); + expect(hasLinkUrlProp(shape)).toBe(false); + backfillTextShapeUrl(shape); + expect(shape.props.url).toBe(""); + }); + + it("makes the shape eligible for the Edit link action", () => { + const shape = makeTextShape(); + backfillTextShapeUrl(shape); + // tldraw's useHasLinkShapeSelected gates on this exact check + expect(hasLinkUrlProp(shape)).toBe(true); + }); + + it("never overwrites a url the user already set", () => { + const shape = makeTextShape({ url: "https://example.com" }); + backfillTextShapeUrl(shape); + backfillTextShapeUrl(shape); + expect(shape.props.url).toBe("https://example.com"); + }); + + it("leaves non-text shapes untouched", () => { + const geo = { ...makeTextShape(), type: "geo" }; + backfillTextShapeUrl(geo); + expect(hasLinkUrlProp(geo)).toBe(false); + }); +}); + +describe("text shape url persistence", () => { + const extendedSchema = createTLSchema({ + shapes: { + ...defaultShapeSchemas, + text: { + ...defaultShapeSchemas.text, + props: { + ...defaultShapeSchemas.text.props, + url: defaultShapeSchemas.geo.props.url, + }, + }, + }, + }); + + const validate = (shape: unknown) => + extendedSchema.validateRecord( + {} as never, + shape as never, + "initialize", + null, + ); + + it("rejects a pre-link text shape that was never migrated", () => { + expect(() => validate(makeTextShape())).toThrow(/url/); + }); + + it("accepts a migrated text shape and round-trips the link", () => { + const shape = makeTextShape(); + backfillTextShapeUrl(shape); + expect(() => validate(shape)).not.toThrow(); + + shape.props.url = "https://example.com"; + const migrated = extendedSchema.migrateStoreSnapshot({ + store: { [shape.id]: structuredClone(shape) } as never, + schema: extendedSchema.serialize(), + }); + expect(migrated.type).toBe("success"); + const migratedShape = ( + migrated as unknown as { value: Record } + ).value[shape.id]; + expect(migratedShape.props.url).toBe("https://example.com"); + }); +}); diff --git a/apps/roam/src/utils/textShapeLink.ts b/apps/roam/src/utils/textShapeLink.ts new file mode 100644 index 0000000000..35cd9bc674 --- /dev/null +++ b/apps/roam/src/utils/textShapeLink.ts @@ -0,0 +1,27 @@ +type UnknownRecord = { + typeName?: unknown; + type?: unknown; + props?: Record; +}; + +export const isTextShapeRecord = (record: unknown): boolean => { + if (typeof record !== "object" || record === null) return false; + const { typeName, type, props } = record as UnknownRecord; + return typeName === "shape" && type === "text" && typeof props === "object"; +}; + +// tldraw gates its Edit link action on `'url' in shape.props`, so a text shape +// missing the key is silently ineligible rather than failing loudly. +export const hasLinkUrlProp = (record: unknown): boolean => { + if (typeof record !== "object" || record === null) return false; + const { props } = record as UnknownRecord; + return typeof props === "object" && props !== null && "url" in props; +}; + +// Never overwrite an existing url: an earlier migration shipped an +// unconditional assignment and had to be corrected (PR #916). +export const backfillTextShapeUrl = (record: unknown): void => { + if (!isTextShapeRecord(record)) return; + const { props } = record as Required; + if (props.url === undefined) props.url = ""; +}; From 9c42b8423b6497b8d4c3358d65416021bf921ed9 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 14 Sep 2026 11:16:39 -0400 Subject: [PATCH 2/3] Fix sync room schema and move the text link icon out of the glyphs Declaring "text" to the sync worker as if it were a custom shape type blanked its schema entry to {}, which dropped textShapeMigrations. The room then reported com.tldraw.shape.text version 0 against every client's 2, so getMigrationsSince returned 'Incompatible schema?' and every client was rejected on connect while existing rooms failed to load. The worker now extends the default text schema instead, keeping its migrations, and refuses to blank any default shape type. Covered by syncWorkerRoomSchema.test.ts. A text shape's bounds hug its glyphs, so tldraw's in-bounds link button landed on the last word. Sit it just outside the right edge instead. Also fix a missing separator that merged the two hyperlink button class names into one invalid token, guard isTextShapeRecord against a null props object, and assert that the text util was actually replaced. Co-Authored-By: Claude Opus 5 --- .../canvas/TextShapeWithLinkUtil.tsx | 10 ++- .../canvas/TldrawCanvasCloudflareSync.tsx | 4 +- .../src/components/canvas/baseShapeUtils.ts | 6 ++ .../src/components/canvas/tldrawStyles.ts | 8 ++ .../__tests__/syncWorkerRoomSchema.test.ts | 69 ++++++++++++++ .../__tests__/textShapeLinkStore.test.ts | 89 +++++++++++++++++++ apps/roam/src/utils/textShapeLink.ts | 7 +- .../worker/TldrawDurableObject.ts | 21 ++++- 8 files changed, 205 insertions(+), 9 deletions(-) create mode 100644 apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts create mode 100644 apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts diff --git a/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx index 208352e750..faef5a67ee 100644 --- a/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx +++ b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx @@ -22,9 +22,13 @@ const HyperlinkButton = ({ zoomLevel: number; }): JSX.Element => ( { const roomId = getSyncRoomId({ pageUid }); const query = new URLSearchParams(); - // The worker validates `text` against stock tldraw props unless it is - // declared here, which would reject the url prop text links add. - for (const shapeType of [...customShapeTypes, "text"]) { + for (const shapeType of customShapeTypes) { query.append("shapeType", shapeType); } for (const bindingType of customBindingTypes) { diff --git a/apps/roam/src/components/canvas/baseShapeUtils.ts b/apps/roam/src/components/canvas/baseShapeUtils.ts index 7992764925..521054ffa8 100644 --- a/apps/roam/src/components/canvas/baseShapeUtils.ts +++ b/apps/roam/src/components/canvas/baseShapeUtils.ts @@ -11,3 +11,9 @@ export const baseShapeUtils: TLAnyShapeUtilConstructor[] = defaultShapeUtils.map((util) => util === TextShapeUtil ? TextShapeWithLinkUtil : util, ); + +// Fail loudly rather than shipping a store whose schema lacks `url` while the +// UI still writes it, which would only surface as validation errors on save. +if (!baseShapeUtils.includes(TextShapeWithLinkUtil)) { + throw new Error("Failed to replace TextShapeUtil in the default shape utils"); +} diff --git a/apps/roam/src/components/canvas/tldrawStyles.ts b/apps/roam/src/components/canvas/tldrawStyles.ts index 03a5265e7d..0a771e78e1 100644 --- a/apps/roam/src/components/canvas/tldrawStyles.ts +++ b/apps/roam/src/components/canvas/tldrawStyles.ts @@ -10,6 +10,14 @@ export default /* css */ ` display: none; } + /* A text shape's bounds hug its glyphs, so tldraw's in-bounds link button + lands on the last word; sit it just outside the right edge instead. */ + .dg-text-link-button { + top: 50%; + right: 0; + transform: translate(100%, -50%); + } + /* Shape Render Fix */ svg.tl-svg-container { overflow: visible; diff --git a/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts new file mode 100644 index 0000000000..592f69b743 --- /dev/null +++ b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "vitest"; +import { createTLSchema, defaultShapeSchemas } from "tldraw"; +import { baseShapeUtils } from "~/components/canvas/baseShapeUtils"; + +// Mirrors apps/tldraw-sync-worker/worker/TldrawDurableObject.ts. The worker +// blanks declared shape types to {}, which is safe only for types the client +// also registers without migrations. Blanking a default type drops its +// migration sequence, so the room reports version 0 against the client's 2 and +// every client is rejected as too old. +const textUtil = baseShapeUtils.find( + (util) => util.type === "text", +) as unknown as { + props: never; + migrations: never; +}; + +const clientSchema = createTLSchema({ + shapes: { + ...defaultShapeSchemas, + text: { props: textUtil.props, migrations: textUtil.migrations }, + "discourse-node": {}, + } as never, +}); + +const textSequenceVersion = (schema: ReturnType) => + (schema.serialize() as unknown as { sequences: Record }) + .sequences["com.tldraw.shape.text"]; + +describe("sync worker room schema", () => { + it("stays migration-compatible when text keeps its own migrations", () => { + const workerSchema = createTLSchema({ + shapes: { + ...defaultShapeSchemas, + text: { + ...defaultShapeSchemas.text, + props: { + ...defaultShapeSchemas.text.props, + url: defaultShapeSchemas.geo.props.url, + }, + }, + "discourse-node": {}, + } as never, + }); + + expect(textSequenceVersion(workerSchema)).toBe( + textSequenceVersion(clientSchema), + ); + expect( + workerSchema.migrateStoreSnapshot({ + store: {} as never, + schema: clientSchema.serialize(), + }).type, + ).toBe("success"); + }); + + it("breaks if text is blanked to {} the way custom types are", () => { + const brokenSchema = createTLSchema({ + shapes: { ...defaultShapeSchemas, text: {} } as never, + }); + + expect(textSequenceVersion(brokenSchema)).toBe(0); + expect( + brokenSchema.migrateStoreSnapshot({ + store: {} as never, + schema: clientSchema.serialize(), + }).type, + ).toBe("error"); + }); +}); diff --git a/apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts b/apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts new file mode 100644 index 0000000000..a0e8e65440 --- /dev/null +++ b/apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from "vitest"; +import { createTLSchema, defaultShapeSchemas, TextShapeUtil } from "tldraw"; +import { baseShapeUtils } from "~/components/canvas/baseShapeUtils"; +import { TextShapeWithLinkUtil } from "~/components/canvas/TextShapeWithLinkUtil"; + +// Built the way createTLStore derives a schema from its utils, so this exercises +// the real util rather than a hand-rolled copy of its props. +const textUtil = baseShapeUtils.find((util) => util.type === "text"); + +const schemaFromUtils = createTLSchema({ + shapes: { + ...defaultShapeSchemas, + text: { + props: (textUtil as unknown as { props: never }).props, + migrations: (textUtil as unknown as { migrations: never }).migrations, + }, + }, +}); + +const makeTextShape = (url?: string) => ({ + id: "shape:t1", + typeName: "shape", + type: "text", + x: 0, + y: 0, + rotation: 0, + index: "a1", + parentId: "page:page", + isLocked: false, + opacity: 1, + meta: {}, + props: { + color: "black", + size: "m", + font: "draw", + textAlign: "middle", + w: 100, + text: "hello", + scale: 1, + autoSize: true, + ...(url === undefined ? {} : { url }), + }, +}); + +describe("baseShapeUtils", () => { + it("replaces the stock text util exactly once", () => { + expect(textUtil).toBe(TextShapeWithLinkUtil); + expect(baseShapeUtils).not.toContain(TextShapeUtil); + expect(baseShapeUtils.filter((u) => u.type === "text")).toHaveLength(1); + }); + + it("keeps every other default util", () => { + expect(baseShapeUtils).toHaveLength(12); + }); +}); + +describe("schema derived from the real util", () => { + const validate = (shape: unknown) => + schemaFromUtils.validateRecord( + {} as never, + shape as never, + "initialize", + null, + ); + + it("accepts a text shape carrying a link", () => { + expect(() => validate(makeTextShape("https://example.com"))).not.toThrow(); + }); + + it("accepts an empty url, the value the migration backfills", () => { + expect(() => validate(makeTextShape(""))).not.toThrow(); + }); + + it("rejects a text shape that never got the url prop", () => { + expect(() => validate(makeTextShape())).toThrow(/url/); + }); + + it("rejects a non-http url", () => { + expect(() => validate(makeTextShape("javascript:alert(1)"))).toThrow(/url/); + }); + + it("gives new text shapes a url so they are link-eligible", () => { + const defaults = new TextShapeWithLinkUtil( + {} as never, + ).getDefaultProps() as { url: string }; + expect("url" in defaults).toBe(true); + expect(defaults.url).toBe(""); + }); +}); diff --git a/apps/roam/src/utils/textShapeLink.ts b/apps/roam/src/utils/textShapeLink.ts index 35cd9bc674..88b4646fec 100644 --- a/apps/roam/src/utils/textShapeLink.ts +++ b/apps/roam/src/utils/textShapeLink.ts @@ -7,7 +7,12 @@ type UnknownRecord = { export const isTextShapeRecord = (record: unknown): boolean => { if (typeof record !== "object" || record === null) return false; const { typeName, type, props } = record as UnknownRecord; - return typeName === "shape" && type === "text" && typeof props === "object"; + return ( + typeName === "shape" && + type === "text" && + typeof props === "object" && + props !== null + ); }; // tldraw gates its Edit link action on `'url' in shape.props`, so a text shape diff --git a/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts b/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts index 4b0e2117ac..b4a9884b6d 100644 --- a/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts +++ b/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts @@ -16,17 +16,34 @@ type RoomSchemaConfig = { const STORAGE_SCHEMA_CONFIG_KEY = "schemaConfig"; +// Text shapes carry a link url; reuse geo's validator rather than redeclaring it. +const textShapeSchema = { + ...defaultShapeSchemas.text, + props: { + ...defaultShapeSchemas.text.props, + url: defaultShapeSchemas.geo.props.url, + }, +}; + const createRoomSchema = ({ shapeTypes, bindingTypes }: RoomSchemaConfig) => { + // A default shape type must keep its own props and migrations. Blanking one + // to {} drops its migration sequence, so the room reports version 0 while + // every client reports 2 and all of them are rejected as too old. const customShapeSchemas = Object.fromEntries( - shapeTypes.map((type) => [type, {}]), + shapeTypes + .filter((type) => !(type in defaultShapeSchemas)) + .map((type) => [type, {}]), ); const customBindingSchemas = Object.fromEntries( - bindingTypes.map((type) => [type, {}]), + bindingTypes + .filter((type) => !(type in defaultBindingSchemas)) + .map((type) => [type, {}]), ); return createTLSchema({ shapes: { ...defaultShapeSchemas, + text: textShapeSchema, ...customShapeSchemas, }, bindings: { From c11d41719d3d1e42541451e536108fa5d225c1b2 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 18 Sep 2026 16:38:51 -0400 Subject: [PATCH 3/3] Remove unnecessary type assertions in sync worker schema test Co-Authored-By: Claude Opus 5 --- apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts index 592f69b743..25fe47304e 100644 --- a/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts +++ b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts @@ -19,7 +19,7 @@ const clientSchema = createTLSchema({ ...defaultShapeSchemas, text: { props: textUtil.props, migrations: textUtil.migrations }, "discourse-node": {}, - } as never, + }, }); const textSequenceVersion = (schema: ReturnType) => @@ -39,7 +39,7 @@ describe("sync worker room schema", () => { }, }, "discourse-node": {}, - } as never, + }, }); expect(textSequenceVersion(workerSchema)).toBe( @@ -55,7 +55,7 @@ describe("sync worker room schema", () => { it("breaks if text is blanked to {} the way custom types are", () => { const brokenSchema = createTLSchema({ - shapes: { ...defaultShapeSchemas, text: {} } as never, + shapes: { ...defaultShapeSchemas, text: {} }, }); expect(textSequenceVersion(brokenSchema)).toBe(0);