Refactor closest droppable with instances index (#945)

Ref https://github.com/webstudio-is/webstudio-designer/issues/696

A lot of our utilities can reuse indexed instances instead of
retraversing tree. This will also simplify migration to normalized
instances.

Here I refactored droppable instance lookup.

apps/designer/app/canvas/shared/instance.test.ts

@@ -1,69 +0,0 @@
-import { describe, test, expect } from "@jest/globals";
-import type { Instance } from "@webstudio-is/project-build";
-import { findInsertLocation } from "./instance";
-
-const tree: Instance = {
-  type: "instance",
-  component: "Body",
-  id: "root",
-  children: [
-    {
-      type: "instance",
-      component: "Heading",
-      id: "heading1",
-      children: [],
-    },
-    {
-      type: "instance",
-      component: "Box",
-      id: "box1",
-      children: [
-        {
-          type: "instance",
-          component: "Box",
-          id: "box2",
-          children: [],
-        },
-        {
-          type: "instance",
-          component: "Heading",
-          id: "heading2",
-          children: [],
-        },
-        {
-          type: "instance",
-          component: "Heading",
-          id: "heading3",
-          children: [],
-        },
-      ],
-    },
-  ],
-};
-
-describe("findInsertLocation", () => {
-  test("if no selected instance, insert into root", () => {
-    expect(findInsertLocation(tree, undefined)).toEqual({
-      parentId: "root",
-      position: "end",
-    });
-  });
-
-  test("if selected instance can accept children insert into it", () => {
-    expect(findInsertLocation(tree, "box1")).toEqual({
-      parentId: "box1",
-      position: "end",
-    });
-  });
-
-  test("if selected instance can not accept children insert into parent", () => {
-    expect(findInsertLocation(tree, "heading1")).toEqual({
-      parentId: "root",
-      position: 1,
-    });
-    expect(findInsertLocation(tree, "heading2")).toEqual({
-      parentId: "box1",
-      position: 2,
-    });
-  });
-});

apps/designer/app/canvas/shared/instance.ts

@@ -1,8 +1,7 @@
 import { useEffect } from "react";
 import store from "immerhin";
 import type { Instance } from "@webstudio-is/project-build";
-import { getComponentMeta } from "@webstudio-is/react-sdk";
-import { utils, type InstanceInsertionSpec } from "@webstudio-is/project";
+import { utils } from "@webstudio-is/project";
 import { useSubscribe } from "~/shared/pubsub";
 import {
   rootInstanceContainer,
@@ -16,57 +15,20 @@ declare module "~/shared/pubsub" {
     textEditingInstanceId?: Instance["id"];
     insertInstance: {
       instance: Instance;
-      dropTarget?: { parentId: Instance["id"]; position: number };
+      dropTarget?: { parentId: Instance["id"]; position: number | "end" };
     };
   }
 }
 
-export const findInsertLocation = (
-  rootInstance: Instance,
-  selectedInstanceId: Instance["id"] | undefined
-): InstanceInsertionSpec => {
-  if (selectedInstanceId === undefined) {
-    return { parentId: rootInstance.id, position: "end" };
-  }
-
-  const path = utils.tree.getInstancePathWithPositions(
-    rootInstance,
-    selectedInstanceId
-  );
-  path.reverse();
-
-  const parentIndex = path.findIndex(({ item }) => {
-    const meta = getComponentMeta(item.component);
-    return meta?.type === "body" || meta?.type === "container";
-  });
-
-  // Just in case selected Instance is not in the tree for some reason.
-  if (parentIndex === -1) {
-    return { parentId: rootInstance.id, position: "end" };
-  }
-
-  return {
-    parentId: path[parentIndex].item.id,
-    position: parentIndex === 0 ? "end" : path[parentIndex - 1].position + 1,
-  };
-};
-
 export const useInsertInstance = () => {
   useSubscribe("insertInstance", ({ instance, dropTarget }) => {
-    const selectedInstanceId = selectedInstanceIdStore.get();
     store.createTransaction([rootInstanceContainer], (rootInstance) => {
       if (rootInstance === undefined) {
         return;
       }
-      const hasInserted = utils.tree.insertInstanceMutable(
-        rootInstance,
-        instance,
-        dropTarget ?? findInsertLocation(rootInstance, selectedInstanceId)
-      );
-      if (hasInserted) {
-        selectedInstanceIdStore.set(instance.id);
-      }
+      utils.tree.insertInstanceMutable(rootInstance, instance, dropTarget);
     });
+    selectedInstanceIdStore.set(instance.id);
   });
 };
 

apps/designer/app/designer/features/sidebar-left/panels/components/components.tsx

@@ -9,6 +9,11 @@ import {
 import { theme, Flex, useDrag, type Point } from "@webstudio-is/design-system";
 import { PlusIcon } from "@webstudio-is/icons";
 import { utils } from "@webstudio-is/project";
+import { findClosestDroppableTarget } from "~/shared/tree-utils";
+import {
+  instancesIndexStore,
+  selectedInstanceIdStore,
+} from "~/shared/nano-states";
 import { useSubscribe, type Publish } from "~/shared/pubsub";
 import { useCanvasRect, useZoom } from "~/designer/shared/nano-states";
 import type { TabName } from "../../types";
@@ -168,9 +173,13 @@ export const TabContent = ({ publish, onSetActiveTab }: TabContentProps) => {
               const instance = utils.tree.createInstance({
                 component,
               });
+              const dropTarget = findClosestDroppableTarget(
+                instancesIndexStore.get(),
+                selectedInstanceIdStore.get()
+              );
               publish({
                 type: "insertInstance",
-                payload: { instance },
+                payload: { instance, dropTarget },
               });
             }}
           />

apps/designer/app/shared/copy-paste/copy-paste-instance.ts

@@ -9,7 +9,6 @@ import {
   StyleSourceSelections,
 } from "@webstudio-is/project-build";
 import { utils } from "@webstudio-is/project";
-import { findInsertLocation } from "~/canvas/shared/instance";
 import {
   rootInstanceContainer,
   propsStore,
@@ -17,6 +16,7 @@ import {
   selectedInstanceIdStore,
   styleSourceSelectionsStore,
   styleSourcesStore,
+  instancesIndexStore,
 } from "../nano-states";
 import {
   cloneInstance,
@@ -24,6 +24,7 @@ import {
   cloneStyles,
   cloneStyleSources,
   cloneStyleSourceSelections,
+  findClosestDroppableTarget,
   findSubtree,
   findSubtreeLocalStyleSources,
 } from "../tree-utils";
@@ -89,7 +90,10 @@ const copyInstanceData = (targetInstanceId: string) => {
 };
 
 const pasteInstance = (data: InstanceData) => {
-  const selectedInstanceId = selectedInstanceIdStore.get();
+  const dropTarget = findClosestDroppableTarget(
+    instancesIndexStore.get(),
+    selectedInstanceIdStore.get()
+  );
   store.createTransaction(
     [
       rootInstanceContainer,
@@ -102,14 +106,7 @@ const pasteInstance = (data: InstanceData) => {
       if (rootInstance === undefined) {
         return;
       }
-      const hasInserted = utils.tree.insertInstanceMutable(
-        rootInstance,
-        data.instance,
-        findInsertLocation(rootInstance, selectedInstanceId)
-      );
-      if (hasInserted === false) {
-        return;
-      }
+      utils.tree.insertInstanceMutable(rootInstance, data.instance, dropTarget);
       // append without checking existing
       // because all data already cloned with new ids
       props.push(...data.props);

apps/designer/app/shared/nano-states/nano-states.ts

@@ -1,6 +1,7 @@
 import { useMemo } from "react";
 import { atom, computed, type WritableAtom } from "nanostores";
 import { useStore } from "@nanostores/react";
+import type { AuthPermit } from "@webstudio-is/trpc-interface";
 import type { Asset } from "@webstudio-is/asset-uploader";
 import type {
   Instance,
@@ -21,7 +22,7 @@ import type {
 } from "~/designer/shared/assets";
 import { useSyncInitializeOnce } from "../hook-utils";
 import { shallowComputed } from "../store-utils";
-import type { AuthPermit } from "@webstudio-is/trpc-interface";
+import { createInstancesIndex } from "../tree-utils";
 
 const useValue = <T>(atom: WritableAtom<T>) => {
   const value = useStore(atom);
@@ -37,23 +38,7 @@ export const useSetRootInstance = (root: Instance) => {
 };
 export const instancesIndexStore = computed(
   rootInstanceContainer,
-  (rootInstance) => {
-    const instancesById = new Map<Instance["id"], Instance>();
-    const traverseInstances = (instance: Instance) => {
-      instancesById.set(instance.id, instance);
-      for (const child of instance.children) {
-        if (child.type === "instance") {
-          traverseInstances(child);
-        }
-      }
-    };
-    if (rootInstance !== undefined) {
-      traverseInstances(rootInstance);
-    }
-    return {
-      instancesById,
-    };
-  }
+  createInstancesIndex
 );
 
 export const propsStore = atom<Props>([]);

apps/designer/app/shared/tree-utils.test.ts

@@ -12,17 +12,23 @@ import {
   cloneStyles,
   cloneStyleSources,
   cloneStyleSourceSelections,
+  createInstancesIndex,
+  findClosestDroppableTarget,
   findSubtree,
   findSubtreeLocalStyleSources,
 } from "./tree-utils";
 
 const expectString = expect.any(String) as unknown as string;
 
-const createInstance = (id: Instance["id"], children: Instance[]): Instance => {
+const createInstance = (
+  id: Instance["id"],
+  component: string,
+  children: Instance[]
+): Instance => {
   return {
     type: "instance",
     id,
-    component: "Box",
+    component,
     children: children,
   };
 };
@@ -77,31 +83,75 @@ const createStyleDecl = (styleSourceId: string): StyleDecl => {
   };
 };
 
+test("find closest droppable target", () => {
+  const rootInstance = createInstance("root", "Body", [
+    createInstance("box1", "Box", [
+      createInstance("box11", "Box", []),
+      createInstance("box12", "Box", []),
+      createInstance("box13", "Box", []),
+    ]),
+    createInstance("box2", "Box", [
+      createInstance("paragraph21", "Paragraph", [
+        createInstance("bold", "Bold", []),
+      ]),
+      createInstance("box22", "Box", []),
+    ]),
+    createInstance("box3", "Box", [
+      createInstance("box31", "Box", []),
+      createInstance("box32", "Box", []),
+      createInstance("box33", "Box", []),
+    ]),
+  ]);
+  const instancesIndex = createInstancesIndex(rootInstance);
+  expect(findClosestDroppableTarget(instancesIndex, "bold")).toEqual({
+    parentId: "box2",
+    position: 1,
+  });
+  expect(findClosestDroppableTarget(instancesIndex, "box3")).toEqual({
+    parentId: "box3",
+    position: 3,
+  });
+  expect(findClosestDroppableTarget(instancesIndex, "root")).toEqual({
+    parentId: "root",
+    position: 3,
+  });
+  expect(findClosestDroppableTarget(instancesIndex, undefined)).toEqual({
+    parentId: "root",
+    position: 3,
+  });
+});
+
 test("find subtree with all descendants and parent instance", () => {
-  const rootInstance: Instance = createInstance("root", [
-    createInstance("box1", []),
-    createInstance("box2", [
-      createInstance("box3", [
-        createInstance("child1", []),
-        createInstance("child2", [createInstance("descendant", [])]),
-        createInstance("child3", []),
+  const rootInstance: Instance = createInstance("root", "Box", [
+    createInstance("box1", "Box", []),
+    createInstance("box2", "Box", [
+      createInstance("box3", "Box", [
+        createInstance("child1", "Box", []),
+        createInstance("child2", "Box", [
+          createInstance("descendant", "Box", []),
+        ]),
+        createInstance("child3", "Box", []),
       ]),
     ]),
-    createInstance("box4", []),
+    createInstance("box4", "Box", []),
   ]);
 
   expect(findSubtree(rootInstance, "box3")).toEqual({
-    parentInstance: createInstance("box2", [
-      createInstance("box3", [
-        createInstance("child1", []),
-        createInstance("child2", [createInstance("descendant", [])]),
-        createInstance("child3", []),
+    parentInstance: createInstance("box2", "Box", [
+      createInstance("box3", "Box", [
+        createInstance("child1", "Box", []),
+        createInstance("child2", "Box", [
+          createInstance("descendant", "Box", []),
+        ]),
+        createInstance("child3", "Box", []),
       ]),
     ]),
-    targetInstance: createInstance("box3", [
-      createInstance("child1", []),
-      createInstance("child2", [createInstance("descendant", [])]),
-      createInstance("child3", []),
+    targetInstance: createInstance("box3", "Box", [
+      createInstance("child1", "Box", []),
+      createInstance("child2", "Box", [
+        createInstance("descendant", "Box", []),
+      ]),
+      createInstance("child3", "Box", []),
     ]),
     subtreeIds: new Set(["box3", "child1", "child2", "child3", "descendant"]),
   });
@@ -116,17 +166,19 @@ test("find subtree with all descendants and parent instance", () => {
 });
 
 test("clone instance tree and provide cloned ids map", () => {
-  const instance = createInstance("box", [
-    createInstance("child1", []),
-    createInstance("child2", [createInstance("descendant", [])]),
-    createInstance("child3", []),
+  const instance = createInstance("box", "Box", [
+    createInstance("child1", "Box", []),
+    createInstance("child2", "Box", [createInstance("descendant", "Box", [])]),
+    createInstance("child3", "Box", []),
   ]);
   const { clonedInstance, clonedInstanceIds } = cloneInstance(instance);
   expect(clonedInstance).toEqual(
-    createInstance(expectString, [
-      createInstance(expectString, []),
-      createInstance(expectString, [createInstance(expectString, [])]),
-      createInstance(expectString, []),
+    createInstance(expectString, "Box", [
+      createInstance(expectString, "Box", []),
+      createInstance(expectString, "Box", [
+        createInstance(expectString, "Box", []),
+      ]),
+      createInstance(expectString, "Box", []),
     ])
   );
   expect(clonedInstanceIds.get(instance.id)).toEqual(clonedInstance.id);

apps/designer/app/shared/tree-utils.ts

@@ -8,6 +8,7 @@ import type {
   StyleSources,
   StyleSourceSelections,
 } from "@webstudio-is/project-build";
+import { getComponentMeta } from "@webstudio-is/react-sdk";
 
 const traverseInstances = (
   instance: Instance,
@@ -24,6 +25,80 @@ const traverseInstances = (
   }
 };
 
+type InstancesIndex = {
+  rootInstanceId: undefined | Instance["id"];
+  instancesById: Map<Instance["id"], Instance>;
+  parentInstancesById: Map<Instance["id"], Instance>;
+};
+
+export const createInstancesIndex = (
+  rootInstance: undefined | Instance
+): InstancesIndex => {
+  const instancesById = new Map<Instance["id"], Instance>();
+  const parentInstancesById = new Map<Instance["id"], Instance>();
+  if (rootInstance) {
+    // traverse skips root without parent
+    instancesById.set(rootInstance.id, rootInstance);
+    traverseInstances(rootInstance, (child, parent) => {
+      parentInstancesById.set(child.id, parent);
+      instancesById.set(child.id, child);
+    });
+  }
+  return {
+    rootInstanceId: rootInstance?.id,
+    instancesById,
+    parentInstancesById,
+  };
+};
+
+const isInstanceDroppable = (instance: Instance) => {
+  const meta = getComponentMeta(instance.component);
+  return meta?.type === "body" || meta?.type === "container";
+};
+
+type DroppableTarget = {
+  parentId: Instance["id"];
+  position: number;
+};
+
+export const findClosestDroppableTarget = (
+  instancesIndex: InstancesIndex,
+  instanceId: undefined | Instance["id"]
+): undefined | DroppableTarget => {
+  const { instancesById, parentInstancesById } = instancesIndex;
+
+  if (instancesIndex.rootInstanceId === undefined) {
+    return;
+  }
+  // fallback to root instance
+  let droppableInstance = instancesById.get(
+    instanceId ?? instancesIndex.rootInstanceId
+  );
+  if (droppableInstance === undefined) {
+    return;
+  }
+  let position = -1;
+  while (isInstanceDroppable(droppableInstance) === false) {
+    const parentInstance = parentInstancesById.get(droppableInstance.id);
+    if (parentInstance === undefined) {
+      break;
+    }
+    // source of lookup in children
+    const sourceInstanceId = droppableInstance.id;
+    droppableInstance = parentInstance;
+    position = droppableInstance.children.findIndex(
+      (child) => child.type === "instance" && child.id === sourceInstanceId
+    );
+  }
+
+  return {
+    parentId: droppableInstance.id,
+    // put in the end when no position provided
+    position:
+      position === -1 ? droppableInstance.children.length : position + 1,
+  };
+};
+
 export const findSubtree = (
   rootInstance: Instance,
   targetInstanceId: Instance["id"]

packages/project/src/shared/tree-utils/insert-instance.ts

@@ -8,8 +8,11 @@ export type InstanceInsertionSpec = {
 export const insertInstanceMutable = (
   rootInstance: Instance,
   instance: Instance,
-  spec: InstanceInsertionSpec
+  spec: undefined | InstanceInsertionSpec
 ): boolean => {
+  if (spec === undefined) {
+    return false;
+  }
   if (spec.parentId !== rootInstance.id) {
     for (const child of rootInstance.children) {
       if (child.type === "text") {