diff --git a/apps/builder/app/builder/features/settings-panel/variables-section.tsx b/apps/builder/app/builder/features/settings-panel/variables-section.tsx index 446b6a852bf1..a9e5c4e55dc0 100644 --- a/apps/builder/app/builder/features/settings-panel/variables-section.tsx +++ b/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 ( @@ -213,6 +203,7 @@ const VariablesItem = ({ }) => { const [inspectDialogOpen, setInspectDialogOpen] = useState(false); const [isMenuOpen, setIsMenuOpen] = useState(false); + const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false); return ( {undefined} + {/* a11y is completely broken here @@ -263,17 +255,53 @@ const VariablesItem = ({ setInspectDialogOpen(true)}> Inspect - {source === "local" && ( + {source === "local" && variable.type !== "parameter" && ( 0} - onSelect={() => deleteVariable(variable.id)} + onSelect={() => { + if (usageCount > 0) { + setIsDeleteDialogOpen(true); + } else { + updateWebstudioData((data) => { + deleteVariableMutable(data, variable.id); + }); + } + }} > Delete {usageCount > 0 && `(${usageCount} bindings)`} )} + + + + Delete Variable? + + Variable "{variable.name}" is used in {usageCount}  + {usageCount === 1 ? "expression" : "expressions"}. + + + + + + } /> diff --git a/apps/builder/app/shared/data-variables.test.tsx b/apps/builder/app/shared/data-variables.test.tsx index 096e99dbd296..b314641d38a7 100644 --- a/apps/builder/app/shared/data-variables.test.tsx +++ b/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}`} + + + ); + 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} + > + + ); + 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", + }), + ]); +}); diff --git a/apps/builder/app/shared/data-variables.ts b/apps/builder/app/shared/data-variables.ts index b9449e779433..7d3f275a35f9 100644 --- a/apps/builder/app/shared/data-variables.ts +++ b/apps/builder/app/shared/data-variables.ts @@ -1,10 +1,12 @@ import { type DataSource, type DataSources, + Instance, type Instances, type Props, Resource, type Resources, + type WebstudioData, decodeDataVariableId, encodeDataVariableId, findTreeInstanceIdsExcludingSlotDescendants, @@ -201,49 +203,37 @@ export const findMaskedVariables = ({ return maskedVariables; }; -export const findUnsetVariableNames = ({ - instancePath, +const traverseExpressions = ({ + startingInstanceId, instances, props, dataSources, resources, + update, }: { - instancePath: InstancePath; + startingInstanceId: Instance["id"]; instances: Instances; props: Props; dataSources: DataSources; resources: Resources; + update: (expression: string, args?: string[]) => void | string; }) => { - const unsetVariables = new Set(); - const [{ instance: startingInstance }] = instancePath; const instanceIds = findTreeInstanceIdsExcludingSlotDescendants( instances, - startingInstance.id + startingInstanceId ); const resourceIds = new Set(); - const collectUnsetVariables = ( - expression: string, - exclude: string[] = [] - ) => { - transpileExpression({ - expression, - replaceVariable: (identifier) => { - const id = decodeDataVariableId(identifier); - if (id === undefined && exclude.includes(identifier) === false) { - unsetVariables.add(decodeDataVariableName(identifier)); - } - }, - }); - }; - for (const instance of instances.values()) { if (instanceIds.has(instance.id) === false) { continue; } for (const child of instance.children) { if (child.type === "expression") { - collectUnsetVariables(child.value); + const newExpression = update(child.value); + if (newExpression !== undefined) { + child.value = newExpression; + } } } } @@ -253,12 +243,18 @@ export const findUnsetVariableNames = ({ continue; } if (prop.type === "expression") { - collectUnsetVariables(prop.value); + const newExpression = update(prop.value); + if (newExpression !== undefined) { + prop.value = newExpression; + } continue; } if (prop.type === "action") { for (const action of prop.value) { - collectUnsetVariables(action.code, action.args); + const newExpression = update(action.code, action.args); + if (newExpression !== undefined) { + action.code = newExpression; + } } continue; } @@ -281,14 +277,58 @@ export const findUnsetVariableNames = ({ if (resourceIds.has(resource.id) === false) { continue; } - collectUnsetVariables(resource.url); + const newExpression = update(resource.url); + if (newExpression !== undefined) { + resource.url = newExpression; + } for (const header of resource.headers) { - collectUnsetVariables(header.value); + const newExpression = update(header.value); + if (newExpression !== undefined) { + header.value = newExpression; + } } if (resource.body) { - collectUnsetVariables(resource.body); + const newExpression = update(resource.body); + if (newExpression !== undefined) { + resource.body = newExpression; + } } } +}; + +export const findUnsetVariableNames = ({ + instancePath, + instances, + props, + dataSources, + resources, +}: { + instancePath: InstancePath; + instances: Instances; + props: Props; + dataSources: DataSources; + resources: Resources; +}) => { + const unsetVariables = new Set(); + const [{ instance: startingInstance }] = instancePath; + traverseExpressions({ + startingInstanceId: startingInstance.id, + instances, + props, + dataSources, + resources, + update: (expression, args = []) => { + transpileExpression({ + expression, + replaceVariable: (identifier) => { + const id = decodeDataVariableId(identifier); + if (id === undefined && args.includes(identifier) === false) { + unsetVariables.add(decodeDataVariableName(identifier)); + } + }, + }); + }, + }); return Array.from(unsetVariables); }; @@ -305,114 +345,62 @@ export const rebindTreeVariablesMutable = ({ dataSources: DataSources; resources: Resources; }) => { - const maskedIdByName = findMaskedVariables({ instancePath, dataSources }); + const maskedVariables = findMaskedVariables({ instancePath, dataSources }); const unsetNameById = new Map(); for (const { id, name } of dataSources.values()) { unsetNameById.set(id, name); } const [{ instance: startingInstance }] = instancePath; - const instanceIds = findTreeInstanceIdsExcludingSlotDescendants( + traverseExpressions({ + startingInstanceId: startingInstance.id, instances, - startingInstance.id - ); - const resourceIds = new Set(); - - for (const instance of instances.values()) { - if (instanceIds.has(instance.id) === false) { - continue; - } - for (const child of instance.children) { - if (child.type === "expression") { - child.value = unsetExpressionVariables({ - expression: child.value, - unsetNameById, - }); - child.value = restoreExpressionVariables({ - expression: child.value, - maskedIdByName, - }); + props, + dataSources, + resources, + update: (expression, args) => { + let maskedIdByName = new Map(maskedVariables); + if (args) { + maskedIdByName = new Map(maskedIdByName); + for (const arg of args) { + maskedIdByName.delete(arg); + } } - } - } - - for (const prop of props.values()) { - if (instanceIds.has(prop.instanceId) === false) { - continue; - } - if (prop.type === "expression") { - prop.value = unsetExpressionVariables({ - expression: prop.value, + expression = unsetExpressionVariables({ + expression, unsetNameById, }); - prop.value = restoreExpressionVariables({ - expression: prop.value, + expression = restoreExpressionVariables({ + expression, maskedIdByName, }); - continue; - } - if (prop.type === "action") { - for (const action of prop.value) { - const maskedVariablesWithoutArgs = new Map(maskedIdByName); - for (const arg of action.args) { - maskedVariablesWithoutArgs.delete(arg); - } - action.code = unsetExpressionVariables({ - expression: action.code, - unsetNameById, - }); - action.code = restoreExpressionVariables({ - expression: action.code, - maskedIdByName: maskedVariablesWithoutArgs, - }); - } - continue; - } - if (prop.type === "resource") { - resourceIds.add(prop.value); - continue; - } - } + return expression; + }, + }); +}; - for (const dataSource of dataSources.values()) { - if ( - instanceIds.has(dataSource.scopeInstanceId) && - dataSource.type === "resource" - ) { - resourceIds.add(dataSource.resourceId); - } +export const deleteVariableMutable = ( + data: Omit, + variableId: DataSource["id"] +) => { + const dataSource = data.dataSources.get(variableId); + if (dataSource === undefined) { + return; } - - for (const resource of resources.values()) { - if (resourceIds.has(resource.id) === false) { - continue; - } - resource.url = unsetExpressionVariables({ - expression: resource.url, - unsetNameById, - }); - resource.url = restoreExpressionVariables({ - expression: resource.url, - maskedIdByName, - }); - for (const header of resource.headers) { - header.value = unsetExpressionVariables({ - expression: header.value, - unsetNameById, - }); - header.value = restoreExpressionVariables({ - expression: header.value, - maskedIdByName, - }); - } - if (resource.body) { - resource.body = unsetExpressionVariables({ - expression: resource.body, + data.dataSources.delete(variableId); + if (dataSource.type === "resource") { + data.resources.delete(dataSource.resourceId); + } + const unsetNameById = new Map(); + unsetNameById.set(dataSource.id, dataSource.name); + // unset deleted variable in expressions + traverseExpressions({ + ...data, + startingInstanceId: dataSource.scopeInstanceId, + update: (expression) => { + return unsetExpressionVariables({ + expression, unsetNameById, }); - resource.body = restoreExpressionVariables({ - expression: resource.body, - maskedIdByName, - }); - } - } + }, + }); };