Fix binding bugs

packages/tldraw/src/state/command/delete/delete.command.spec.ts

@@ -112,4 +112,6 @@ describe('Delete command', () => {
       expect(tlstate.getShape('newGroup')).toBeUndefined()
     })
   })
+
+  it.todo('Does not delete uneffected bindings.')
 })

packages/tldraw/src/state/command/duplicate/duplicate.command.spec.ts

@@ -158,4 +158,6 @@ describe('Duplicate command', () => {
       expect(Object.keys(tlstate.page.shapes).length).toBe(beforeShapeIds.length + 1)
     })
   })
+
+  it.todo('Does not delete uneffected bindings.')
 })

packages/tldraw/src/state/command/duplicate/duplicate.command.ts

@@ -83,46 +83,48 @@ export function duplicate(data: Data, ids: string[]): TLDrawCommand {
   })
 
   // Which ids did we end up duplicating?
-  const duplicatedShapeIds = Object.keys(duplicateMap)
+  const dupedShapeIds = new Set(Object.keys(duplicateMap))
 
   // Handle bindings that effect duplicated shapes
-  Object.values(page.bindings).forEach((binding) => {
-    if (duplicatedShapeIds.includes(binding.fromId)) {
-      if (duplicatedShapeIds.includes(binding.toId)) {
-        // If the binding is between two duplicating shapes then
-        // duplicate the binding, too
-        const duplicatedBindingId = Utils.uniqueId()
+  Object.values(page.bindings)
+    .filter((binding) => dupedShapeIds.has(binding.fromId) || dupedShapeIds.has(binding.toId))
+    .forEach((binding) => {
+      if (dupedShapeIds.has(binding.fromId)) {
+        if (dupedShapeIds.has(binding.toId)) {
+          // If the binding is between two duplicating shapes then
+          // duplicate the binding, too
+          const duplicatedBindingId = Utils.uniqueId()
 
-        const duplicatedBinding = {
-          ...Utils.deepClone(binding),
-          id: duplicatedBindingId,
-          fromId: duplicateMap[binding.fromId],
-          toId: duplicateMap[binding.toId],
+          const duplicatedBinding = {
+            ...Utils.deepClone(binding),
+            id: duplicatedBindingId,
+            fromId: duplicateMap[binding.fromId],
+            toId: duplicateMap[binding.toId],
+          }
+
+          before.bindings[duplicatedBindingId] = undefined
+          after.bindings[duplicatedBindingId] = duplicatedBinding
+
+          // Change the duplicated shape's handle so that it reference
+          // the duplicated binding
+          const boundShape = after.shapes[duplicatedBinding.fromId]
+          Object.values(boundShape!.handles!).forEach((handle) => {
+            if (handle!.bindingId === binding.id) {
+              handle!.bindingId = duplicatedBindingId
+            }
+          })
+        } else {
+          // If only the fromId is selected, delete the binding on
+          // the duplicated shape's handles
+          const boundShape = after.shapes[duplicateMap[binding.fromId]]
+          Object.values(boundShape!.handles!).forEach((handle) => {
+            if (handle!.bindingId === binding.id) {
+              handle!.bindingId = undefined
+            }
+          })
         }
-
-        before.bindings[duplicatedBindingId] = undefined
-        after.bindings[duplicatedBindingId] = duplicatedBinding
-
-        // Change the duplicated shape's handle so that it reference
-        // the duplicated binding
-        const boundShape = after.shapes[duplicatedBinding.fromId]
-        Object.values(boundShape!.handles!).forEach((handle) => {
-          if (handle!.bindingId === binding.id) {
-            handle!.bindingId = duplicatedBindingId
-          }
-        })
-      } else {
-        // If only the fromId is selected, delete the binding on
-        // the duplicated shape's handles
-        const boundShape = after.shapes[duplicateMap[binding.fromId]]
-        Object.values(boundShape!.handles!).forEach((handle) => {
-          if (handle!.bindingId === binding.id) {
-            handle!.bindingId = undefined
-          }
-        })
       }
-    }
-  })
+    })
 
   return {
     id: 'duplicate',
@@ -142,7 +144,9 @@ export function duplicate(data: Data, ids: string[]): TLDrawCommand {
           [currentPageId]: after,
         },
         pageStates: {
-          [currentPageId]: { selectedIds: duplicatedShapeIds.map((id) => duplicateMap[id]) },
+          [currentPageId]: {
+            selectedIds: Array.from(dupedShapeIds.values()).map((id) => duplicateMap[id]),
+          },
         },
       },
     },

packages/tldraw/src/state/command/move-to-page/move-to-page.command.spec.ts

@@ -247,4 +247,6 @@ describe('Move to page command', () => {
       ])
     })
   })
+
+  it.todo('Does not delete uneffected bindings.')
 })

packages/tldraw/src/state/command/utils/removeShapesFromPage.ts

