From c93c4ddac3c8fb7ee46c0b1d3b78cc3938885d2a Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Thu, 12 Jun 2025 15:03:13 +0300 Subject: [PATCH] feat: allow editing remove resources Did you ever wonder why you can't edit inherited data variables? We wondered the same. Now you can edit variables and resources from anywhere deep in the tree. --- .../controls/resource-control.tsx | 32 ++- .../settings-panel/resource-panel.tsx | 227 +++++++++++------- .../settings-panel/variable-popover.tsx | 116 ++++----- .../settings-panel/variables-section.tsx | 1 - 4 files changed, 217 insertions(+), 159 deletions(-) diff --git a/apps/builder/app/builder/features/settings-panel/controls/resource-control.tsx b/apps/builder/app/builder/features/settings-panel/controls/resource-control.tsx index 269ab9103030..aaf1cab21a7e 100644 --- a/apps/builder/app/builder/features/settings-panel/controls/resource-control.tsx +++ b/apps/builder/app/builder/features/settings-panel/controls/resource-control.tsx @@ -25,16 +25,25 @@ import { BindingPopover, type BindingVariant, } from "~/builder/shared/binding-popover"; -import { $props, $resources } from "~/shared/nano-states"; +import { + $dataSources, + $props, + $resources, + $variableValuesByInstanceSelector, +} from "~/shared/nano-states"; import { computeExpression } from "~/shared/data-variables"; import { updateWebstudioData } from "~/shared/instance-utils"; -import { $selectedInstance } from "~/shared/awareness"; import { - $selectedInstanceResourceScope, + $selectedInstance, + $selectedInstanceKeyWithRoot, + $selectedPage, +} from "~/shared/awareness"; +import { UrlField, MethodField, Headers, parseResource, + getResourceScopeForInstance, } from "../resource-panel"; import { type ControlProps, useLocalValue, VerticalLayout } from "../shared"; import { PropertyLabel } from "../property-label"; @@ -77,6 +86,23 @@ const ResourceButton = forwardRef< }); ResourceButton.displayName = "ResourceButton"; +const $selectedInstanceResourceScope = computed( + [ + $selectedPage, + $selectedInstanceKeyWithRoot, + $variableValuesByInstanceSelector, + $dataSources, + ], + (page, instanceKey, variableValuesByInstanceSelector, dataSources) => { + return getResourceScopeForInstance({ + page, + instanceKey, + dataSources, + variableValuesByInstanceSelector, + }); + } +); + const ResourceForm = ({ resource }: { resource: Resource }) => { const { scope, aliases } = useStore($selectedInstanceResourceScope); const [url, setUrl] = useState(resource.url); diff --git a/apps/builder/app/builder/features/settings-panel/resource-panel.tsx b/apps/builder/app/builder/features/settings-panel/resource-panel.tsx index de254a6688b7..df295ea67bc1 100644 --- a/apps/builder/app/builder/features/settings-panel/resource-panel.tsx +++ b/apps/builder/app/builder/features/settings-panel/resource-panel.tsx @@ -11,7 +11,12 @@ import { useState, } from "react"; import { useStore } from "@nanostores/react"; -import { Resource, type DataSource } from "@webstudio-is/sdk"; +import { + DataSources, + Resource, + type DataSource, + type Page, +} from "@webstudio-is/sdk"; import { encodeDataVariableId, generateObjectExpression, @@ -53,14 +58,16 @@ import { EditorDialogButton, EditorDialogControl, } from "~/builder/shared/code-editor-base"; -import { parseCurl, type CurlRequest } from "./curl"; import { $selectedInstance, - $selectedInstanceKeyWithRoot, + $selectedInstancePathWithRoot, $selectedPage, + getInstanceKey, + type InstancePath, } from "~/shared/awareness"; import { updateWebstudioData } from "~/shared/instance-utils"; import { rebindTreeVariablesMutable } from "~/shared/data-variables"; +import { parseCurl, type CurlRequest } from "./curl"; export const parseResource = ({ id, @@ -408,85 +415,114 @@ export const Headers = ({ ); }; -const $hiddenDataSourceIds = computed( - [$dataSources, $selectedPage], - (dataSources, page) => { - const dataSourceIds = new Set(); - for (const dataSource of dataSources.values()) { - // hide collection item and component parameters from resources - // to prevent waterfall and loop requests ans not complicate compiler - if (dataSource.type === "parameter") { - dataSourceIds.add(dataSource.id); - } - // prevent resources using data of other resources - if (dataSource.type === "resource") { - dataSourceIds.add(dataSource.id); - } +export const getResourceScopeForInstance = ({ + page, + instanceKey, + dataSources, + variableValuesByInstanceSelector, +}: { + page: undefined | Page; + instanceKey: undefined | string; + dataSources: DataSources; + variableValuesByInstanceSelector: Map>; +}) => { + const scope: Record = {}; + const aliases = new Map(); + const variableValues = new Map(); + const hiddenDataSourceIds = new Set(); + for (const dataSource of dataSources.values()) { + // hide collection item and component parameters from resources + // to prevent waterfall and loop requests ans not complicate compiler + if (dataSource.type === "parameter") { + hiddenDataSourceIds.add(dataSource.id); } - if (page?.systemDataSourceId) { - dataSourceIds.delete(page.systemDataSourceId); + // prevent resources using data of other resources + if (dataSource.type === "resource") { + hiddenDataSourceIds.add(dataSource.id); } - return dataSourceIds; } -); - -export const $selectedInstanceResourceScope = computed( - [ - $selectedInstanceKeyWithRoot, - $variableValuesByInstanceSelector, - $dataSources, - $hiddenDataSourceIds, - ], - ( - instanceKey, - variableValuesByInstanceSelector, - dataSources, - hiddenDataSourceIds - ) => { - const scope: Record = {}; - const aliases = new Map(); - const variableValues = new Map(); - if (instanceKey === undefined) { - return { variableValues, scope, aliases }; - } - const values = variableValuesByInstanceSelector.get(instanceKey); - if (values) { - for (const [dataSourceId, value] of values) { - if (hiddenDataSourceIds.has(dataSourceId)) { - continue; - } - let dataSource = dataSources.get(dataSourceId); - if (dataSourceId === SYSTEM_VARIABLE_ID) { - dataSource = systemParameter; - } - if (dataSource) { - const name = encodeDataVariableId(dataSourceId); - variableValues.set(dataSourceId, value); - scope[name] = value; - aliases.set(name, dataSource.name); - } + if (page?.systemDataSourceId) { + hiddenDataSourceIds.delete(page.systemDataSourceId); + } + const values = variableValuesByInstanceSelector.get(instanceKey ?? ""); + if (values) { + for (const [dataSourceId, value] of values) { + if (hiddenDataSourceIds.has(dataSourceId)) { + continue; + } + let dataSource = dataSources.get(dataSourceId); + if (dataSourceId === SYSTEM_VARIABLE_ID) { + dataSource = systemParameter; + } + if (dataSource) { + const name = encodeDataVariableId(dataSourceId); + variableValues.set(dataSourceId, value); + scope[name] = value; + aliases.set(name, dataSource.name); } } - return { variableValues, scope, aliases }; } -); + return { variableValues, scope, aliases }; +}; + +const getVariableInstanceKey = ({ + variable, + instancePath, +}: { + variable: undefined | DataSource; + instancePath: undefined | InstancePath; +}) => { + if (instancePath === undefined) { + return; + } + // find instance key for variable instance + for (const { instance, instanceSelector } of instancePath) { + if (instance.id === variable?.scopeInstanceId) { + return getInstanceKey(instanceSelector); + } + } + // and fallback to currently selected instance + return getInstanceKey(instancePath[0].instanceSelector); +}; const useScope = ({ variable }: { variable?: DataSource }) => { - const { scope: scopeWithCurrentVariable, aliases } = useStore( - $selectedInstanceResourceScope + return useStore( + useMemo( + () => + computed( + [ + $selectedPage, + $selectedInstancePathWithRoot, + $variableValuesByInstanceSelector, + $dataSources, + ], + ( + page, + instancePath, + variableValuesByInstanceSelector, + dataSources + ) => { + const { scope, aliases } = getResourceScopeForInstance({ + page, + instanceKey: getVariableInstanceKey({ + variable, + instancePath, + }), + dataSources, + variableValuesByInstanceSelector, + }); + // prevent showing currently edited variable in suggestions + // to avoid cirular dependeny + const newScope = { ...scope }; + if (variable) { + delete newScope[encodeDataVariableId(variable.id)]; + } + return { scope: newScope, aliases }; + } + ), + [variable] + ) ); - const currentVariableId = variable?.id; - // prevent showing currently edited variable in suggestions - // to avoid cirular dependeny - const scope = useMemo(() => { - if (currentVariableId === undefined) { - return scopeWithCurrentVariable; - } - const newScope: Record = { ...scopeWithCurrentVariable }; - delete newScope[encodeDataVariableId(currentVariableId)]; - return newScope; - }, [scopeWithCurrentVariable, currentVariableId]); - return { scope, aliases }; }; type PanelApi = { @@ -635,8 +671,10 @@ export const ResourceForm = forwardRef< useImperativeHandle(ref, () => ({ save: (formData) => { - const selectedInstance = $selectedInstance.get(); - if (selectedInstance === undefined) { + // preserve existing instance scope when edit + const scopeInstanceId = + variable?.scopeInstanceId ?? $selectedInstance.get()?.id; + if (scopeInstanceId === undefined) { return; } const name = z.string().parse(formData.get("name")); @@ -647,8 +685,7 @@ export const ResourceForm = forwardRef< }); const newVariable: DataSource = { id: variable?.id ?? nanoid(), - // preserve existing instance scope when edit - scopeInstanceId: variable?.scopeInstanceId ?? selectedInstance.id, + scopeInstanceId, name, type: "resource", resourceId: newResource.id, @@ -656,8 +693,10 @@ export const ResourceForm = forwardRef< updateWebstudioData((data) => { data.dataSources.set(newVariable.id, newVariable); data.resources.set(newResource.id, newResource); - const startingInstanceId = selectedInstance.id; - rebindTreeVariablesMutable({ startingInstanceId, ...data }); + rebindTreeVariablesMutable({ + startingInstanceId: scopeInstanceId, + ...data, + }); }); }, })); @@ -756,8 +795,10 @@ export const SystemResourceForm = forwardRef< useImperativeHandle(ref, () => ({ save: (formData) => { - const selectedInstance = $selectedInstance.get(); - if (selectedInstance === undefined) { + // preserve existing instance scope when edit + const scopeInstanceId = + variable?.scopeInstanceId ?? $selectedInstance.get()?.id; + if (scopeInstanceId === undefined) { return; } const name = z.string().parse(formData.get("name")); @@ -771,8 +812,7 @@ export const SystemResourceForm = forwardRef< }; const newVariable: DataSource = { id: variable?.id ?? nanoid(), - // preserve existing instance scope when edit - scopeInstanceId: variable?.scopeInstanceId ?? selectedInstance.id, + scopeInstanceId, name, type: "resource", resourceId: newResource.id, @@ -780,8 +820,10 @@ export const SystemResourceForm = forwardRef< updateWebstudioData((data) => { data.dataSources.set(newVariable.id, newVariable); data.resources.set(newResource.id, newResource); - const startingInstanceId = selectedInstance.id; - rebindTreeVariablesMutable({ startingInstanceId, ...data }); + rebindTreeVariablesMutable({ + startingInstanceId: scopeInstanceId, + ...data, + }); }); }, })); @@ -865,8 +907,10 @@ export const GraphqlResourceForm = forwardRef< useImperativeHandle(ref, () => ({ save: (formData) => { - const selectedInstance = $selectedInstance.get(); - if (selectedInstance === undefined) { + // preserve existing instance scope when edit + const scopeInstanceId = + variable?.scopeInstanceId ?? $selectedInstance.get()?.id; + if (scopeInstanceId === undefined) { return; } const name = z.string().parse(formData.get("name")); @@ -887,8 +931,7 @@ export const GraphqlResourceForm = forwardRef< }; const newVariable: DataSource = { id: variable?.id ?? nanoid(), - // preserve existing instance scope when edit - scopeInstanceId: variable?.scopeInstanceId ?? selectedInstance.id, + scopeInstanceId, name, type: "resource", resourceId: newResource.id, @@ -896,8 +939,10 @@ export const GraphqlResourceForm = forwardRef< updateWebstudioData((data) => { data.dataSources.set(newVariable.id, newVariable); data.resources.set(newResource.id, newResource); - const startingInstanceId = selectedInstance.id; - rebindTreeVariablesMutable({ startingInstanceId, ...data }); + rebindTreeVariablesMutable({ + startingInstanceId: scopeInstanceId, + ...data, + }); }); }, })); diff --git a/apps/builder/app/builder/features/settings-panel/variable-popover.tsx b/apps/builder/app/builder/features/settings-panel/variable-popover.tsx index d3ae0c3ad803..287d94fa3add 100644 --- a/apps/builder/app/builder/features/settings-panel/variable-popover.tsx +++ b/apps/builder/app/builder/features/settings-panel/variable-popover.tsx @@ -1,6 +1,5 @@ import { z } from "zod"; import { nanoid } from "nanoid"; -import { computed } from "nanostores"; import { useStore } from "@nanostores/react"; import { type ReactNode, @@ -67,73 +66,49 @@ import { EditorDialogButton, EditorDialogControl, } from "~/builder/shared/code-editor-base"; +import { updateWebstudioData } from "~/shared/instance-utils"; +import { + findUnsetVariableNames, + rebindTreeVariablesMutable, +} from "~/shared/data-variables"; import { GraphqlResourceForm, ResourceForm, SystemResourceForm, } from "./resource-panel"; import { generateCurl } from "./curl"; -import { updateWebstudioData } from "~/shared/instance-utils"; -import { - findUnsetVariableNames, - rebindTreeVariablesMutable, -} from "~/shared/data-variables"; - -const $variablesByName = computed( - [$selectedInstance, $dataSources], - (instance, dataSources) => { - const variablesByName = new Map(); - for (const dataSource of dataSources.values()) { - if (dataSource.scopeInstanceId === instance?.id) { - variablesByName.set(dataSource.name, dataSource.id); - } - } - return variablesByName; - } -); - -const $unsetVariableNames = computed( - [$selectedInstance, $instances, $props, $dataSources, $resources], - (selectedInstance, instances, props, dataSources, resources) => { - if (selectedInstance === undefined) { - return []; - } - return findUnsetVariableNames({ - startingInstanceId: selectedInstance.id, - instances, - props, - dataSources, - resources, - }); - } -); const NameField = ({ - variableId, + variable, defaultValue, }: { - variableId: undefined | DataSource["id"]; + variable: undefined | DataSource; defaultValue: string; }) => { const ref = useRef(null); const [error, setError] = useState(""); const nameId = useId(); - const variablesByName = useStore($variablesByName); - const unsetVariableNames = useStore($unsetVariableNames); const validateName = useCallback( (value: string) => { - if ( - variablesByName.has(value) && - variablesByName.get(value) !== variableId - ) { - return "Name is already used by another variable on this instance"; + // validate same name on variable instance + // and fallback to selected instance for new variables + const scopeInstanceId = + variable?.scopeInstanceId ?? $selectedInstance.get(); + for (const dataSource of $dataSources.get().values()) { + if ( + dataSource.scopeInstanceId === scopeInstanceId && + dataSource.name === value && + dataSource.id !== variable?.id + ) { + return "Name is already used by another variable on this instance"; + } } if (value.trim().length === 0) { return "Name is required"; } return ""; }, - [variablesByName, variableId] + [variable] ); const [value, setValue] = useState(defaultValue); useEffect(() => { @@ -158,7 +133,22 @@ const NameField = ({ in expressions but not yet created )} - getItems={() => unsetVariableNames} + getItems={() => { + // find unset variables for variable instance + // and fallback to selected instance for new variables + const scopeInstanceId = + variable?.scopeInstanceId ?? $selectedInstance.get()?.id; + if (scopeInstanceId === undefined) { + return []; + } + return findUnsetVariableNames({ + startingInstanceId: scopeInstanceId, + instances: $instances.get(), + props: $props.get(), + dataSources: $dataSources.get(), + resources: $resources.get(), + }); + }} value={value} onItemSelect={(newValue) => { ref.current?.setCustomValidity(validateName(newValue)); @@ -289,19 +279,18 @@ const ParameterForm = forwardRef< >(({ variable }, ref) => { useImperativeHandle(ref, () => ({ save: (formData) => { - const selectedInstance = $selectedInstance.get(); - if (selectedInstance === undefined) { - return; - } // only existing parameter variables can be renamed - if (variable === undefined) { + if (variable?.scopeInstanceId === undefined) { return; } + const scopeInstanceId = variable.scopeInstanceId; const name = z.string().parse(formData.get("name")); updateWebstudioData((data) => { data.dataSources.set(variable.id, { ...variable, name }); - const startingInstanceId = selectedInstance.id; - rebindTreeVariablesMutable({ startingInstanceId, ...data }); + rebindTreeVariablesMutable({ + startingInstanceId: scopeInstanceId, + ...data, + }); }); }, })); @@ -320,13 +309,13 @@ const useValuePanelRef = ({ }) => { useImperativeHandle(ref, () => ({ save: (formData) => { - const selectedInstance = $selectedInstance.get(); - if (selectedInstance === undefined) { - return; - } const dataSourceId = variable?.id ?? nanoid(); // preserve existing instance scope when edit - const scopeInstanceId = variable?.scopeInstanceId ?? selectedInstance.id; + const scopeInstanceId = + variable?.scopeInstanceId ?? $selectedInstance.get()?.id; + if (scopeInstanceId === undefined) { + return; + } const name = z.string().parse(formData.get("name")); updateWebstudioData((data) => { // cleanup resource when value variable is set @@ -340,8 +329,10 @@ const useValuePanelRef = ({ type: "variable", value: variableValue, }); - const startingInstanceId = selectedInstance.id; - rebindTreeVariablesMutable({ startingInstanceId, ...data }); + rebindTreeVariablesMutable({ + startingInstanceId: scopeInstanceId, + ...data, + }); }); }, })); @@ -679,10 +670,7 @@ const VariablePanel = forwardRef< p: theme.panel.padding, }} > - + {fields} 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 3c808c83688b..90d7d54096ce 100644 --- a/apps/builder/app/builder/features/settings-panel/variables-section.tsx +++ b/apps/builder/app/builder/features/settings-panel/variables-section.tsx @@ -166,7 +166,6 @@ const VariablesItem = ({ )} } - disabled={source === "remote"} data-state={isMenuOpen ? "open" : undefined} buttons={ <>