From da09f94deb75e7bccec1238a0548bdeafa87c9a9 Mon Sep 17 00:00:00 2001 From: Michael Gartner Date: Wed, 24 Jun 2026 17:48:29 -0600 Subject: [PATCH 1/2] ENG-1955 Full migration to discourse relation shape --- apps/roam/src/components/Export.tsx | 5 +- .../components/canvas/DiscourseNodeUtil.tsx | 65 ++++--- .../DiscourseRelationBindings.tsx | 18 +- .../DiscourseRelationTool.tsx | 12 +- .../DiscourseRelationUtil.tsx | 141 +++++++++------ .../discourseRelationMigrations.ts | 164 ++++++++++++++++-- .../canvas/DiscourseRelationShape/helpers.tsx | 21 ++- apps/roam/src/components/canvas/Tldraw.tsx | 11 +- .../canvas/overlays/DragHandleOverlay.tsx | 5 +- .../canvas/useCanvasStoreAdapterArgs.ts | 16 +- .../discourseRelationMigrations.test.ts | 134 ++++++++++++++ .../src/utils/syncCanvasNodeTitlesOnLoad.ts | 9 +- 12 files changed, 461 insertions(+), 140 deletions(-) create mode 100644 apps/roam/src/utils/__tests__/discourseRelationMigrations.test.ts diff --git a/apps/roam/src/components/Export.tsx b/apps/roam/src/components/Export.tsx index 2520189b5..09525c632 100644 --- a/apps/roam/src/components/Export.tsx +++ b/apps/roam/src/components/Export.tsx @@ -364,8 +364,7 @@ const ExportDialog: ExportDialogComponent = ({ // UTILS const discourseNodeUtils = [DiscourseNodeUtil]; - const discourseRelationUtils = - createAllRelationShapeUtils(allRelationIds); + const discourseRelationUtils = createAllRelationShapeUtils(); const referencedNodeUtils = createAllReferencedNodeUtils( allAddReferencedNodeByAction, ); @@ -375,7 +374,7 @@ const ExportDialog: ExportDialogComponent = ({ ...referencedNodeUtils, ]; // BINDINGS - const relationBindings = createAllRelationBindings(allRelationIds); + const relationBindings = createAllRelationBindings(); const referencedNodeBindings = createAllReferencedNodeBindings( allAddReferencedNodeByAction, ); diff --git a/apps/roam/src/components/canvas/DiscourseNodeUtil.tsx b/apps/roam/src/components/canvas/DiscourseNodeUtil.tsx index 3adc629d1..8657afc29 100644 --- a/apps/roam/src/components/canvas/DiscourseNodeUtil.tsx +++ b/apps/roam/src/components/canvas/DiscourseNodeUtil.tsx @@ -37,7 +37,10 @@ import getDiscourseContextResults from "~/utils/getDiscourseContextResults"; import calcCanvasNodeSizeAndImg from "~/utils/calcCanvasNodeSizeAndImg"; import { createTextJsxFromSpans } from "./DiscourseRelationShape/helpers"; import { loadImage } from "~/utils/loadImage"; -import { getRelationColor } from "./DiscourseRelationShape/DiscourseRelationUtil"; +import { + DISCOURSE_RELATION_SHAPE_TYPE, + getRelationColor, +} from "./DiscourseRelationShape/DiscourseRelationUtil"; import { getPersonalSetting } from "~/components/settings/utils/accessors"; import { PERSONAL_KEYS } from "~/components/settings/utils/settingKeys"; import DiscourseContextOverlay from "~/components/DiscourseContextOverlay"; @@ -216,7 +219,11 @@ export class DiscourseNodeUtil extends BaseBoxShapeUtil { relationIds?: Set; }) { const editor = this.editor; - const bindingsToThisShape = Array.from(relationIds).flatMap((r) => + const relationBindingTypes = new Set([ + DISCOURSE_RELATION_SHAPE_TYPE, + ...relationIds, + ]); + const bindingsToThisShape = Array.from(relationBindingTypes).flatMap((r) => editor.getBindingsToShape(shape.id, r), ); const relationIdsAndType = bindingsToThisShape.map((b) => { @@ -268,24 +275,28 @@ export class DiscourseNodeUtil extends BaseBoxShapeUtil { ) .filter((id) => id !== undefined), ); - const currentShapeRelations = Array.from( - discourseContextRelationIds, - ).flatMap((relationId) => { - const bindingsToThisShape = editor.getBindingsToShape( - shape.id, - relationId, - ); - return bindingsToThisShape.map((b) => { - const arrowId = b.fromId; - const bindingsFromArrow = editor.getBindingsFromShape( - arrowId, + const relationBindingTypes = new Set([ + DISCOURSE_RELATION_SHAPE_TYPE, + ...discourseContextRelationIds, + ]); + const currentShapeRelations = Array.from(relationBindingTypes).flatMap( + (relationId) => { + const bindingsToThisShape = editor.getBindingsToShape( + shape.id, relationId, ); - const endBinding = bindingsFromArrow.find((b) => b.toId !== shape.id); - if (!endBinding) return null; - return { startId: shape.id, endId: endBinding.toId }; - }); - }); + return bindingsToThisShape.map((b) => { + const arrowId = b.fromId; + const bindingsFromArrow = editor.getBindingsFromShape( + arrowId, + relationId, + ); + const endBinding = bindingsFromArrow.find((b) => b.toId !== shape.id); + if (!endBinding) return null; + return { startId: shape.id, endId: endBinding.toId }; + }); + }, + ); const toCreate = discourseContextResults .flatMap((r) => @@ -318,13 +329,25 @@ export class DiscourseNodeUtil extends BaseBoxShapeUtil { const shapesToCreate = toCreate.map( ({ relationId, arrowId, label }, index) => { const color = getRelationColor(label, index); - return { id: arrowId, type: relationId, props: { color } }; + return { + id: arrowId, + type: DISCOURSE_RELATION_SHAPE_TYPE, + props: { + color, + labelColor: color, + text: label, + relationTypeId: relationId, + }, + }; }, ); const bindingsToCreate = toCreate.flatMap( - ({ relationId, complement, nodeId, arrowId }) => { - const staticRelationProps = { type: relationId, fromId: arrowId }; + ({ complement, nodeId, arrowId }) => { + const staticRelationProps = { + type: DISCOURSE_RELATION_SHAPE_TYPE, + fromId: arrowId, + }; return [ { ...staticRelationProps, diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationBindings.tsx b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationBindings.tsx index b9ac6c502..e173e81a3 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationBindings.tsx +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationBindings.tsx @@ -45,18 +45,12 @@ export const createAllReferencedNodeBindings = ( }; }); }; -export const createAllRelationBindings = (relationIds: string[]) => { - const relationBindings = relationIds.map((id) => { - return class RelationBindingUtil extends BaseRelationBindingUtil { - static override type = id; - }; - }); - - class DiscourseRelationFallbackBindingUtil extends BaseRelationBindingUtil { - static override type = DISCOURSE_RELATION_SHAPE_TYPE; - } - - return [...relationBindings, DiscourseRelationFallbackBindingUtil]; +export const createAllRelationBindings = () => { + return [ + class RelationBindingUtil extends BaseRelationBindingUtil { + static override type = DISCOURSE_RELATION_SHAPE_TYPE; + }, + ]; }; export type RelationBindings = { diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx index 3669a6202..e38367cde 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx @@ -5,6 +5,7 @@ import { TLStateNodeConstructor, } from "tldraw"; import { + DISCOURSE_RELATION_SHAPE_TYPE, DiscourseRelationShape, getRelationColor, } from "./DiscourseRelationUtil"; @@ -483,11 +484,14 @@ export const createAllRelationShapeTools = ( this.editor.createShape({ id, - type: this.shapeType, + type: DISCOURSE_RELATION_SHAPE_TYPE, x: originPagePoint.x, y: originPagePoint.y, props: { color, + labelColor: color, + text: name, + relationTypeId: this.shapeType, scale: this.editor.user.getIsDynamicResizeMode() ? 1 / this.editor.getZoomLevel() : 1, @@ -501,7 +505,7 @@ export const createAllRelationShapeTools = ( if (!handles) throw Error(`expected handles for arrow`); const util = this.editor.getShapeUtil( - this.shapeType, + DISCOURSE_RELATION_SHAPE_TYPE, ); const initial = this.shape; const startHandle = handles.find((h) => h.id === "start")!; @@ -536,7 +540,7 @@ export const createAllRelationShapeTools = ( { const util = this.editor.getShapeUtil( - this.shapeType, + DISCOURSE_RELATION_SHAPE_TYPE, ); const initial = this.shape; const startHandle = handles.find((h) => h.id === "start")!; @@ -554,7 +558,7 @@ export const createAllRelationShapeTools = ( // end update { const util = this.editor.getShapeUtil( - this.shapeType, + DISCOURSE_RELATION_SHAPE_TYPE, ); const initial = this.shape; const point = this.editor.getPointInShapeSpace( diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx index a44668b73..88d0f4c00 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx @@ -154,6 +154,46 @@ export const getRelationColor = ( return `${COLOR_ARRAY[index || 0]}`; }; +type ShapeWithOptionalRelationTypeId = TLShape & { + props?: { + relationTypeId?: string | null; + }; +}; + +export const getDiscourseRelationTypeId = ({ + shape, +}: { + shape: ShapeWithOptionalRelationTypeId; +}): string => { + return shape.props?.relationTypeId || shape.type; +}; + +export const getDiscourseRelationBindingType = ({ + shape, +}: { + shape: ShapeWithOptionalRelationTypeId; +}): string => { + return shape.type === DISCOURSE_RELATION_SHAPE_TYPE + ? DISCOURSE_RELATION_SHAPE_TYPE + : shape.type; +}; + +export const isDiscourseRelationShape = ( + shape: TLShape, +): shape is DiscourseRelationShape => { + return ( + shape.type === DISCOURSE_RELATION_SHAPE_TYPE || + isRelationShapeType(shape.type) + ); +}; + +const getDiscourseRelationById = ( + relationTypeId: string, +): DiscourseRelation | undefined => + Object.values(discourseContext.relations) + .flat() + .find((relation) => relation.id === relationTypeId); + export const createAllReferencedNodeUtils = ( allAddReferencedNodeByAction: AddReferencedNodeType, ): TLShapeUtilConstructor[] => { @@ -183,7 +223,10 @@ export const createAllReferencedNodeUtils = ( }; const target = editor.getShape(targetId); - const bindings = editor.getBindingsFromShape(arrow, arrow.type); + const bindings = editor.getBindingsFromShape( + arrow, + getDiscourseRelationBindingType({ shape: arrow }), + ); const sourceId = bindings.find((b) => b.toId !== targetId)?.toId; if (!sourceId) return; const source = editor.getShape(sourceId); @@ -610,12 +653,10 @@ const asDiscourseNodeShape = ( : null; }; -export const createAllRelationShapeUtils = ( - allRelationIds: string[], -): TLShapeUtilConstructor[] => { - const relationShapeUtils = allRelationIds.map((id) => { +export const createAllRelationShapeUtils = + (): TLShapeUtilConstructor[] => { class DiscourseRelationUtil extends BaseDiscourseRelationUtil { - static override type = id; + static override type = DISCOURSE_RELATION_SHAPE_TYPE; handleCreateRelationsInRoam = async ({ arrow, @@ -634,7 +675,10 @@ export const createAllRelationShapeUtils = ( this.editor.deleteShapes([arrow.id]); }; const target = editor.getShape(targetId); - const bindings = editor.getBindingsFromShape(arrow, arrow.type); + const bindings = editor.getBindingsFromShape( + arrow, + getDiscourseRelationBindingType({ shape: arrow }), + ); const sourceId = bindings.find((b) => b.toId !== targetId)?.toId; if (!sourceId) return; const source = editor.getShape(sourceId); @@ -647,8 +691,8 @@ export const createAllRelationShapeUtils = ( "Invalid shape type. Expected a DiscourseNodeShape.", ); } - const relations = Object.values(discourseContext.relations).flat(); - const relation = relations.find((r) => r.id === arrow.type); + const relationTypeId = getDiscourseRelationTypeId({ shape: arrow }); + const relation = getDiscourseRelationById(relationTypeId); if (!relation) return; const sourceNodeType = getDiscourseNodeTypeId({ shape: source }); @@ -679,22 +723,14 @@ export const createAllRelationShapeUtils = ( ); } - // If we found a matching relation with a different ID, switch to it - if (matchingRelation.id !== arrow.type) { - // Get bindings before updating the shape type - const existingBindings = editor.getBindingsFromShape( - arrow, - arrow.type, - ); - // Update the shape type - editor.updateShapes([{ id: arrow.id, type: matchingRelation.id }]); - // Update bindings to use the new relation type - for (const binding of existingBindings) { - editor.updateBinding({ - ...binding, - type: matchingRelation.id, - }); - } + if (matchingRelation.id !== relationTypeId) { + editor.updateShapes([ + { + id: arrow.id, + type: arrow.type, + props: { relationTypeId: matchingRelation.id }, + }, + ]); } if (getStoredRelationsEnabled()) { const sourceAsDNS = asDiscourseNodeShape(source, editor); @@ -783,29 +819,18 @@ export const createAllRelationShapeUtils = ( }; override getDefaultProps(): DiscourseRelationShape["props"] { - // TODO: get color from canvasSettings - - const relations = Object.values(discourseContext.relations); - // TODO - add canvas settings to relations config - const relationIndex = relations.findIndex((rs) => - rs.some((r) => r.id === id), - ); - const isValid = relationIndex >= 0 && relationIndex < relations.length; - const color = isValid ? COLOR_ARRAY[relationIndex + 1] : COLOR_ARRAY[0]; - const text = isValid ? relations[relationIndex][0].label : ""; - return { dash: "draw", size: "m", fill: "none", - color: color, - labelColor: color, + color: COLOR_ARRAY[0], + labelColor: COLOR_ARRAY[0], bend: 0, start: { x: 0, y: 0 }, end: { x: 0, y: 0 }, arrowheadStart: "none", arrowheadEnd: "arrow", - text: text, + text: "", labelPosition: 0.5, font: "draw", scale: 1, @@ -818,6 +843,8 @@ export const createAllRelationShapeUtils = ( ) => { const handleId = handle.id as ARROW_HANDLES; const bindings = getArrowBindings(this.editor, shape); + const relationTypeId = getDiscourseRelationTypeId({ shape }); + const relationBindingType = getDiscourseRelationBindingType({ shape }); if (handleId === ARROW_HANDLES.MIDDLE) { // Bending the arrow... @@ -844,7 +871,7 @@ export const createAllRelationShapeUtils = ( const update: TLShapePartial = { id: shape.id, - type: id, + type: shape.type, props: {}, }; @@ -879,7 +906,7 @@ export const createAllRelationShapeUtils = ( this.editor.canBindShapes({ fromShape: shape, toShape: targetShape, - binding: id, + binding: relationBindingType, }) ); }, @@ -923,11 +950,16 @@ export const createAllRelationShapeUtils = ( const targetNodeType = getDiscourseNodeTypeId({ shape: target }); const sourceNodeType = getDiscourseNodeTypeId({ shape: sourceNode }); - if (sourceNodeType && targetNodeType && shape.type) { + if ( + sourceNodeType && + targetNodeType && + relationTypeId && + getDiscourseRelationById(relationTypeId) + ) { const isValidConnection = this.isValidNodeConnection( sourceNodeType, targetNodeType, - shape.type, + relationTypeId, ); if (!isValidConnection) { @@ -1017,8 +1049,7 @@ export const createAllRelationShapeUtils = ( // Check if both ends are bound and determine the correct text based on direction if (newBindings.start && newBindings.end) { - const relations = Object.values(discourseContext.relations).flat(); - const relation = relations.find((r) => r.id === shape.type); + const relation = getDiscourseRelationById(relationTypeId); if (relation) { const startNode = this.editor.getShape(newBindings.start.toId); @@ -1049,6 +1080,11 @@ export const createAllRelationShapeUtils = ( ? effectiveRelation.complement : effectiveRelation.label; + if (effectiveRelation.id !== relationTypeId) { + update.props = update.props || {}; + update.props.relationTypeId = effectiveRelation.id; + } + if (shape.props.text !== newText) { update.props = update.props || {}; update.props.text = newText; @@ -1175,7 +1211,7 @@ export const createAllRelationShapeUtils = ( this.editor.canBindShapes({ fromShape: shape, toShape: targetShape, - binding: id, + binding: getDiscourseRelationBindingType({ shape }), }) ); }, @@ -1211,17 +1247,8 @@ export const createAllRelationShapeUtils = ( } }; } - return DiscourseRelationUtil; - }); - - class DiscourseRelationFallbackUtil extends BaseDiscourseRelationUtil { - static override type = DISCOURSE_RELATION_SHAPE_TYPE; - - handleCreateRelationsInRoam = (): Promise => Promise.resolve(); - } - - return [...relationShapeUtils, DiscourseRelationFallbackUtil]; -}; + return [DiscourseRelationUtil]; + }; const relationShapeProps = { ...arrowShapeProps, diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts b/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts index 9e49a075a..be50d4f88 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts @@ -15,9 +15,12 @@ import { TLBaseShape, } from "tldraw"; import { createMigrationIds } from "tldraw"; -import { RelationBinding } from "./DiscourseRelationBindings"; -import { getRelationColor } from "./DiscourseRelationUtil"; +import { + DISCOURSE_RELATION_SHAPE_TYPE, + getRelationColor, +} from "./DiscourseRelationUtil"; import { DISCOURSE_NODE_SHAPE_TYPE } from "~/components/canvas/DiscourseNodeUtil"; +import type { RelationBinding } from "./DiscourseRelationBindings"; const SEQUENCE_ID_BASE = "com.roam-research.discourse-graphs"; @@ -33,6 +36,122 @@ const getRecordTypeName = (record: unknown): string | undefined => { return typeof candidate === "string" ? candidate : undefined; }; +const hasArrowLikeProps = (record: unknown): boolean => { + if (typeof record !== "object" || record === null) return false; + const props = (record as { props?: unknown }).props; + if (typeof props !== "object" || props === null) return false; + return ( + "start" in props && + "end" in props && + "arrowheadEnd" in props && + ("bend" in props || "labelPosition" in props || "text" in props) + ); +}; + +const isIgnoredArrowLikeShapeType = ({ + recordType, + allNodeTypes, +}: { + recordType: string; + allNodeTypes: string[]; +}): boolean => { + return ( + recordType === "arrow" || + recordType === DISCOURSE_RELATION_SHAPE_TYPE || + recordType === DISCOURSE_NODE_SHAPE_TYPE || + allNodeTypes.includes(recordType) + ); +}; + +const isLegacyRelationOrReferencedShape = ({ + record, + allRelationIds, + allAddReferencedNodeActions, + allNodeTypes, +}: { + record: unknown; + allRelationIds: string[]; + allAddReferencedNodeActions: string[]; + allNodeTypes: string[]; +}): boolean => { + const recordType = getRecordType(record); + if (getRecordTypeName(record) !== "shape" || !recordType) return false; + if ( + [...allRelationIds, ...allAddReferencedNodeActions].includes(recordType) + ) { + return true; + } + return ( + hasArrowLikeProps(record) && + !isIgnoredArrowLikeShapeType({ recordType, allNodeTypes }) + ); +}; + +const isLegacyCanonicalRelationShape = ({ + record, + allRelationIds, + allAddReferencedNodeActions, + allNodeTypes, +}: { + record: unknown; + allRelationIds: string[]; + allAddReferencedNodeActions: string[]; + allNodeTypes: string[]; +}): boolean => { + const recordType = getRecordType(record); + if (getRecordTypeName(record) !== "shape" || !recordType) return false; + if (allRelationIds.includes(recordType)) return true; + if (allAddReferencedNodeActions.includes(recordType)) return false; + return ( + hasArrowLikeProps(record) && + !isIgnoredArrowLikeShapeType({ recordType, allNodeTypes }) + ); +}; + +export const migrateRelationTypesToDiscourseRelation = ({ + oldStore, + allRelationIds, + allAddReferencedNodeActions, + allNodeTypes, +}: { + oldStore: Record; + allRelationIds: string[]; + allAddReferencedNodeActions: string[]; + allNodeTypes: string[]; +}): void => { + const migratedRelationTypeIdsByShapeId = new Map(); + + for (const record of Object.values(oldStore)) { + if ( + !isLegacyCanonicalRelationShape({ + record, + allRelationIds, + allAddReferencedNodeActions, + allNodeTypes, + }) + ) { + continue; + } + + const shape = record; + const relationTypeId = shape.props?.relationTypeId || shape.type; + if (!shape.props) shape.props = {}; + shape.props.relationTypeId = relationTypeId; + shape.type = DISCOURSE_RELATION_SHAPE_TYPE; + migratedRelationTypeIdsByShapeId.set(shape.id, relationTypeId); + } + + for (const record of Object.values(oldStore)) { + if (getRecordTypeName(record) !== "binding") continue; + + const binding = record as RelationBinding; + if (!migratedRelationTypeIdsByShapeId.has(binding.fromId)) { + continue; + } + binding.type = DISCOURSE_RELATION_SHAPE_TYPE; + } +}; + export const createMigrations = ({ allRelationIds, allAddReferencedNodeActions, @@ -42,16 +161,13 @@ export const createMigrations = ({ allAddReferencedNodeActions: string[]; allNodeTypes: string[]; }) => { - const allRelationShapeIds = [ - ...allRelationIds, - ...allAddReferencedNodeActions, - ]; const versions = createMigrationIds(`${SEQUENCE_ID_BASE}`, { ExtractBindings: 1, "2.3.0": 2, AddSizeAndFontFamily: 3, RemoveNullAssetFileSize: 4, MigrateNodeTypeToDiscourseNode: 5, + MigrateRelationTypeToDiscourseRelation: 6, }); return createMigrationSequence({ sequenceId: `${SEQUENCE_ID_BASE}`, @@ -78,12 +194,12 @@ export const createMigrations = ({ >; const arrows = Object.values(oldStore).filter((r): r is OldArrow => { - const recordType = getRecordType(r); - return ( - getRecordTypeName(r) === "shape" && - !!recordType && - allRelationShapeIds.includes(recordType) - ); + return isLegacyRelationOrReferencedShape({ + record: r, + allRelationIds, + allAddReferencedNodeActions, + allNodeTypes, + }); }); for (const a of arrows) { @@ -141,12 +257,12 @@ export const createMigrations = ({ id: versions["2.3.0"], scope: "record", filter: (r: any) => { - const recordType = getRecordType(r); - return ( - getRecordTypeName(r) === "shape" && - !!recordType && - allRelationShapeIds.includes(recordType) - ); + return isLegacyRelationOrReferencedShape({ + record: r, + allRelationIds, + allAddReferencedNodeActions, + allNodeTypes, + }); }, up: (arrow: any) => { arrow.props.start = { x: 0, y: 0 }; @@ -203,6 +319,18 @@ export const createMigrations = ({ shape.type = DISCOURSE_NODE_SHAPE_TYPE; }, }, + { + id: versions["MigrateRelationTypeToDiscourseRelation"], + scope: "store", + up: (oldStore) => { + migrateRelationTypesToDiscourseRelation({ + oldStore, + allRelationIds, + allAddReferencedNodeActions, + allNodeTypes, + }); + }, + }, ], }); }; diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx b/apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx index 96aa967b1..c7220b641 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx @@ -72,6 +72,7 @@ import { import { BaseDiscourseRelationUtil, DiscourseRelationShape, + getDiscourseRelationBindingType, } from "./DiscourseRelationUtil"; let defaultPixels: { white: string; black: string } | null = null; @@ -143,7 +144,7 @@ export function getArrowBindings( ): RelationBindings { const bindings = editor.getBindingsFromShape( relation, - relation.type, // we expect relation.type to = binding.type + getDiscourseRelationBindingType({ shape: relation }), ); return { start: bindings.find((b) => b.props.terminal === "start"), @@ -376,7 +377,10 @@ function getBoundShapeInfoForTerminal( terminalName: "start" | "end", ): BoundShapeInfo | undefined { const binding = editor - .getBindingsFromShape(relation, relation.type) // we expect relation.type to = binding.type + .getBindingsFromShape( + relation, + getDiscourseRelationBindingType({ shape: relation }), + ) .find((b) => b.props.terminal === terminalName); if (!binding) return; @@ -1631,7 +1635,10 @@ export function removeArrowBinding( terminal: "start" | "end", ) { const existing = editor - .getBindingsFromShape(relation, relation.type) // we expect relation.type to = binding.type + .getBindingsFromShape( + relation, + getDiscourseRelationBindingType({ shape: relation }), + ) .filter((b) => b.props.terminal === terminal); editor.deleteBindings(existing); @@ -1645,12 +1652,10 @@ export function createOrUpdateArrowBinding( ) { const arrowId = typeof relation === "string" ? relation : relation.id; const targetId = typeof target === "string" ? target : target.id; + const bindingType = getDiscourseRelationBindingType({ shape: relation }); const existingMany = editor - .getBindingsFromShape( - arrowId, - relation.type, // we expect relation.type to = binding.type - ) + .getBindingsFromShape(arrowId, bindingType) .filter((b) => b.props.terminal === props.terminal); // if we've somehow ended up with too many bindings, delete the extras @@ -1667,7 +1672,7 @@ export function createOrUpdateArrowBinding( }); } else { editor.createBinding({ - type: relation.type, + type: bindingType, fromId: arrowId, toId: targetId, props, diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index af6ad4f3d..41e7bf04e 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/apps/roam/src/components/canvas/Tldraw.tsx @@ -89,6 +89,10 @@ import { createAllReferencedNodeTools, createAllRelationShapeTools, } from "./DiscourseRelationShape/DiscourseRelationTool"; +import { + getDiscourseRelationTypeId, + isDiscourseRelationShape, +} from "./DiscourseRelationShape/DiscourseRelationUtil"; import ConvertToDialog from "./ConvertToDialog"; import ToastListener, { dispatchToastEvent } from "./ToastListener"; import { CanvasDrawerPanel } from "./CanvasDrawer"; @@ -575,17 +579,20 @@ const TldrawCanvasShared = ({ const selectedShapes = app.getSelectedShapes(); const relationShape = selectedShapes.find( (shape) => - allRelationIds.includes(shape.type) || + isDiscourseRelationShape(shape) || allAddReferencedNodeActions.includes(shape.type), ); if (relationShape) { relationCreationRef.current.relationShapeId = relationShape.id; + const relationType = isDiscourseRelationShape(relationShape) + ? getDiscourseRelationTypeId({ shape: relationShape }) + : relationShape.type; // Check if we have a target shape if (shapeAtPoint && isDiscourseNodeShape(app, shapeAtPoint)) { posthog.capture("Canvas: Relation Created", { - relationType: relationShape.type, + relationType, toolType: relationCreationRef.current.toolType || "", }); // We have a valid target, call the relation creation method diff --git a/apps/roam/src/components/canvas/overlays/DragHandleOverlay.tsx b/apps/roam/src/components/canvas/overlays/DragHandleOverlay.tsx index 55811b5c8..3a12bec8f 100644 --- a/apps/roam/src/components/canvas/overlays/DragHandleOverlay.tsx +++ b/apps/roam/src/components/canvas/overlays/DragHandleOverlay.tsx @@ -6,6 +6,7 @@ import { } from "~/components/canvas/DiscourseNodeUtil"; import { BaseDiscourseRelationUtil, + DISCOURSE_RELATION_SHAPE_TYPE, DiscourseRelationShape, getRelationColor, } from "~/components/canvas/DiscourseRelationShape/DiscourseRelationUtil"; @@ -312,12 +313,14 @@ export const DragHandleOverlay = () => { const arrowId = createShapeId(); editor.createShape({ id: arrowId, - type: relationId, + type: DISCOURSE_RELATION_SHAPE_TYPE, x: sourceBounds.midX, y: sourceBounds.midY, props: { color, + labelColor: color, text: label, + relationTypeId: relationId, dash: "draw", size: "m", fill: "none", diff --git a/apps/roam/src/components/canvas/useCanvasStoreAdapterArgs.ts b/apps/roam/src/components/canvas/useCanvasStoreAdapterArgs.ts index aa9fcbb3c..0ff6321b7 100644 --- a/apps/roam/src/components/canvas/useCanvasStoreAdapterArgs.ts +++ b/apps/roam/src/components/canvas/useCanvasStoreAdapterArgs.ts @@ -60,12 +60,10 @@ const getUtilTypes = ({ const createShapeUtils = ({ allNodes, - allRelationIds, allAddReferencedNodeByAction, includeLegacyNodeTypes = false, }: { allNodes: DiscourseNode[]; - allRelationIds: string[]; allAddReferencedNodeByAction: AddReferencedNodeType; includeLegacyNodeTypes?: boolean; }): TLAnyShapeUtilConstructor[] => { @@ -74,20 +72,18 @@ const createShapeUtils = ({ ...(includeLegacyNodeTypes ? createLegacyDiscourseNodeShapeUtils(allNodes) : []), - ...createAllRelationShapeUtils(allRelationIds), + ...createAllRelationShapeUtils(), ...createAllReferencedNodeUtils(allAddReferencedNodeByAction), ]; }; const createBindingUtils = ({ - allRelationIds, allAddReferencedNodeByAction, }: { - allRelationIds: string[]; allAddReferencedNodeByAction: AddReferencedNodeType; }): TLAnyBindingUtilConstructor[] => { return [ - ...createAllRelationBindings(allRelationIds), + ...createAllRelationBindings(), ...createAllReferencedNodeBindings(allAddReferencedNodeByAction), ]; }; @@ -107,11 +103,9 @@ export const useCanvasStoreAdapterArgs = ({ }): CanvasStoreAdapterArgs => { const customShapeUtils = createShapeUtils({ allNodes, - allRelationIds, allAddReferencedNodeByAction, }); const customBindingUtils = createBindingUtils({ - allRelationIds, allAddReferencedNodeByAction, }); const customShapeTypes = getUtilTypes({ @@ -136,23 +130,21 @@ export const useCanvasStoreAdapterArgs = ({ pageUid, value: createShapeUtils({ allNodes, - allRelationIds, allAddReferencedNodeByAction, // Cloudflare rooms may still stream pre-migration node-id shape records. includeLegacyNodeTypes: true, }), }), - [pageUid, allNodes, allRelationIds, allAddReferencedNodeByAction], + [pageUid, allNodes, allAddReferencedNodeByAction], ).value; const stableCustomBindingUtils = useMemo( () => ({ pageUid, value: createBindingUtils({ - allRelationIds, allAddReferencedNodeByAction, }), }), - [pageUid, allRelationIds, allAddReferencedNodeByAction], + [pageUid, allAddReferencedNodeByAction], ).value; const stableCustomShapeTypes = useMemo( () => ({ diff --git a/apps/roam/src/utils/__tests__/discourseRelationMigrations.test.ts b/apps/roam/src/utils/__tests__/discourseRelationMigrations.test.ts new file mode 100644 index 000000000..b47ccfe2a --- /dev/null +++ b/apps/roam/src/utils/__tests__/discourseRelationMigrations.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("tldraw", () => ({ + createBindingId: () => "binding:mock", + createMigrationIds: (_sequenceId: string, versions: Record) => + versions, + createMigrationSequence: (migrationSequence: unknown) => migrationSequence, +})); + +vi.mock("~/components/canvas/DiscourseNodeUtil", () => ({ + DISCOURSE_NODE_SHAPE_TYPE: "discourse-node", +})); + +vi.mock( + "~/components/canvas/DiscourseRelationShape/DiscourseRelationUtil", + () => ({ + DISCOURSE_RELATION_SHAPE_TYPE: "discourse-relation", + getRelationColor: () => "grey", + }), +); + +import { migrateRelationTypesToDiscourseRelation } from "~/components/canvas/DiscourseRelationShape/discourseRelationMigrations"; + +const createArrowLikeShape = ({ + id, + type, + text = "Supports", +}: { + id: string; + type: string; + text?: string; +}) => ({ + typeName: "shape", + id, + type, + props: { + start: { x: 0, y: 0 }, + end: { x: 100, y: 0 }, + arrowheadEnd: "arrow", + bend: 0, + labelPosition: 0.5, + text, + }, +}); + +const createBinding = ({ + id, + type, + fromId, +}: { + id: string; + type: string; + fromId: string; +}) => ({ + typeName: "binding", + id, + type, + fromId, + toId: "shape:node", + meta: {}, + props: { terminal: "start" }, +}); + +describe("migrateRelationTypesToDiscourseRelation", () => { + it("moves known relation type ids to relationTypeId", () => { + const relationShape = createArrowLikeShape({ + id: "shape:relation", + type: "rel-uid", + }); + const relationBinding = createBinding({ + id: "binding:relation", + type: "rel-uid", + fromId: relationShape.id, + }); + const addReferencedShape = createArrowLikeShape({ + id: "shape:add-referenced", + type: "Add Source", + }); + const defaultArrowShape = createArrowLikeShape({ + id: "shape:arrow", + type: "arrow", + }); + const store = { + [relationShape.id]: relationShape, + [relationBinding.id]: relationBinding, + [addReferencedShape.id]: addReferencedShape, + [defaultArrowShape.id]: defaultArrowShape, + }; + + migrateRelationTypesToDiscourseRelation({ + oldStore: store, + allRelationIds: ["rel-uid"], + allAddReferencedNodeActions: ["Add Source"], + allNodeTypes: ["claim-node"], + }); + + expect(relationShape.type).toBe("discourse-relation"); + expect( + (relationShape.props as { relationTypeId?: string }).relationTypeId, + ).toBe("rel-uid"); + expect(relationBinding.type).toBe("discourse-relation"); + expect(addReferencedShape.type).toBe("Add Source"); + expect(defaultArrowShape.type).toBe("arrow"); + }); + + it("migrates unknown arrow-like relation shapes for deleted relation schemas", () => { + const relationShape = createArrowLikeShape({ + id: "shape:deleted-relation", + type: "deleted-rel-uid", + }); + const relationBinding = createBinding({ + id: "binding:deleted-relation", + type: "deleted-rel-uid", + fromId: relationShape.id, + }); + const store = { + [relationShape.id]: relationShape, + [relationBinding.id]: relationBinding, + }; + + migrateRelationTypesToDiscourseRelation({ + oldStore: store, + allRelationIds: [], + allAddReferencedNodeActions: [], + allNodeTypes: ["claim-node"], + }); + + expect(relationShape.type).toBe("discourse-relation"); + expect( + (relationShape.props as { relationTypeId?: string }).relationTypeId, + ).toBe("deleted-rel-uid"); + expect(relationBinding.type).toBe("discourse-relation"); + }); +}); diff --git a/apps/roam/src/utils/syncCanvasNodeTitlesOnLoad.ts b/apps/roam/src/utils/syncCanvasNodeTitlesOnLoad.ts index 2572f4a28..47aa64e68 100644 --- a/apps/roam/src/utils/syncCanvasNodeTitlesOnLoad.ts +++ b/apps/roam/src/utils/syncCanvasNodeTitlesOnLoad.ts @@ -4,6 +4,7 @@ import { getDiscourseNodeTypeId, type DiscourseNodeShape, } from "~/components/canvas/DiscourseNodeUtil"; +import { DISCOURSE_RELATION_SHAPE_TYPE } from "~/components/canvas/DiscourseRelationShape/DiscourseRelationUtil"; /** * Query Roam for current :node/title or :block/string for each uid. @@ -35,8 +36,12 @@ const deleteNodeShapeAndRelations = ( shape: DiscourseNodeShape, relationIds: Set, ): void => { - const bindingsToThisShape = Array.from(relationIds).flatMap((typeId) => - editor.getBindingsToShape(shape.id, typeId), + const relationBindingTypes = new Set([ + DISCOURSE_RELATION_SHAPE_TYPE, + ...relationIds, + ]); + const bindingsToThisShape = Array.from(relationBindingTypes).flatMap( + (typeId) => editor.getBindingsToShape(shape.id, typeId), ); const relationShapeIdsAndType = bindingsToThisShape.map((b) => ({ id: b.fromId, From 1c143e9756370e83456bed20392d0184bfee94d0 Mon Sep 17 00:00:00 2001 From: Michael Gartner Date: Mon, 6 Jul 2026 00:06:40 -0600 Subject: [PATCH 2/2] Address relation review feedback --- .../components/canvas/DiscourseNodeUtil.tsx | 38 +++++++++++++------ .../canvas/DiscourseRelationShape/helpers.tsx | 2 +- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/apps/roam/src/components/canvas/DiscourseNodeUtil.tsx b/apps/roam/src/components/canvas/DiscourseNodeUtil.tsx index 5df17f571..b36cbef23 100644 --- a/apps/roam/src/components/canvas/DiscourseNodeUtil.tsx +++ b/apps/roam/src/components/canvas/DiscourseNodeUtil.tsx @@ -40,7 +40,9 @@ import { createTextJsxFromSpans } from "./DiscourseRelationShape/helpers"; import { loadImage } from "~/utils/loadImage"; import { DISCOURSE_RELATION_SHAPE_TYPE, + getDiscourseRelationTypeId, getRelationColor, + isDiscourseRelationShape, } from "./DiscourseRelationShape/DiscourseRelationUtil"; import { getPersonalSetting } from "~/components/settings/utils/accessors"; import { PERSONAL_KEYS } from "~/components/settings/utils/settingKeys"; @@ -281,20 +283,32 @@ export class DiscourseNodeUtil extends BaseBoxShapeUtil { ...discourseContextRelationIds, ]); const currentShapeRelations = Array.from(relationBindingTypes).flatMap( - (relationId) => { + (bindingType) => { const bindingsToThisShape = editor.getBindingsToShape( shape.id, - relationId, + bindingType, ); - return bindingsToThisShape.map((b) => { - const arrowId = b.fromId; + return bindingsToThisShape.flatMap((bindingToThisShape) => { + const arrowId = bindingToThisShape.fromId; + const arrow = editor.getShape(arrowId); + if (!arrow || !isDiscourseRelationShape(arrow)) return []; + const bindingsFromArrow = editor.getBindingsFromShape( arrowId, - relationId, + bindingType, + ); + const endBinding = bindingsFromArrow.find( + (bindingFromArrow) => bindingFromArrow.toId !== shape.id, ); - const endBinding = bindingsFromArrow.find((b) => b.toId !== shape.id); - if (!endBinding) return null; - return { startId: shape.id, endId: endBinding.toId }; + if (!endBinding) return []; + + return [ + { + startId: shape.id, + endId: endBinding.toId, + relationTypeId: getDiscourseRelationTypeId({ shape: arrow }), + }, + ]; }); }, ); @@ -312,13 +326,15 @@ export class DiscourseNodeUtil extends BaseBoxShapeUtil { }; }), ) - .filter(({ complement, nodeId }) => { + .filter(({ relationId, complement, nodeId }) => { const startId = complement ? nodesInCanvas[nodeId].id : shape.id; const endId = complement ? shape.id : nodesInCanvas[nodeId].id; const relationAlreadyExists = currentShapeRelations.some((r) => { + if (r.relationTypeId !== relationId) return false; + return complement - ? r?.startId === endId && r?.endId === startId - : r?.startId === startId && r?.endId === endId; + ? r.startId === endId && r.endId === startId + : r.startId === startId && r.endId === endId; }); return !relationAlreadyExists; }) diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx b/apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx index c7220b641..54754105b 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/helpers.tsx @@ -1650,7 +1650,7 @@ export function createOrUpdateArrowBinding( props: TLArrowBindingProps, shouldCreateRelation = false, ) { - const arrowId = typeof relation === "string" ? relation : relation.id; + const arrowId = relation.id; const targetId = typeof target === "string" ? target : target.id; const bindingType = getDiscourseRelationBindingType({ shape: relation });