feat: allow deleting variables used in expressions

Ref #4768

Before we disabled deleting variables which are used in expressions.
Now we will ask confirmation and unset this variable. This way user will
not be locked if variable is lost and we will show diagnostics (in the
future) where all unset variables are.

Author: TrySound

apps/builder/app/builder/features/settings-panel/variables-section.tsx

@@ -6,6 +6,12 @@ import {
   css,
   CssValueListArrowFocus,
   CssValueListItem,
+  Dialog,
+  DialogActions,
+  DialogClose,
+  DialogContent,
+  DialogDescription,
+  DialogTitle,
   DropdownMenu,
   DropdownMenuContent,
   DropdownMenuItem,
@@ -32,7 +38,6 @@ import {
   $resources,
   $variableValuesByInstanceSelector,
 } from "~/shared/nano-states";
-import { serverSyncStore } from "~/shared/sync";
 import {
   CollapsibleSectionRoot,
   useOpenState,
@@ -51,6 +56,8 @@ import {
   $selectedInstancePath,
   $selectedPage,
 } from "~/shared/awareness";
+import { updateWebstudioData } from "~/shared/instance-utils";
+import { deleteVariableMutable } from "~/shared/data-variables";
 
 /**
  * find variables defined specifically on this selected instance
@@ -157,22 +164,6 @@ const $usedVariables = computed(
   }
 );
 
-const deleteVariable = (variableId: DataSource["id"]) => {
-  serverSyncStore.createTransaction(
-    [$dataSources, $resources],
-    (dataSources, resources) => {
-      const dataSource = dataSources.get(variableId);
-      if (dataSource === undefined) {
-        return;
-      }
-      dataSources.delete(variableId);
-      if (dataSource.type === "resource") {
-        resources.delete(dataSource.resourceId);
-      }
-    }
-  );
-};
-
 const EmptyVariables = () => {
   return (
     <Flex direction="column" gap="2">
@@ -213,6 +204,7 @@ const VariablesItem = ({
 }) => {
   const [inspectDialogOpen, setInspectDialogOpen] = useState(false);
   const [isMenuOpen, setIsMenuOpen] = useState(false);
+  const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false);
   return (
     <VariablePopoverTrigger key={variable.id} variable={variable}>
       <CssValueListItem
@@ -243,6 +235,7 @@ const VariablesItem = ({
             >
               {undefined}
             </ValuePreviewDialog>
+
             <DropdownMenu modal onOpenChange={setIsMenuOpen}>
               <DropdownMenuTrigger asChild>
                 {/* a11y is completely broken here
@@ -263,17 +256,52 @@ const VariablesItem = ({
                 <DropdownMenuItem onSelect={() => setInspectDialogOpen(true)}>
                   Inspect
                 </DropdownMenuItem>
-                {source === "local" && (
+                {source === "local" && variable.type !== "parameter" && (
                   <DropdownMenuItem
-                    // allow to delete only unused variables
-                    disabled={variable.type === "parameter" || usageCount > 0}
-                    onSelect={() => deleteVariable(variable.id)}
+                    onSelect={() => {
+                      if (usageCount > 0) {
+                        setIsDeleteDialogOpen(true);
+                      } else {
+                        updateWebstudioData((data) => {
+                          deleteVariableMutable(data, variable.id);
+                        });
+                      }
+                    }}
                   >
                     Delete {usageCount > 0 && `(${usageCount} bindings)`}
                   </DropdownMenuItem>
                 )}
               </DropdownMenuContent>
             </DropdownMenu>
+
+            <Dialog
+              open={isDeleteDialogOpen}
+              onOpenChange={setIsDeleteDialogOpen}
+            >
+              <DialogContent>
+                <DialogTitle>Do you want to delete this variable?</DialogTitle>
+                <DialogDescription
+                  className={css({
+                    paddingInline: theme.panel.paddingInline,
+                  }).toString()}
+                >
+                  This variable is used in {usageCount}{" "}
+                  {usageCount === 1 ? "expression" : "expressions"}.
+                </DialogDescription>
+                <DialogActions>
+                  <Button
+                    color="destructive"
+                    onClick={() => {
+                      updateWebstudioData((data) => {
+                        deleteVariableMutable(data, variable.id);
+                      });
+                    }}
+                  >
+                    Delete
+                  </Button>
+                </DialogActions>
+              </DialogContent>
+            </Dialog>
           </>
         }
       />

apps/builder/app/shared/data-variables.test.tsx

@@ -16,6 +16,7 @@ import {
   restoreExpressionVariables,
   rebindTreeVariablesMutable,
   unsetExpressionVariables,
+  deleteVariableMutable,
 } from "./data-variables";
 import { getInstancePath } from "./awareness";
 
@@ -390,3 +391,77 @@ test("rebind tree variables in resources", () => {
     }),
   ]);
 });
+
+test("delete variable and unset it in expressions", () => {
+  const bodyVariable = new Variable("bodyVariable", "one value of body");
+  const data = renderData(
+    <$.Body ws:id="bodyId" data-body-vars={expression`${bodyVariable}`}>
+      <$.Box
+        ws:id="boxId"
+        data-action={new ActionValue([], expression`${bodyVariable}`)}
+      >
+        {expression`${bodyVariable}`}
+      </$.Box>
+    </$.Body>
+  );
+  expect(Array.from(data.dataSources.values())).toEqual([
+    expect.objectContaining({ scopeInstanceId: "bodyId" }),
+  ]);
+  const [bodyVariableId] = data.dataSources.keys();
+  deleteVariableMutable(data, bodyVariableId);
+  expect(Array.from(data.props.values())).toEqual([
+    expect.objectContaining({ name: "data-body-vars", value: "bodyVariable" }),
+    expect.objectContaining({
+      name: "data-action",
+      value: [{ type: "execute", args: [], code: "bodyVariable" }],
+    }),
+  ]);
+  expect(data.instances.get("boxId")?.children).toEqual([
+    { type: "expression", value: "bodyVariable" },
+  ]);
+});
+
+test("delete variable and unset it in resources", () => {
+  const bodyVariable = new Variable("bodyVariable", "one value of body");
+  const resourceVariable = new ResourceValue("resourceVariable", {
+    url: expression`${bodyVariable}`,
+    method: "post",
+    headers: [{ name: "auth", value: expression`${bodyVariable}` }],
+    body: expression`${bodyVariable}`,
+  });
+  const resourceProp = new ResourceValue("resourceProp", {
+    url: expression`${bodyVariable}`,
+    method: "post",
+    headers: [{ name: "auth", value: expression`${bodyVariable}` }],
+    body: expression`${bodyVariable}`,
+  });
+  const data = renderData(
+    <$.Body ws:id="bodyId" data-body-vars={expression`${bodyVariable}`}>
+      <$.Box
+        ws:id="boxId"
+        data-box-vars={expression`${resourceVariable}`}
+        data-resource={resourceProp}
+      ></$.Box>
+    </$.Body>
+  );
+  expect(Array.from(data.dataSources.values())).toEqual([
+    expect.objectContaining({ scopeInstanceId: "bodyId" }),
+    expect.objectContaining({ scopeInstanceId: "boxId" }),
+  ]);
+  const [bodyVariableId] = data.dataSources.keys();
+  deleteVariableMutable(data, bodyVariableId);
+  expect(Array.from(data.resources.values())).toEqual([
+    expect.objectContaining({
+      url: "bodyVariable",
+      method: "post",
+      headers: [{ name: "auth", value: "bodyVariable" }],
+      body: "bodyVariable",
+    }),
+    expect.objectContaining({
+      url: "bodyVariable",
+      method: "post",
+      headers: [{ name: "auth", value: "bodyVariable" }],
+      body: "bodyVariable",
+    }),
+  ]);
+});