Bindings onBeforeShapeIsolate? (#3871)

So we were kinda bending over backwards to capture the use case where we
update the arrow's terminal x,y coords when unbinding, copy-pasting, and
duplicating.

- At first we abused the `onBeforeShapeDelete` callbacks, but that was
footgunny.
- Then we created a `onBeforeUnbind` callback, which was less footgunny
but still subtly footgunny.

This PR proposes reverting the `onBeforeUnbind` stuff, taking us back to
having `onBeforeShapeDelete` stuff. But at the same time it adds
`onBeforeShapeIsolate` callbacks which are triggered at the following
times:

- When you delete the other shape in a bound shape pair
- When you copy/paste or duplicate one shape in a bound shape pair but
not the other one
- When you opt-in while deleting bindings e.g. `deleteBindings([...],
{isolateShapes: true})`

This PR also fixes the bound arrow drag interaction. We can probably
extract that out to a separate PR if needed.


![Kapture 2024-06-04 at 12 42
40](https://github.com/tldraw/tldraw/assets/1242537/95b51e14-1119-4dad-91e4-8b19fdb5e862)

### Change Type

<!-- ❗ Please select a 'Scope' label ❗️ -->

- [x] `sdk` — Changes the tldraw SDK
- [ ] `dotcom` — Changes the tldraw.com web app
- [ ] `docs` — Changes to the documentation, examples, or templates.
- [ ] `vs code` — Changes to the vscode plugin
- [ ] `internal` — Does not affect user-facing stuff

<!-- ❗ Please select a 'Type' label ❗️ -->

- [x] `bugfix` — Bug fix
- [ ] `feature` — New feature
- [x] `improvement` — Improving existing features
- [ ] `chore` — Updating dependencies, other boring stuff
- [ ] `galaxy brain` — Architectural changes
- [ ] `tests` — Changes to any test code
- [ ] `tools` — Changes to infrastructure, CI, internal scripts,
debugging tools, etc.
- [ ] `dunno` — I don't know


### Test Plan

1. Add a step-by-step description of how to test your PR here.
2.

- [ ] Unit Tests
- [ ] End to end tests

### Release Notes

- Add a brief release note for your PR here.

apps/examples/src/examples/pin-bindings/PinExample.tsx

@@ -1,7 +1,6 @@
 import {
 	BindingOnShapeChangeOptions,
-	BindingOnUnbindOptions,
-	BindingUnbindReason,
+	BindingOnShapeDeleteOptions,
 	BindingUtil,
 	Box,
 	DefaultFillStyle,
@@ -257,12 +256,10 @@ class PinBindingUtil extends BindingUtil<PinBinding> {
 	}
 
 	// when the thing we're stuck to is deleted, delete the pin too
-	override onBeforeUnbind({ binding, reason }: BindingOnUnbindOptions<PinBinding>): void {
-		if (reason === BindingUnbindReason.DeletingToShape) {
+	override onBeforeDeleteToShape({ binding }: BindingOnShapeDeleteOptions<PinBinding>): void {
 		this.editor.deleteShape(binding.fromId)
 	}
 }
-}
 
 class PinTool extends StateNode {
 	static override id = 'pin'

apps/examples/src/examples/sticker-bindings/StickerExample.tsx

@@ -1,7 +1,6 @@
 import {
 	BindingOnShapeChangeOptions,
-	BindingOnUnbindOptions,
-	BindingUnbindReason,
+	BindingOnShapeDeleteOptions,
 	BindingUtil,
 	Box,
 	DefaultToolbar,
@@ -158,12 +157,10 @@ class StickerBindingUtil extends BindingUtil<StickerBinding> {
 	}
 
 	// when the thing we're stuck to is deleted, delete the sticker too
-	override onBeforeUnbind({ binding, reason }: BindingOnUnbindOptions<StickerBinding>): void {
-		if (reason === BindingUnbindReason.DeletingToShape) {
+	override onBeforeDeleteToShape({ binding }: BindingOnShapeDeleteOptions<StickerBinding>): void {
 		this.editor.deleteShape(binding.fromId)
 	}
 }
-}
 
 class StickerTool extends StateNode {
 	static override id = 'sticker'

packages/editor/api-report.md

@@ -189,6 +189,12 @@ export interface BindingOnCreateOptions<Binding extends TLUnknownBinding> {
     binding: Binding;
 }
 
+// @public (undocumented)
+export interface BindingOnDeleteOptions<Binding extends TLUnknownBinding> {
+    // (undocumented)
+    binding: Binding;
+}
+
 // @public (undocumented)
 export interface BindingOnShapeChangeOptions<Binding extends TLUnknownBinding> {
     // (undocumented)
@@ -200,21 +206,19 @@ export interface BindingOnShapeChangeOptions<Binding extends TLUnknownBinding> {
 }
 
 // @public (undocumented)
-export interface BindingOnUnbindOptions<Binding extends TLUnknownBinding> {
+export interface BindingOnShapeDeleteOptions<Binding extends TLUnknownBinding> {
     // (undocumented)
     binding: Binding;
     // (undocumented)
-    reason: BindingUnbindReason;
+    shape: TLShape;
 }
 
 // @public (undocumented)
-export enum BindingUnbindReason {
+export interface BindingOnShapeIsolateOptions<Binding extends TLUnknownBinding> {
     // (undocumented)
-    DeletingBinding = "deleting_binding",
+    binding: Binding;
     // (undocumented)
-    DeletingFromShape = "deleting_from_shape",
-    // (undocumented)
-    DeletingToShape = "deleting_to_shape"
+    shape: TLShape;
 }
 
 // @public (undocumented)
@@ -234,13 +238,21 @@ export abstract class BindingUtil<Binding extends TLUnknownBinding = TLUnknownBi
     // (undocumented)
     onAfterCreate?(options: BindingOnCreateOptions<Binding>): void;
     // (undocumented)
-    onAfterUnbind?(options: BindingOnUnbindOptions<Binding>): void;
+    onAfterDelete?(options: BindingOnDeleteOptions<Binding>): void;
     // (undocumented)
     onBeforeChange?(options: BindingOnChangeOptions<Binding>): Binding | void;
     // (undocumented)
     onBeforeCreate?(options: BindingOnCreateOptions<Binding>): Binding | void;
     // (undocumented)
-    onBeforeUnbind?(options: BindingOnUnbindOptions<Binding>): void;
+    onBeforeDelete?(options: BindingOnDeleteOptions<Binding>): Binding | void;
+    // (undocumented)
+    onBeforeDeleteFromShape?(options: BindingOnShapeDeleteOptions<Binding>): void;
+    // (undocumented)
+    onBeforeDeleteToShape?(options: BindingOnShapeDeleteOptions<Binding>): void;
+    // (undocumented)
+    onBeforeIsolateFromShape?(options: BindingOnShapeIsolateOptions<Binding>): void;
+    // (undocumented)
+    onBeforeIsolateToShape?(options: BindingOnShapeIsolateOptions<Binding>): void;
     // (undocumented)
     onOperationComplete?(): void;
     // (undocumented)
@@ -604,7 +616,7 @@ export const DefaultSvgDefs: () => null;
 export const defaultTldrawOptions: {
     readonly adjacentShapeMargin: 10;
     readonly animationMediumMs: 320;
-    readonly cameraMovingTimoutMs: 64;
+    readonly cameraMovingTimeoutMs: 64;
     readonly cameraSlideFriction: 0.09;
     readonly coarseDragDistanceSquared: 36;
     readonly coarseHandleRadius: 20;
@@ -799,9 +811,11 @@ export class Editor extends EventEmitter<TLEventMap> {
     createShapes<T extends TLUnknownShape>(shapes: OptionalKeys<TLShapePartial<T>, 'id'>[]): this;
     deleteAssets(assets: TLAsset[] | TLAssetId[]): this;
     // (undocumented)
-    deleteBinding(binding: TLBinding | TLBindingId): this;
+    deleteBinding(binding: TLBinding | TLBindingId, opts?: Parameters<this['deleteBindings']>[1]): this;
     // (undocumented)
-    deleteBindings(bindings: (TLBinding | TLBindingId)[]): this;
+    deleteBindings(bindings: (TLBinding | TLBindingId)[], { isolateShapes }?: {
+        isolateShapes?: boolean | undefined;
+    }): this;
     deleteOpenMenu(id: string): this;
     deletePage(page: TLPage | TLPageId): this;
     deleteShape(id: TLShapeId): this;
@@ -2344,7 +2358,7 @@ export interface TldrawOptions {
     // (undocumented)
     readonly animationMediumMs: number;
     // (undocumented)
-    readonly cameraMovingTimoutMs: number;
+    readonly cameraMovingTimeoutMs: number;
     // (undocumented)
     readonly cameraSlideFriction: number;
     // (undocumented)

packages/editor/src/index.ts

@@ -110,12 +110,13 @@ export { coreShapes, type TLAnyShapeUtilConstructor } from './lib/config/default
 export { DEFAULT_ANIMATION_OPTIONS, DEFAULT_CAMERA_OPTIONS, SIDES } from './lib/constants'
 export { Editor, type TLEditorOptions, type TLResizeShapeOptions } from './lib/editor/Editor'
 export {
-	BindingUnbindReason,
 	BindingUtil,
 	type BindingOnChangeOptions,
 	type BindingOnCreateOptions,
+	type BindingOnDeleteOptions,
 	type BindingOnShapeChangeOptions,
-	type BindingOnUnbindOptions,
+	type BindingOnShapeDeleteOptions,
+	type BindingOnShapeIsolateOptions,
 	type TLBindingUtilConstructor,
 } from './lib/editor/bindings/BindingUtil'
 export { HistoryManager } from './lib/editor/managers/HistoryManager'

packages/editor/src/lib/editor/Editor.ts

@@ -110,8 +110,7 @@ import { getReorderingShapesChanges } from '../utils/reorderShapes'
 import { applyRotationToSnapshotShapes, getRotationSnapshot } from '../utils/rotation'
 import { uniqueId } from '../utils/uniqueId'
 import {
-	BindingOnUnbindOptions,
-	BindingUnbindReason,
+	BindingOnDeleteOptions,
 	BindingUtil,
 	TLBindingUtilConstructor,
 } from './bindings/BindingUtil'
@@ -359,7 +358,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 
 		this.sideEffects = this.store.sideEffects
 
-		let deletedBindings = new Map<TLBindingId, BindingOnUnbindOptions<any>>()
+		let deletedBindings = new Map<TLBindingId, BindingOnDeleteOptions<any>>()
 		const deletedShapeIds = new Set<TLShapeId>()
 		const invalidParents = new Set<TLShapeId>()
 		let invalidBindingTypes = new Set<string>()
@@ -395,7 +394,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 					const t = deletedBindings
 					deletedBindings = new Map()
 					for (const opts of t.values()) {
-						this.getBindingUtil(opts.binding).onAfterUnbind?.(opts)
+						this.getBindingUtil(opts.binding).onAfterDelete?.(opts)
 					}
 				}
 
@@ -491,8 +490,19 @@ export class Editor extends EventEmitter<TLEventMap> {
 						for (const binding of this.getBindingsInvolvingShape(shape)) {
 							invalidBindingTypes.add(binding.type)
 							deleteBindingIds.push(binding.id)
+							const util = this.getBindingUtil(binding)
+							if (binding.fromId === shape.id) {
+								util.onBeforeIsolateToShape?.({ binding, shape })
+								util.onBeforeDeleteFromShape?.({ binding, shape })
+							} else {
+								util.onBeforeIsolateFromShape?.({ binding, shape })
+								util.onBeforeDeleteToShape?.({ binding, shape })
 							}
+						}
+
+						if (deleteBindingIds.length) {
 							this.deleteBindings(deleteBindingIds)
+						}
 
 						const deletedIds = new Set([shape.id])
 						const updates = compact(
@@ -529,25 +539,10 @@ export class Editor extends EventEmitter<TLEventMap> {
 						this.getBindingUtil(bindingAfter).onAfterChange?.({ bindingBefore, bindingAfter })
 					},
 					beforeDelete: (binding) => {
-						const util = this.getBindingUtil(binding)
-						// No need to track this binding if it's util doesn't care about the unbind operation
-						if (!util.onBeforeUnbind && !util.onAfterUnbind) return
-						// We only want to call this once per binding and it might be possible that the onBeforeUnbind
-						// callback will trigger a nested delete operation on the same binding so let's bail out if
-						// that is happening
-						if (deletedBindings.has(binding.id)) return
-						const opts: BindingOnUnbindOptions<any> = {
-							binding,
-							reason: deletedShapeIds.has(binding.fromId)
-								? BindingUnbindReason.DeletingFromShape
-								: deletedShapeIds.has(binding.toId)
-									? BindingUnbindReason.DeletingToShape
-									: BindingUnbindReason.DeletingBinding,
-						}
-						deletedBindings.set(binding.id, opts)
-						this.getBindingUtil(binding).onBeforeUnbind?.(opts)
+						this.getBindingUtil(binding).onBeforeDelete?.({ binding })
 					},
 					afterDelete: (binding) => {
+						this.getBindingUtil(binding).onAfterDelete?.({ binding })
 						invalidBindingTypes.add(binding.type)
 					},
 				},
@@ -3461,7 +3456,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 	}
 	private _tickCameraState = () => {
 		// always reset the timeout
-		this._cameraStateTimeoutRemaining = this.options.cameraMovingTimoutMs
+		this._cameraStateTimeoutRemaining = this.options.cameraMovingTimeoutMs
 		// If the state is idle, then start the tick
 		if (this._cameraState.__unsafe__getWithoutCapture() !== 'idle') return
 		this._cameraState.set('moving')
@@ -5247,13 +5242,26 @@ export class Editor extends EventEmitter<TLEventMap> {
 		return this.updateBindings([partial])
 	}
 
-	deleteBindings(bindings: (TLBinding | TLBindingId)[]) {
+	deleteBindings(bindings: (TLBinding | TLBindingId)[], { isolateShapes = false } = {}) {
 		const ids = bindings.map((binding) => (typeof binding === 'string' ? binding : binding.id))
+		if (isolateShapes) {
+			this.store.atomic(() => {
+				for (const id of ids) {
+					const binding = this.getBinding(id)
+					if (!binding) continue
+					const util = this.getBindingUtil(binding)
+					util.onBeforeIsolateFromShape?.({ binding, shape: this.getShape(binding.fromId)! })
+					util.onBeforeIsolateToShape?.({ binding, shape: this.getShape(binding.toId)! })
+					this.store.remove([id])
+				}
+			})
+		} else {
 			this.store.remove(ids)
+		}
 		return this
 	}
-	deleteBinding(binding: TLBinding | TLBindingId) {
-		return this.deleteBindings([binding])
+	deleteBinding(binding: TLBinding | TLBindingId, opts?: Parameters<this['deleteBindings']>[1]) {
+		return this.deleteBindings([binding], opts)
 	}
 	canBindShapes({
 		fromShape,
@@ -5406,7 +5414,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 				shapeIds.set(shapeId, createShapeId())
 			}
 
-			const { shapesToCreate, bindingsToCreate } = withoutBindingsToUnrelatedShapes(
+			const { shapesToCreate, bindingsToCreate } = withIsolatedShapes(
 				this,
 				shapeIdSet,
 				(bindingIdsToMaintain) => {
@@ -7664,7 +7672,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 
 		const shapeIds = this.getShapeAndDescendantIds(ids)
 
-		return withoutBindingsToUnrelatedShapes(this, shapeIds, (bindingIdsToKeep) => {
+		return withIsolatedShapes(this, shapeIds, (bindingIdsToKeep) => {
 			const bindings: TLBinding[] = []
 			for (const id of bindingIdsToKeep) {
 				const binding = this.getBinding(id)
@@ -9005,11 +9013,15 @@ function pushShapeWithDescendants(editor: Editor, id: TLShapeId, result: TLShape
  *
  * The callback is given the set of bindings that should be maintained.
  */
-function withoutBindingsToUnrelatedShapes<T>(
+function withIsolatedShapes<T>(
 	editor: Editor,
 	shapeIds: Set<TLShapeId>,
 	callback: (bindingsWithBoth: Set<TLBindingId>) => T
 ): T {
+	let result!: Result<T, unknown>
+
+	editor.history.ignore(() => {
+		const changes = editor.store.extractingChanges(() => {
 			const bindingsWithBoth = new Set<TLBindingId>()
 			const bindingsToRemove = new Set<TLBindingId>()
 
@@ -9030,11 +9042,7 @@ function withoutBindingsToUnrelatedShapes<T>(
 				}
 			}
 
-	let result!: Result<T, unknown>
-
-	editor.history.ignore(() => {
-		const changes = editor.store.extractingChanges(() => {
-			editor.deleteBindings([...bindingsToRemove])
+			editor.deleteBindings([...bindingsToRemove], { isolateShapes: true })
 
 			try {
 				result = Result.ok(callback(bindingsWithBoth))

packages/editor/src/lib/editor/bindings/BindingUtil.ts

@@ -17,25 +17,17 @@ export interface BindingOnCreateOptions<Binding extends TLUnknownBinding> {
 	binding: Binding
 }
 
-/** @public */
-export enum BindingUnbindReason {
-	DeletingFromShape = 'deleting_from_shape',
-	DeletingToShape = 'deleting_to_shape',
-	DeletingBinding = 'deleting_binding',
-}
-
-/** @public */
-export interface BindingOnUnbindOptions<Binding extends TLUnknownBinding> {
-	binding: Binding
-	reason: BindingUnbindReason
-}
-
 /** @public */
 export interface BindingOnChangeOptions<Binding extends TLUnknownBinding> {
 	bindingBefore: Binding
 	bindingAfter: Binding
 }
 
+/** @public */
+export interface BindingOnDeleteOptions<Binding extends TLUnknownBinding> {
+	binding: Binding
+}
+
 /** @public */
 export interface BindingOnShapeChangeOptions<Binding extends TLUnknownBinding> {
 	binding: Binding
@@ -43,6 +35,18 @@ export interface BindingOnShapeChangeOptions<Binding extends TLUnknownBinding> {
 	shapeAfter: TLShape
 }
 
+/** @public */
+export interface BindingOnShapeIsolateOptions<Binding extends TLUnknownBinding> {
+	binding: Binding
+	shape: TLShape
+}
+
+/** @public */
+export interface BindingOnShapeDeleteOptions<Binding extends TLUnknownBinding> {
+	binding: Binding
+	shape: TLShape
+}
+
 /** @public */
 export abstract class BindingUtil<Binding extends TLUnknownBinding = TLUnknownBinding> {
 	constructor(public editor: Editor) {}
@@ -65,15 +69,20 @@ export abstract class BindingUtil<Binding extends TLUnknownBinding = TLUnknownBi
 
 	onOperationComplete?(): void
 
-	onBeforeUnbind?(options: BindingOnUnbindOptions<Binding>): void
-	onAfterUnbind?(options: BindingOnUnbindOptions<Binding>): void
-
 	// self lifecycle hooks
 	onBeforeCreate?(options: BindingOnCreateOptions<Binding>): Binding | void
 	onAfterCreate?(options: BindingOnCreateOptions<Binding>): void
 	onBeforeChange?(options: BindingOnChangeOptions<Binding>): Binding | void
 	onAfterChange?(options: BindingOnChangeOptions<Binding>): void
+	onBeforeDelete?(options: BindingOnDeleteOptions<Binding>): Binding | void
+	onAfterDelete?(options: BindingOnDeleteOptions<Binding>): void
 
 	onAfterChangeFromShape?(options: BindingOnShapeChangeOptions<Binding>): void
 	onAfterChangeToShape?(options: BindingOnShapeChangeOptions<Binding>): void
+
+	onBeforeIsolateFromShape?(options: BindingOnShapeIsolateOptions<Binding>): void
+	onBeforeIsolateToShape?(options: BindingOnShapeIsolateOptions<Binding>): void
+
+	onBeforeDeleteFromShape?(options: BindingOnShapeDeleteOptions<Binding>): void
+	onBeforeDeleteToShape?(options: BindingOnShapeDeleteOptions<Binding>): void
 }

packages/editor/src/lib/editor/derivations/bindingsIndex.ts

@@ -50,14 +50,14 @@ export const bindingsIndex = (editor: Editor): Computed<TLBindingsIndex> => {
 
 		function removingBinding(binding: TLBinding) {
 			nextValue ??= new Map(lastValue)
-			const prevFrom = lastValue.get(binding.fromId)
+			const prevFrom = nextValue.get(binding.fromId)
 			const nextFrom = prevFrom?.filter((b) => b.id !== binding.id)
 			if (!nextFrom?.length) {
 				nextValue.delete(binding.fromId)
 			} else {
 				nextValue.set(binding.fromId, nextFrom)
 			}
-			const prevTo = lastValue.get(binding.toId)
+			const prevTo = nextValue.get(binding.toId)
 			const nextTo = prevTo?.filter((b) => b.id !== binding.id)
 			if (!nextTo?.length) {
 				nextValue.delete(binding.toId)

packages/editor/src/lib/options.ts

@@ -34,7 +34,7 @@ export interface TldrawOptions {
 	readonly collaboratorInactiveTimeoutMs: number
 	readonly collaboratorIdleTimeoutMs: number
 	readonly collaboratorCheckIntervalMs: number
-	readonly cameraMovingTimoutMs: number
+	readonly cameraMovingTimeoutMs: number
 	readonly hitTestMargin: number
 	readonly edgeScrollSpeed: number
 	readonly edgeScrollDistance: number
@@ -67,7 +67,7 @@ export const defaultTldrawOptions = {
 	collaboratorInactiveTimeoutMs: 60000,
 	collaboratorIdleTimeoutMs: 3000,
 	collaboratorCheckIntervalMs: 1200,
-	cameraMovingTimoutMs: 64,
+	cameraMovingTimeoutMs: 64,
 	hitTestMargin: 8,
 	edgeScrollSpeed: 20,
 	edgeScrollDistance: 8,

packages/tldraw/src/lib/bindings/arrow/ArrowBindingUtil.ts

@@ -2,8 +2,7 @@ import {
 	BindingOnChangeOptions,
 	BindingOnCreateOptions,
 	BindingOnShapeChangeOptions,
-	BindingOnUnbindOptions,
-	BindingUnbindReason,
+	BindingOnShapeIsolateOptions,
 	BindingUtil,
 	Editor,
 	IndexKey,
@@ -15,6 +14,7 @@ import {
 	TLShapeId,
 	TLShapePartial,
 	Vec,
+	approximately,
 	arrowBindingMigrations,
 	arrowBindingProps,
 	assert,
@@ -60,13 +60,17 @@ export class ArrowBindingUtil extends BindingUtil<TLArrowBinding> {
 		reparentArrow(this.editor, binding.fromId)
 	}
 
-	// when the shape the arrow is pointing to is deleted
-	override onBeforeUnbind({ binding, reason }: BindingOnUnbindOptions<TLArrowBinding>): void {
-		// don't need to do anything if the arrow is being deleted
-		if (reason === BindingUnbindReason.DeletingFromShape) return
+	// when the arrow is isolated we need to update it's x,y positions
+	override onBeforeIsolateFromShape({
+		binding,
+	}: BindingOnShapeIsolateOptions<TLArrowBinding>): void {
 		const arrow = this.editor.getShape<TLArrowShape>(binding.fromId)
 		if (!arrow) return
-		unbindArrowTerminal(this.editor, arrow, binding.props.terminal)
+		updateArrowTerminal({
+			editor: this.editor,
+			arrow,
+			terminal: binding.props.terminal,
+		})
 	}
 }
 
@@ -171,7 +175,7 @@ function arrowDidUpdate(editor: Editor, arrow: TLArrowShape) {
 		const isShapeInSamePageAsArrow =
 			editor.getAncestorPageId(arrow) === editor.getAncestorPageId(boundShape)
 		if (!boundShape || !isShapeInSamePageAsArrow) {
-			unbindArrowTerminal(editor, arrow, handle)
+			updateArrowTerminal({ editor, arrow, terminal: handle, unbind: true })
 		}
 	}
 
@@ -179,17 +183,34 @@ function arrowDidUpdate(editor: Editor, arrow: TLArrowShape) {
 	reparentArrow(editor, arrow.id)
 }
 
-function unbindArrowTerminal(editor: Editor, arrow: TLArrowShape, terminal: 'start' | 'end') {
-	const info = getArrowInfo(editor, arrow)!
+/** @internal */
+export function updateArrowTerminal({
+	editor,
+	arrow,
+	terminal,
+	unbind = false,
+	useHandle = false,
+}: {
+	editor: Editor
+	arrow: TLArrowShape
+	terminal: 'start' | 'end'
+	unbind?: boolean
+	useHandle?: boolean
+}) {
+	const info = getArrowInfo(editor, arrow)
 	if (!info) {
 		throw new Error('expected arrow info')
 	}
 
+	const startPoint = useHandle ? info.start.handle : info.start.point
+	const endPoint = useHandle ? info.end.handle : info.end.point
+	const point = terminal === 'start' ? startPoint : endPoint
+
 	const update = {
 		id: arrow.id,
 		type: 'arrow',
 		props: {
-			[terminal]: { x: info[terminal].point.x, y: info[terminal].point.y },
+			[terminal]: { x: point.x, y: point.y },
 			bend: arrow.props.bend,
 		},
 	} satisfies TLShapePartial<TLArrowShape>
@@ -197,8 +218,8 @@ function unbindArrowTerminal(editor: Editor, arrow: TLArrowShape, terminal: 'sta
 	// fix up the bend:
 	if (!info.isStraight) {
 		// find the new start/end points of the resulting arrow
-		const newStart = terminal === 'start' ? info.start.point : info.start.handle
-		const newEnd = terminal === 'end' ? info.end.point : info.end.handle
+		const newStart = terminal === 'start' ? startPoint : info.start.handle
+		const newEnd = terminal === 'end' ? endPoint : info.end.handle
 		const newMidPoint = Vec.Med(newStart, newEnd)
 
 		// intersect a line segment perpendicular to the new arrow with the old arrow arc to
@@ -218,9 +239,14 @@ function unbindArrowTerminal(editor: Editor, arrow: TLArrowShape, terminal: 'sta
 
 		assert(intersections?.length === 1)
 		const bend = Vec.Dist(newMidPoint, intersections[0]) * Math.sign(arrow.props.bend)
+		// use `approximately` to avoid endless update loops
+		if (!approximately(bend, update.props.bend)) {
 			update.props.bend = bend
 		}
+	}
 
 	editor.updateShape(update)
+	if (unbind) {
 		removeArrowBinding(editor, arrow, terminal)
 	}
+}

packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx

@@ -34,6 +34,7 @@ import {
 	useIsEditing,
 } from '@tldraw/editor'
 import React from 'react'
+import { updateArrowTerminal } from '../../bindings/arrow/ArrowBindingUtil'
 import { ShapeFill, useDefaultColorTheme } from '../shared/ShapeFill'
 import { SvgTextLabel } from '../shared/SvgTextLabel'
 import { ARROW_LABEL_FONT_SIZES, STROKE_SIZES } from '../shared/default-shape-constants'
@@ -355,6 +356,25 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
 			}),
 		})
 
+		// update arrow terminal bindings eagerly to make sure the arrows unbind nicely when translating
+		if (bindings.start) {
+			updateArrowTerminal({
+				editor: this.editor,
+				arrow: shape,
+				terminal: 'start',
+				useHandle: true,
+			})
+			shape = this.editor.getShape(shape.id) as TLArrowShape
+		}
+		if (bindings.end) {
+			updateArrowTerminal({
+				editor: this.editor,
+				arrow: shape,
+				terminal: 'end',
+				useHandle: true,
+			})
+		}
+
 		for (const handleName of [ARROW_HANDLES.START, ARROW_HANDLES.END] as const) {
 			const binding = bindings[handleName]
 			if (!binding) continue
@@ -570,7 +590,7 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
 				<SVGContainer id={shape.id} style={{ minWidth: 50, minHeight: 50 }}>
 					<ArrowSvg
 						shape={shape}
-						shouldDisplayHandles={shouldDisplayHandles && onlySelectedShape === shape}
+						shouldDisplayHandles={shouldDisplayHandles && onlySelectedShape?.id === shape.id}
 					/>
 				</SVGContainer>
 				{showArrowLabel && (

packages/tldraw/src/test/bindings.test.tsx

@@ -1,210 +0,0 @@
-import {
-	BindingOnUnbindOptions,
-	BindingUnbindReason,
-	BindingUtil,
-	TLShapeId,
-	TLUnknownBinding,
-	createBindingId,
-	createShapeId,
-} from '@tldraw/editor'
-import { TestEditor } from './TestEditor'
-import { TL } from './test-jsx'
-
-let editor: TestEditor
-
-const ids = {
-	box1: createShapeId('box1'),
-	box2: createShapeId('box2'),
-	box3: createShapeId('box3'),
-	box4: createShapeId('box4'),
-}
-
-const mockOnBeforeUnbind = jest.fn() as jest.Mock<void, [BindingOnUnbindOptions<TLUnknownBinding>]>
-const mockOnAfterUnbind = jest.fn() as jest.Mock<void, [BindingOnUnbindOptions<TLUnknownBinding>]>
-
-class TestBindingUtil extends BindingUtil {
-	static override type = 'test'
-
-	static override props = {}
-
-	override getDefaultProps(): object {
-		return {}
-	}
-
-	override onBeforeUnbind(options: BindingOnUnbindOptions<TLUnknownBinding>): void {
-		mockOnBeforeUnbind(options)
-	}
-
-	override onAfterUnbind(options: BindingOnUnbindOptions<TLUnknownBinding>): void {
-		mockOnAfterUnbind(options)
-	}
-}
-
-beforeEach(() => {
-	editor = new TestEditor({ bindingUtils: [TestBindingUtil] })
-
-	editor.createShapesFromJsx([
-		<TL.geo id={ids.box1} x={0} y={0} />,
-		<TL.geo id={ids.box2} x={0} y={0} />,
-		<TL.geo id={ids.box3} x={0} y={0} />,
-		<TL.geo id={ids.box4} x={0} y={0} />,
-	])
-
-	mockOnBeforeUnbind.mockClear()
-	mockOnAfterUnbind.mockClear()
-})
-
-function bindShapes(fromId: TLShapeId, toId: TLShapeId) {
-	const bindingId = createBindingId()
-	editor.createBinding({
-		id: bindingId,
-		type: 'test',
-		fromId,
-		toId,
-	})
-	return bindingId
-}
-
-test('deleting the from shape causes the reason to be "deleting_from_shape"', () => {
-	bindShapes(ids.box1, ids.box2)
-	editor.deleteShape(ids.box1)
-	expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingFromShape })
-	)
-	expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnAfterUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingFromShape })
-	)
-})
-
-test('deleting the to shape causes the reason to be "deleting_to_shape"', () => {
-	bindShapes(ids.box1, ids.box2)
-	editor.deleteShape(ids.box2)
-	expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingToShape })
-	)
-	expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnAfterUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingToShape })
-	)
-})
-
-test('deleting the binding itself causes the reason to be "deleting_binding"', () => {
-	const bindingId = bindShapes(ids.box1, ids.box2)
-	editor.deleteBinding(bindingId)
-
-	expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
-	)
-	expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnAfterUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
-	)
-})
-
-test('copying both bound shapes does not trigger the unbind operation', () => {
-	bindShapes(ids.box1, ids.box2)
-	editor.select(ids.box1, ids.box2)
-	editor.copy()
-	expect(mockOnBeforeUnbind).not.toHaveBeenCalled()
-	expect(mockOnAfterUnbind).not.toHaveBeenCalled()
-})
-
-test('copying the from shape on its own does trigger the unbind operation', () => {
-	bindShapes(ids.box1, ids.box2)
-	editor.select(ids.box1)
-	editor.copy()
-	expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
-	)
-	expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnAfterUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
-	)
-})
-
-test('copying the to shape on its own does trigger the unbind operation', () => {
-	bindShapes(ids.box1, ids.box2)
-	editor.select(ids.box2)
-	editor.copy()
-	expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
-	)
-	expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
-	expect(mockOnAfterUnbind).toHaveBeenCalledWith(
-		expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
-	)
-})
-
-test('cascading deletes in afterUnbind are handled correctly', () => {
-	mockOnAfterUnbind.mockImplementation((options) => {
-		if (options.reason === BindingUnbindReason.DeletingFromShape) {
-			editor.deleteShape(options.binding.toId)
-		}
-	})
-
-	bindShapes(ids.box1, ids.box2)
-	bindShapes(ids.box2, ids.box3)
-	bindShapes(ids.box3, ids.box4)
-
-	editor.deleteShape(ids.box1)
-
-	expect(editor.getShape(ids.box1)).toBeUndefined()
-	expect(editor.getShape(ids.box2)).toBeUndefined()
-	expect(editor.getShape(ids.box3)).toBeUndefined()
-	expect(editor.getShape(ids.box4)).toBeUndefined()
-
-	expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(3)
-	expect(mockOnAfterUnbind).toHaveBeenCalledTimes(3)
-})
-
-test('cascading deletes in beforeUnbind are handled correctly', () => {
-	mockOnBeforeUnbind.mockImplementation((options) => {
-		if (options.reason === BindingUnbindReason.DeletingFromShape) {
-			editor.deleteShape(options.binding.toId)
-		}
-	})
-
-	bindShapes(ids.box1, ids.box2)
-	bindShapes(ids.box2, ids.box3)
-	bindShapes(ids.box3, ids.box4)
-
-	editor.deleteShape(ids.box1)
-
-	expect(editor.getShape(ids.box1)).toBeUndefined()
-	expect(editor.getShape(ids.box2)).toBeUndefined()
-	expect(editor.getShape(ids.box3)).toBeUndefined()
-	expect(editor.getShape(ids.box4)).toBeUndefined()
-
-	expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(3)
-	expect(mockOnAfterUnbind).toHaveBeenCalledTimes(3)
-})
-
-test('beforeUnbind is called before the from shape is deleted or the binding is deleted', () => {
-	mockOnBeforeUnbind.mockImplementationOnce(() => {
-		expect(editor.getShape(ids.box1)).toBeDefined()
-		expect(editor.getShape(ids.box2)).toBeDefined()
-		expect(editor.getBindingsFromShape(ids.box1, 'test')).toHaveLength(1)
-	})
-	bindShapes(ids.box1, ids.box2)
-	editor.deleteShape(ids.box1)
-
-	expect.assertions(3)
-})
-
-test('beforeUnbind is called before the to shape is deleted or the binding is deleted', () => {
-	mockOnBeforeUnbind.mockImplementationOnce(() => {
-		expect(editor.getShape(ids.box1)).toBeDefined()
-		expect(editor.getShape(ids.box2)).toBeDefined()
-		expect(editor.getBindingsToShape(ids.box2, 'test')).toHaveLength(1)
-	})
-	bindShapes(ids.box1, ids.box2)
-	editor.deleteShape(ids.box2)
-
-	expect.assertions(3)
-})