@@ -52,44 +52,49 @@ export function removeShapesFromPage(data: Data, ids: string[], pageId: string)
   const page = TLDR.getPage(data, pageId)
 
   // We also need to delete bindings that reference the deleted shapes
-  Object.values(page.bindings).forEach((binding) => {
-    for (const id of [binding.toId, binding.fromId]) {
-      // If the binding references a deleted shape...
-      if (after.shapes[id] === undefined) {
-        // Delete this binding
-        before.bindings[binding.id] = binding
-        after.bindings[binding.id] = undefined
+  Object.values(page.bindings)
+    .filter((binding) => deletedIds.has(binding.fromId) || deletedIds.has(binding.toId))
+    .forEach((binding) => {
+      for (const id of [binding.toId, binding.fromId]) {
+        // If the binding references a deleted shape...
+        if (after.shapes[id] === undefined) {
+          // Delete this binding
+          before.bindings[binding.id] = binding
+          after.bindings[binding.id] = undefined
 
-        // Let's also look each the bound shape...
-        const shape = TLDR.getShape(data, id, pageId)
+          // Let's also look each the bound shape...
+          const shape = TLDR.getShape(data, id, pageId)
 
-        // If the bound shape has a handle that references the deleted binding...
-        if (shape.handles) {
-          Object.values(shape.handles)
-            .filter((handle) => handle.bindingId === binding.id)
-            .forEach((handle) => {
-              // Save the binding reference in the before patch
-              before.shapes[id] = {
-                ...before.shapes[id],
-                handles: {
-                  ...before.shapes[id]?.handles,
-                  [handle.id]: { bindingId: binding.id },
-                },
-              }
-
-              // Unless we're currently deleting the shape, remove the
-              // binding reference from the after patch
-              if (!deletedIds.has(id)) {
-                after.shapes[id] = {
-                  ...after.shapes[id],
-                  handles: { ...after.shapes[id]?.handles, [handle.id]: { bindingId: undefined } },
+          // If the bound shape has a handle that references the deleted binding...
+          if (shape.handles) {
+            Object.values(shape.handles)
+              .filter((handle) => handle.bindingId === binding.id)
+              .forEach((handle) => {
+                // Save the binding reference in the before patch
+                before.shapes[id] = {
+                  ...before.shapes[id],
+                  handles: {
+                    ...before.shapes[id]?.handles,
+                    [handle.id]: { bindingId: binding.id },
+                  },
                 }
-              }
-            })
+
+                // Unless we're currently deleting the shape, remove the
+                // binding reference from the after patch
+                if (!deletedIds.has(id)) {
+                  after.shapes[id] = {
+                    ...after.shapes[id],
+                    handles: {
+                      ...after.shapes[id]?.handles,
+                      [handle.id]: { bindingId: undefined },
+                    },
+                  }
+                }
+              })
+          }
         }
       }
-    }
-  })
+    })
 
   return { before, after }
 }

packages/tldraw/src/state/session/sessions/translate/translate.session.ts

@@ -382,30 +382,32 @@ export function getTranslateSnapshot(data: Data) {
 
   // Potentially confusing name here: these are the ids of the
   // original shapes that were cloned, not their clones' ids.
-  const clonedShapeIds = Object.keys(cloneMap)
+  const clonedShapeIds = new Set(Object.keys(cloneMap))
 
   const bindingsToDelete: TLDrawBinding[] = []
 
   // Create cloned bindings for shapes where both to and from shapes are selected
   // (if the user clones, then we will create a new binding for the clones)
-  Object.values(page.bindings).forEach((binding) => {
-    if (clonedShapeIds.includes(binding.fromId)) {
-      if (clonedShapeIds.includes(binding.toId)) {
-        const cloneId = Utils.uniqueId()
-        const cloneBinding = {
-          ...Utils.deepClone(binding),
-          id: cloneId,
-          fromId: cloneMap[binding.fromId] || binding.fromId,
-          toId: cloneMap[binding.toId] || binding.toId,
-        }
+  Object.values(page.bindings)
+    .filter((binding) => clonedShapeIds.has(binding.fromId) || clonedShapeIds.has(binding.toId))
+    .forEach((binding) => {
+      if (clonedShapeIds.has(binding.fromId)) {
+        if (clonedShapeIds.has(binding.toId)) {
+          const cloneId = Utils.uniqueId()
+          const cloneBinding = {
+            ...Utils.deepClone(binding),
+            id: cloneId,
+            fromId: cloneMap[binding.fromId] || binding.fromId,
+            toId: cloneMap[binding.toId] || binding.toId,
+          }
 
-        clonedBindingsMap[binding.id] = cloneId
-        clonedBindings.push(cloneBinding)
-      } else {
-        bindingsToDelete.push(binding)
+          clonedBindingsMap[binding.id] = cloneId
+          clonedBindings.push(cloneBinding)
+        } else {
+          bindingsToDelete.push(binding)
+        }
       }
-    }
-  })
+    })
 
   // Assign new binding ids to clones (or delete them!)
   clones.forEach((clone) => {