From 48fa9018f42f5193a56d6eb3c04a03f621200082 Mon Sep 17 00:00:00 2001 From: David Sheldrick Date: Thu, 16 May 2024 14:48:36 +0100 Subject: [PATCH] [bindings] beforeUnbind/afterUnbind to replace beforeDelete/afterDelete (#3761) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this PR the interface for doing cleanup when shapes/bindings were deleted was quite footgunny and inexpressive. We were abusing the shape beforeDelete callbacks to implement copy+paste, which doesn't work in situations where cascading deletes are required. This caused bugs in both our pin and sticker examples, where copy+paste was broken. I noticed the same bug in my experiment with text labels, and I think the fact that it took us a while to notice these bugs indicates other users are gonna fall prey to the same bugs unless we help them out. One suggestion to fix this was to add `onAfterDelete(From|To)Shape` callbacks. The cascading deletes could happen in those, while keeping the 'commit changes' kinds of updates in the `before` callbacks and theoretically that would fix the issues with copy+paste. However, expecting people to figure this out on their own is asking a heckuva lot IMO, and it's a heavy bit of nuance to try to convey in the docs. It's hard enough to convey it here. Plus I could imagine for some users it might easily even leave the store in an inconsistent state to allow a bound shape to exist for any length of time after the shape it was bound to was already deleted. It also just makes an already large and muddy API surface area even larger and muddier and if that can be avoided let's avoid it. This PR clears things up by making it so that there's only one callback for when a binding is removed. The callback is given a `reason` for why it is being called The `reason` is one of the following: - The 'from' is being deleted - The 'to' shape is being deleted - The binding is being deleted on it's own. Technically a binding might end up being deleted when both the `from` and `to` shapes are being deleted, but it's very hard to know for certain when that is happening, so I decided to just ignore it for now. I think it would only matter for perf reasons, to avoid doing useless work. So this PR replaces the `onBeforeDelete`, `onAfterDelete`, `onBeforeFromShapeDelete` and `onBeforeToShapeDelete` (and the prospective `onAfterFromShapeDelete` and `onAfterToShapeDelete`) with just two callbacks: - `onBeforeUnbind({binding, reason})` - called before any shapes or the binding have been deleted. - `onAfterUnbind({binding, reason})` - called after the binding and any shapes have been deleted. This still allows all the same behaviour as before, without having to spread the logic between multiple callbacks. It's also just clearer IMO since you only get one callback invocation per unbinding rather than potentially two. It also fixes our copy+paste footgun since we can now implement that by just deleting the bindings rather than invoking the `onBeforeDelete(From|To)Shape` callbacks. I'm not worried about losing the explicit before/after delete callbacks for the binding record or shape records because sdk users still have the ability to detect all those situations with full nuance in obvious ways. The one thing that would even require extra bookkeeping is getting access to a shape record after the shape was deleted, but that's probably not a thing anybody would want to do 🀷🏼 ### Change Type - [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 - [ ] `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. --- .../src/examples/pin-bindings/PinExample.tsx | 10 +- .../sticker-bindings/StickerExample.tsx | 10 +- packages/editor/api-report.md | 28 +-- packages/editor/src/index.ts | 4 +- packages/editor/src/lib/editor/Editor.ts | 105 ++++----- .../src/lib/editor/bindings/BindingUtil.ts | 30 +-- .../lib/bindings/arrow/ArrowBindingUtil.ts | 7 +- packages/tldraw/src/test/bindings.test.tsx | 210 ++++++++++++++++++ 8 files changed, 311 insertions(+), 93 deletions(-) create mode 100644 packages/tldraw/src/test/bindings.test.tsx diff --git a/apps/examples/src/examples/pin-bindings/PinExample.tsx b/apps/examples/src/examples/pin-bindings/PinExample.tsx index e6b8431dd..30325d466 100644 --- a/apps/examples/src/examples/pin-bindings/PinExample.tsx +++ b/apps/examples/src/examples/pin-bindings/PinExample.tsx @@ -1,6 +1,7 @@ import { BindingOnShapeChangeOptions, - BindingOnShapeDeleteOptions, + BindingOnUnbindOptions, + BindingUnbindReason, BindingUtil, Box, DefaultFillStyle, @@ -250,9 +251,10 @@ class PinBindingUtil extends BindingUtil { } // when the thing we're stuck to is deleted, delete the pin too - override onBeforeDeleteToShape({ binding }: BindingOnShapeDeleteOptions): void { - const pin = this.editor.getShape(binding.fromId) - if (pin) this.editor.deleteShape(pin.id) + override onBeforeUnbind({ binding, reason }: BindingOnUnbindOptions): void { + if (reason === BindingUnbindReason.DeletingToShape) { + this.editor.deleteShape(binding.fromId) + } } } diff --git a/apps/examples/src/examples/sticker-bindings/StickerExample.tsx b/apps/examples/src/examples/sticker-bindings/StickerExample.tsx index b5a9a664f..540cee142 100644 --- a/apps/examples/src/examples/sticker-bindings/StickerExample.tsx +++ b/apps/examples/src/examples/sticker-bindings/StickerExample.tsx @@ -1,6 +1,7 @@ import { BindingOnShapeChangeOptions, - BindingOnShapeDeleteOptions, + BindingOnUnbindOptions, + BindingUnbindReason, BindingUtil, Box, DefaultToolbar, @@ -152,9 +153,10 @@ class StickerBindingUtil extends BindingUtil { } // when the thing we're stuck to is deleted, delete the sticker too - override onBeforeDeleteToShape({ binding }: BindingOnShapeDeleteOptions): void { - const sticker = this.editor.getShape(binding.fromId) - if (sticker) this.editor.deleteShape(sticker.id) + override onBeforeUnbind({ binding, reason }: BindingOnUnbindOptions): void { + if (reason === BindingUnbindReason.DeletingToShape) { + this.editor.deleteShape(binding.fromId) + } } } diff --git a/packages/editor/api-report.md b/packages/editor/api-report.md index 60e14d077..a8fa65e97 100644 --- a/packages/editor/api-report.md +++ b/packages/editor/api-report.md @@ -187,12 +187,6 @@ export interface BindingOnCreateOptions { binding: Binding; } -// @public (undocumented) -export interface BindingOnDeleteOptions { - // (undocumented) - binding: Binding; -} - // @public (undocumented) export interface BindingOnShapeChangeOptions { // (undocumented) @@ -204,11 +198,21 @@ export interface BindingOnShapeChangeOptions { } // @public (undocumented) -export interface BindingOnShapeDeleteOptions { +export interface BindingOnUnbindOptions { // (undocumented) binding: Binding; // (undocumented) - shape: TLShape; + reason: BindingUnbindReason; +} + +// @public (undocumented) +export enum BindingUnbindReason { + // (undocumented) + DeletingBinding = "deleting_binding", + // (undocumented) + DeletingFromShape = "deleting_from_shape", + // (undocumented) + DeletingToShape = "deleting_to_shape" } // @public (undocumented) @@ -228,17 +232,13 @@ export abstract class BindingUtil): void; // (undocumented) - onAfterDelete?(options: BindingOnDeleteOptions): void; + onAfterUnbind?(options: BindingOnUnbindOptions): void; // (undocumented) onBeforeChange?(options: BindingOnChangeOptions): Binding | void; // (undocumented) onBeforeCreate?(options: BindingOnCreateOptions): Binding | void; // (undocumented) - onBeforeDelete?(options: BindingOnDeleteOptions): void; - // (undocumented) - onBeforeDeleteFromShape?(options: BindingOnShapeDeleteOptions): void; - // (undocumented) - onBeforeDeleteToShape?(options: BindingOnShapeDeleteOptions): void; + onBeforeUnbind?(options: BindingOnUnbindOptions): void; // (undocumented) onOperationComplete?(): void; // (undocumented) diff --git a/packages/editor/src/index.ts b/packages/editor/src/index.ts index 87d751ba7..28fd807d2 100644 --- a/packages/editor/src/index.ts +++ b/packages/editor/src/index.ts @@ -124,12 +124,12 @@ export { } 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 BindingOnShapeDeleteOptions, + type BindingOnUnbindOptions, type TLBindingUtilConstructor, } from './lib/editor/bindings/BindingUtil' export { HistoryManager } from './lib/editor/managers/HistoryManager' diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index 5453ef912..eecc85b6a 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -118,7 +118,12 @@ import { getIncrementedName } from '../utils/getIncrementedName' import { getReorderingShapesChanges } from '../utils/reorderShapes' import { applyRotationToSnapshotShapes, getRotationSnapshot } from '../utils/rotation' import { uniqueId } from '../utils/uniqueId' -import { BindingUtil, TLBindingUtilConstructor } from './bindings/BindingUtil' +import { + BindingOnUnbindOptions, + BindingUnbindReason, + BindingUtil, + TLBindingUtilConstructor, +} from './bindings/BindingUtil' import { bindingsIndex } from './derivations/bindingsIndex' import { notVisibleShapes } from './derivations/notVisibleShapes' import { parentsToChildren } from './derivations/parentsToChildren' @@ -349,10 +354,16 @@ export class Editor extends EventEmitter { this.sideEffects = this.store.sideEffects + let deletedBindings = new Map>() + const deletedShapeIds = new Set() const invalidParents = new Set() let invalidBindingTypes = new Set() this.disposables.add( this.sideEffects.registerOperationCompleteHandler(() => { + // this needs to be cleared here because further effects may delete more shapes + // and we want the next invocation of this handler to handle those separately + deletedShapeIds.clear() + for (const parentId of invalidParents) { invalidParents.delete(parentId) const parent = this.getShape(parentId) @@ -375,6 +386,14 @@ export class Editor extends EventEmitter { } } + if (deletedBindings.size) { + const t = deletedBindings + deletedBindings = new Map() + for (const opts of t.values()) { + this.getBindingUtil(opts.binding).onAfterUnbind?.(opts) + } + } + this.emit('update') }) ) @@ -461,17 +480,12 @@ export class Editor extends EventEmitter { invalidParents.add(shape.parentId) } + deletedShapeIds.add(shape.id) + const deleteBindingIds: TLBindingId[] = [] for (const binding of this.getBindingsInvolvingShape(shape)) { invalidBindingTypes.add(binding.type) - if (binding.fromId === shape.id) { - this.getBindingUtil(binding).onBeforeDeleteFromShape?.({ binding, shape }) - deleteBindingIds.push(binding.id) - } - if (binding.toId === shape.id) { - this.getBindingUtil(binding).onBeforeDeleteToShape?.({ binding, shape }) - deleteBindingIds.push(binding.id) - } + deleteBindingIds.push(binding.id) } this.deleteBindings(deleteBindingIds) @@ -510,11 +524,26 @@ export class Editor extends EventEmitter { this.getBindingUtil(bindingAfter).onAfterChange?.({ bindingBefore, bindingAfter }) }, beforeDelete: (binding) => { - this.getBindingUtil(binding).onBeforeDelete?.({ 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 = { + 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) }, afterDelete: (binding) => { invalidBindingTypes.add(binding.type) - this.getBindingUtil(binding).onAfterDelete?.({ binding }) }, }, page: { @@ -2313,7 +2342,7 @@ export class Editor extends EventEmitter { const { currentScreenPoint, currentPagePoint } = this.inputs const { screenBounds } = this.store.unsafeGetWithoutCapture(TLINSTANCE_ID)! - // compare the next page point (derived from the curent camera) to the current page point + // compare the next page point (derived from the current camera) to the current page point if ( currentScreenPoint.x / z - x !== currentPagePoint.x || currentScreenPoint.y / z - y !== currentPagePoint.y @@ -2809,7 +2838,7 @@ export class Editor extends EventEmitter { * editor.zoomToUser(myUserId, { animation: { duration: 200 } }) * ``` * - * @param userId - The id of the user to aniamte to. + * @param userId - The id of the user to animate to. * @param opts - The camera move options. * @public */ @@ -4235,7 +4264,7 @@ export class Editor extends EventEmitter { const selectedShapeIds = this.getSelectedShapeIds() return this.getCurrentPageShapesSorted() .filter((shape) => shape.type !== 'group' && selectedShapeIds.includes(shape.id)) - .reverse() // findlast + .reverse() // find last .find((shape) => this.isPointInShape(shape, point, { hitInside: true, margin: 0 })) } @@ -4314,7 +4343,7 @@ export class Editor extends EventEmitter { if (this.isShapeOfType(shape, 'frame')) { // On the rare case that we've hit a frame, test again hitInside to be forced true; - // this prevents clicks from passing through the body of a frame to shapes behhind it. + // this prevents clicks from passing through the body of a frame to shapes behind it. // If the hit is within the frame's outer margin, then select the frame const distance = geometry.distanceToPoint(pointInShapeSpace, hitInside) @@ -4396,7 +4425,7 @@ export class Editor extends EventEmitter { inMarginClosestToEdgeHit = shape } } else if (!inMarginClosestToEdgeHit) { - // If we're not within margin distnce to any edge, and if the + // If we're not within margin distance to any edge, and if the // shape is hollow, then we want to hit the shape with the // smallest area. (There's a bug here with self-intersecting // shapes, like a closed drawing of an "8", but that's a bigger @@ -4472,7 +4501,7 @@ export class Editor extends EventEmitter { const { hitInside = false, margin = 0 } = opts const id = typeof shape === 'string' ? shape : shape.id // If the shape is masked, and if the point falls outside of that - // mask, then it's defintely a missβ€”we don't need to test further. + // mask, then it's definitely a missβ€”we don't need to test further. const pageMask = this.getShapeMask(id) if (pageMask && !pointInPolygon(point, pageMask)) return false @@ -4792,7 +4821,7 @@ export class Editor extends EventEmitter { const shapesToReparent = compact(ids.map((id) => this.getShape(id))) - // The user is allowed to re-parent locked shapes. Unintutive? Yeah! But there are plenty of + // The user is allowed to re-parent locked shapes. Unintuitive? Yeah! But there are plenty of // times when a locked shape's parent is deleted... and we need to put that shape somewhere! const lockedShapes = shapesToReparent.filter((shape) => shape.isLocked) @@ -4900,7 +4929,7 @@ export class Editor extends EventEmitter { * * @param ids - The ids of the shapes to get descendants of. * - * @returns The decscendant ids. + * @returns The descendant ids. * * @public */ @@ -8347,7 +8376,7 @@ export class Editor extends EventEmitter { let behavior = wheelBehavior - // If the camera behavior is "zoom" and the ctrl key is presssed, then pan; + // If the camera behavior is "zoom" and the ctrl key is pressed, then pan; // If the camera behavior is "pan" and the ctrl key is not pressed, then zoom if (inputs.ctrlKey) behavior = wheelBehavior === 'pan' ? 'zoom' : 'pan' @@ -8705,8 +8734,7 @@ function withoutBindingsToUnrelatedShapes( callback: (bindingsWithBoth: Set) => T ): T { const bindingsWithBoth = new Set() - const bindingsWithoutFrom = new Set() - const bindingsWithoutTo = new Set() + const bindingsToRemove = new Set() for (const shapeId of shapeIds) { const shape = editor.getShape(shapeId) @@ -8719,11 +8747,8 @@ function withoutBindingsToUnrelatedShapes( bindingsWithBoth.add(binding.id) continue } - if (!hasFrom) { - bindingsWithoutFrom.add(binding.id) - } - if (!hasTo) { - bindingsWithoutTo.add(binding.id) + if (!hasFrom || !hasTo) { + bindingsToRemove.add(binding.id) } } } @@ -8732,31 +8757,7 @@ function withoutBindingsToUnrelatedShapes( editor.history.ignore(() => { const changes = editor.store.extractingChanges(() => { - const bindingsToRemove: TLBindingId[] = [] - - for (const bindingId of bindingsWithoutFrom) { - const binding = editor.getBinding(bindingId) - if (!binding) continue - - const shape = editor.getShape(binding.fromId) - if (!shape) continue - - editor.getBindingUtil(binding).onBeforeDeleteFromShape?.({ binding, shape }) - bindingsToRemove.push(binding.id) - } - - for (const bindingId of bindingsWithoutTo) { - const binding = editor.getBinding(bindingId) - if (!binding) continue - - const shape = editor.getShape(binding.toId) - if (!shape) continue - - editor.getBindingUtil(binding).onBeforeDeleteToShape?.({ binding, shape }) - bindingsToRemove.push(binding.id) - } - - editor.deleteBindings(bindingsToRemove) + editor.deleteBindings([...bindingsToRemove]) try { result = Result.ok(callback(bindingsWithBoth)) diff --git a/packages/editor/src/lib/editor/bindings/BindingUtil.ts b/packages/editor/src/lib/editor/bindings/BindingUtil.ts index a8b9f5454..476feacb3 100644 --- a/packages/editor/src/lib/editor/bindings/BindingUtil.ts +++ b/packages/editor/src/lib/editor/bindings/BindingUtil.ts @@ -18,14 +18,22 @@ export interface BindingOnCreateOptions { } /** @public */ -export interface BindingOnChangeOptions { - bindingBefore: Binding - bindingAfter: Binding +export enum BindingUnbindReason { + DeletingFromShape = 'deleting_from_shape', + DeletingToShape = 'deleting_to_shape', + DeletingBinding = 'deleting_binding', } /** @public */ -export interface BindingOnDeleteOptions { +export interface BindingOnUnbindOptions { binding: Binding + reason: BindingUnbindReason +} + +/** @public */ +export interface BindingOnChangeOptions { + bindingBefore: Binding + bindingAfter: Binding } /** @public */ @@ -35,12 +43,6 @@ export interface BindingOnShapeChangeOptions { shapeAfter: TLShape } -/** @public */ -export interface BindingOnShapeDeleteOptions { - binding: Binding - shape: TLShape -} - /** @public */ export abstract class BindingUtil { constructor(public editor: Editor) {} @@ -63,17 +65,15 @@ export abstract class BindingUtil): void + onAfterUnbind?(options: BindingOnUnbindOptions): void + // self lifecycle hooks onBeforeCreate?(options: BindingOnCreateOptions): Binding | void onAfterCreate?(options: BindingOnCreateOptions): void onBeforeChange?(options: BindingOnChangeOptions): Binding | void onAfterChange?(options: BindingOnChangeOptions): void - onBeforeDelete?(options: BindingOnDeleteOptions): void - onAfterDelete?(options: BindingOnDeleteOptions): void onAfterChangeFromShape?(options: BindingOnShapeChangeOptions): void onAfterChangeToShape?(options: BindingOnShapeChangeOptions): void - - onBeforeDeleteFromShape?(options: BindingOnShapeDeleteOptions): void - onBeforeDeleteToShape?(options: BindingOnShapeDeleteOptions): void } diff --git a/packages/tldraw/src/lib/bindings/arrow/ArrowBindingUtil.ts b/packages/tldraw/src/lib/bindings/arrow/ArrowBindingUtil.ts index 7576be4a1..3583c3162 100644 --- a/packages/tldraw/src/lib/bindings/arrow/ArrowBindingUtil.ts +++ b/packages/tldraw/src/lib/bindings/arrow/ArrowBindingUtil.ts @@ -2,7 +2,8 @@ import { BindingOnChangeOptions, BindingOnCreateOptions, BindingOnShapeChangeOptions, - BindingOnShapeDeleteOptions, + BindingOnUnbindOptions, + BindingUnbindReason, BindingUtil, Editor, IndexKey, @@ -60,7 +61,9 @@ export class ArrowBindingUtil extends BindingUtil { } // when the shape the arrow is pointing to is deleted - override onBeforeDeleteToShape({ binding }: BindingOnShapeDeleteOptions): void { + override onBeforeUnbind({ binding, reason }: BindingOnUnbindOptions): void { + // don't need to do anything if the arrow is being deleted + if (reason === BindingUnbindReason.DeletingFromShape) return const arrow = this.editor.getShape(binding.fromId) if (!arrow) return unbindArrowTerminal(this.editor, arrow, binding.props.terminal) diff --git a/packages/tldraw/src/test/bindings.test.tsx b/packages/tldraw/src/test/bindings.test.tsx new file mode 100644 index 000000000..abd928fd4 --- /dev/null +++ b/packages/tldraw/src/test/bindings.test.tsx @@ -0,0 +1,210 @@ +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]> +const mockOnAfterUnbind = jest.fn() as jest.Mock]> + +class TestBindingUtil extends BindingUtil { + static override type = 'test' + + static override props = {} + + override getDefaultProps(): object { + return {} + } + + override onBeforeUnbind(options: BindingOnUnbindOptions): void { + mockOnBeforeUnbind(options) + } + + override onAfterUnbind(options: BindingOnUnbindOptions): void { + mockOnAfterUnbind(options) + } +} + +beforeEach(() => { + editor = new TestEditor({ bindingUtils: [TestBindingUtil] }) + + editor.createShapesFromJsx([ + , + , + , + , + ]) + + 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) +})