feat: allow deleting variables used in expressions (#4860)

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.

<img width="908" alt="Screenshot 2025-02-12 at 15 38 14"
src="https://github.com/user-attachments/assets/ea9eea58-c276-4251-8b95-1214e9380621"
/>

Author: TrySound

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

@@ -6,6 +6,11 @@ import {
   css,
   CssValueListArrowFocus,
   CssValueListItem,
+  Dialog,
+  DialogActions,
+  DialogContent,
+  DialogDescription,
+  DialogTitle,
   DropdownMenu,
   DropdownMenuContent,
   DropdownMenuItem,
@@ -32,7 +37,6 @@ import {
   $resources,
   $variableValuesByInstanceSelector,
 } from "~/shared/nano-states";
-import { serverSyncStore } from "~/shared/sync";
 import {
   CollapsibleSectionRoot,
   useOpenState,
@@ -51,6 +55,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 +163,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 +203,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 +234,7 @@ const VariablesItem = ({
             >
               {undefined}
             </ValuePreviewDialog>
+
             <DropdownMenu modal onOpenChange={setIsMenuOpen}>
               <DropdownMenuTrigger asChild>
                 {/* a11y is completely broken here
@@ -263,17 +255,53 @@ 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>Delete Variable?</DialogTitle>
+                <DialogDescription
+                  className={css({
+                    paddingInline: theme.panel.paddingInline,
+                    textWrap: "nowrap",
+                  }).toString()}
+                >
+                  Variable "{variable.name}" is used in {usageCount}&nbsp;
+                  {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",
+    }),
+  ]);
+});