fix: stop duplicating required children on reparent (#4537)

Turns our delete does not clear instances when detachable check fails.
Good news: new child constraints were added only to tabs. Bad news: I
think deleting pages kept instances in the build, oops.

Fixed reparent and added checks for delete only where necessary.

How to test

- insert tabs content
- delete second trigger and content
- reposition trigger
- it is duplicated

Author: TrySound

apps/builder/app/builder/features/ai/apply-operations.ts

@@ -7,6 +7,7 @@ import { isBaseBreakpoint } from "~/shared/breakpoints";
 import {
   deleteInstanceMutable,
   insertTemplateData,
+  isInstanceDetachable,
   updateWebstudioData,
 } from "~/shared/instance-utils";
 import {
@@ -101,7 +102,14 @@ const deleteInstanceByOp = (
 ) => {
   const instanceSelector = computeSelectorForInstanceId(operation.wsId);
   if (instanceSelector) {
+    // @todo tell user they can't delete root
+    if (instanceSelector.length === 1) {
+      return;
+    }
     updateWebstudioData((data) => {
+      if (isInstanceDetachable(data.instances, instanceSelector) === false) {
+        return;
+      }
       deleteInstanceMutable(data, instanceSelector);
     });
   }

apps/builder/app/builder/shared/commands.ts

@@ -1,3 +1,5 @@
+import { blockTemplateComponent } from "@webstudio-is/react-sdk";
+import { toast } from "@webstudio-is/design-system";
 import { createCommandsEmitter, type Command } from "~/shared/commands-emitter";
 import {
   $editingItemSelector,
@@ -19,6 +21,7 @@ import {
   extractWebstudioFragment,
   insertWebstudioFragmentCopy,
   updateWebstudioData,
+  isInstanceDetachable,
 } from "~/shared/instance-utils";
 import type { InstanceSelector } from "~/shared/tree-utils";
 import { serverSyncStore } from "~/shared/sync";
@@ -32,7 +35,6 @@ import { selectInstance } from "~/shared/awareness";
 import { openCommandPanel } from "../features/command-panel";
 import { builderApi } from "~/shared/builder-api";
 import { findBlockSelector } from "../features/workspace/canvas-tools/outline/block-instance-outline";
-import { blockTemplateComponent } from "@webstudio-is/react-sdk";
 
 const makeBreakpointCommand = <CommandName extends string>(
   name: CommandName,
@@ -65,8 +67,13 @@ const deleteSelectedInstance = () => {
   if (selectedInstanceSelector.length === 1) {
     return;
   }
-
   const instances = $instances.get();
+  if (isInstanceDetachable(instances, selectedInstanceSelector) === false) {
+    toast.error(
+      "This instance can not be moved outside of its parent component."
+    );
+    return false;
+  }
 
   if ($isContentMode.get()) {
     // In content mode we are allowing to delete childen of the editable block

apps/builder/app/shared/instance-utils.test.tsx

@@ -11,6 +11,7 @@ import { $, ws, renderJsx, ExpressionValue } from "@webstudio-is/sdk/testing";
 import { parseCss } from "@webstudio-is/css-data";
 import { coreMetas } from "@webstudio-is/react-sdk";
 import * as defaultMetas from "@webstudio-is/sdk-components-react/metas";
+import * as radixMetas from "@webstudio-is/sdk-components-react-radix/metas";
 import type {
   Asset,
   Breakpoint,
@@ -844,6 +845,36 @@ describe("reparent instance", () => {
       ])
     );
   });
+
+  test("reparent required child", () => {
+    $instances.set(
+      renderJsx(
+        <$.Body ws:id="body">
+          <$.Tooltip ws:id="tooltip">
+            <$.TooltipTrigger ws:id="trigger"></$.TooltipTrigger>
+            <$.TooltipContent ws:id="content"></$.TooltipContent>
+          </$.Tooltip>
+        </$.Body>
+      ).instances
+    );
+    $registeredComponentMetas.set(
+      new Map(Object.entries({ ...defaultMetas, ...radixMetas }))
+    );
+    reparentInstance(["trigger", "tooltip", "body"], {
+      parentSelector: ["tooltip", "body"],
+      position: "end",
+    });
+    expect($instances.get()).toEqual(
+      renderJsx(
+        <$.Body ws:id="body">
+          <$.Tooltip ws:id="tooltip">
+            <$.TooltipContent ws:id="content"></$.TooltipContent>
+            <$.TooltipTrigger ws:id={expect.any(String)}></$.TooltipTrigger>
+          </$.Tooltip>
+        </$.Body>
+      ).instances
+    );
+  });
 });
 
 const getWebstudioDataStub = (
@@ -888,24 +919,6 @@ describe("delete instance", () => {
     );
   });
 
-  test("prevent deleting root instance", () => {
-    // body
-    //   box1
-    const instances = new Map([
-      createInstancePair("body", "Body", [{ type: "id", value: "box1" }]),
-      createInstancePair("box1", "Box", []),
-    ]);
-    $registeredComponentMetas.set(createFakeComponentMetas({}));
-    const data = getWebstudioDataStub({ instances });
-    deleteInstanceMutable(data, ["body"]);
-    expect(data.instances).toEqual(
-      new Map([
-        createInstancePair("body", "Body", [{ type: "id", value: "box1" }]),
-        createInstancePair("box1", "Box", []),
-      ])
-    );
-  });
-
   test("delete instance from collection item", () => {
     // body
     //   list

apps/builder/app/shared/instance-utils.ts

@@ -496,16 +496,6 @@ export const deleteInstanceMutable = (
   data: WebstudioData,
   instanceSelector: InstanceSelector
 ) => {
-  // @todo tell user they can't delete root
-  if (instanceSelector.length === 1) {
-    return false;
-  }
-  if (isInstanceDetachable(data.instances, instanceSelector) === false) {
-    toast.error(
-      "This instance can not be moved outside of its parent component."
-    );
-    return false;
-  }
   const {
     instances,
     props,