[pull] main from webstudio-is:main

apps/builder/app/builder/features/settings-panel/variable-popover.tsx

@@ -303,15 +303,23 @@ const ParameterForm = forwardRef<
 });
 ParameterForm.displayName = "ParameterForm";
 
-const saveVariable = (variable: undefined | DataSource, formData: FormData) => {
+type ValueVariableType = Extract<
+  VariableType,
+  "string" | "number" | "boolean" | "json"
+>;
+
+const saveVariable = (
+  variable: undefined | DataSource,
+  type: ValueVariableType,
+  formData: FormData
+) => {
   const dataSourceId = variable?.id ?? nanoid();
   // preserve existing instance scope when edit
   const scopeInstanceId =
     variable?.scopeInstanceId ?? $selectedInstance.get()?.id;
   if (scopeInstanceId === undefined) {
     return;
   }
-  const type = z.string().parse(formData.get("type"));
   const name = z.string().parse(formData.get("name"));
   const value = z.string().nullable().parse(formData.get("value"));
   let variableValue: Extract<DataSource, { type: "variable" }>["value"];
@@ -349,13 +357,15 @@ const saveVariable = (variable: undefined | DataSource, formData: FormData) => {
 const useValuePanelRef = ({
   ref,
   variable,
+  type,
 }: {
   ref: Ref<undefined | PanelApi>;
   variable?: DataSource;
+  type: ValueVariableType;
 }) => {
   useImperativeHandle(ref, () => ({
     save: (formData) => {
-      saveVariable(variable, formData);
+      saveVariable(variable, type, formData);
     },
   }));
 };
@@ -369,7 +379,7 @@ const StringForm = forwardRef<
   }
 >(({ variable, value: unknownValue, onChange }, ref) => {
   const value = typeof unknownValue === "string" ? unknownValue : "";
-  useValuePanelRef({ ref, variable });
+  useValuePanelRef({ ref, variable, type: "string" });
   const valueId = useId();
   return (
     <Flex direction="column" css={{ gap: theme.spacing[3] }}>
@@ -429,7 +439,7 @@ const NumberForm = forwardRef<
     valueRef.current?.setCustomValidity(validateNumberValue(value));
     setValueError("");
   }, [value]);
-  useValuePanelRef({ ref, variable });
+  useValuePanelRef({ ref, variable, type: "number" });
   const valueId = useId();
   return (
     <>
@@ -465,7 +475,7 @@ const BooleanForm = forwardRef<
   }
 >(({ variable, value: unknownValue, onChange }, ref) => {
   const value = typeof unknownValue === "boolean" ? unknownValue : false;
-  useValuePanelRef({ ref, variable });
+  useValuePanelRef({ ref, variable, type: "boolean" });
   const valueId = useId();
   return (
     <>
@@ -515,7 +525,7 @@ const JsonForm = forwardRef<
     valueRef.current?.setCustomValidity(validateJsonValue(value));
     setValueError("");
   }, [value]);
-  useValuePanelRef({ ref, variable });
+  useValuePanelRef({ ref, variable, type: "json" });
   return (
     <>
       <input
@@ -728,6 +738,7 @@ const VariablePreview = ({
           css={{ position: "absolute", inset: 0 }}
         >
           <Button
+            type="button"
             color="neutral"
             disabled={hasPendingResources}
             onClick={onLoadData}
@@ -908,6 +919,7 @@ const VariablePopoverContent = ({
               variableType === "graphql-resource") && (
               <Tooltip content="Copy resource as cURL command" side="bottom">
                 <Button
+                  type="button"
                   aria-label="Copy resource as cURL command"
                   prefix={<CopyIcon />}
                   color="ghost"
@@ -920,6 +932,7 @@ const VariablePopoverContent = ({
               variableType === "system-resource") && (
               <Tooltip content="Refresh resource data" side="bottom">
                 <Button
+                  type="button"
                   aria-label="Refresh resource data"
                   prefix={<RefreshIcon />}
                   color="ghost"

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

@@ -107,7 +107,9 @@ const EmptyVariables = () => {
       </Flex>
       <Flex justify="center" align="center">
         <VariablePopoverTrigger>
-          <Button prefix={<PlusIcon />}>Create data variable</Button>
+          <Button type="button" prefix={<PlusIcon />}>
+            Create data variable
+          </Button>
         </VariablePopoverTrigger>
       </Flex>
     </Flex>
@@ -279,6 +281,7 @@ export const VariablesSection = () => {
           suffix={
             <VariablePopoverTrigger>
               <SectionTitleButton
+                type="button"
                 prefix={<PlusIcon />}
                 // open panel when add new varable
                 onClick={() => {

apps/builder/app/builder/shared/binding-popover.test.ts

@@ -13,3 +13,8 @@ test("evaluateExpressionWithinScope works", () => {
     })
   ).toEqual(2);
 });
+
+test("evaluateExpressionWithinScope treats empty expression as undefined", () => {
+  expect(evaluateExpressionWithinScope("", {})).toBeUndefined();
+  expect(evaluateExpressionWithinScope("  ", {})).toBeUndefined();
+});

apps/builder/app/builder/shared/binding-popover.tsx

@@ -46,6 +46,7 @@ import {
   formatValuePreview,
   type EditorApi,
 } from "./expression-editor";
+import { normalizeEditorValue } from "~/shared/code-editor-base";
 
 /**
  * Check if a value is a primitive that can be safely stringified.
@@ -78,6 +79,10 @@ export const evaluateExpressionWithinScope = (
   expression: string,
   scope: Record<string, unknown>
 ) => {
+  if (expression.trim() === "") {
+    return;
+  }
+
   const variables = new Map<string, unknown>();
   for (const [name, value] of Object.entries(scope)) {
     const decodedName = decodeDataSourceVariable(name);
@@ -100,15 +105,16 @@ const BindingPanel = ({
   scope: Record<string, unknown>;
   aliases: Map<string, string>;
   valueError?: string;
-  value: string;
+  value?: string;
   onChange: () => void;
   onSave: (value: string, invalid: boolean) => void;
 }) => {
   const editorApiRef = useRef<undefined | EditorApi>(undefined);
-  const [expression, setExpression] = useState(value);
+  const normalizedValue = normalizeEditorValue(value);
+  const [expression, setExpression] = useState(normalizedValue);
   const usedIdentifiers = useMemo(
-    () => getExpressionIdentifiers(value),
-    [value]
+    () => getExpressionIdentifiers(normalizedValue),
+    [normalizedValue]
   );
   const [errorsCount, setErrorsCount] = useState<number>(0);
   const [touched, setTouched] = useState(false);
@@ -372,7 +378,7 @@ export const BindingPopover = ({
   aliases: Map<string, string>;
   variant: BindingVariant;
   validate?: (value: unknown) => undefined | string;
-  value: string;
+  value?: string;
   onChange: (newValue: string) => void;
   onRemove: (evaluatedValue: unknown) => void;
 }) => {
@@ -385,7 +391,10 @@ export const BindingPopover = ({
     return;
   }
 
-  const valueError = validate?.(evaluateExpressionWithinScope(value, scope));
+  const normalizedValue = normalizeEditorValue(value);
+  const valueError = validate?.(
+    evaluateExpressionWithinScope(normalizedValue, scope)
+  );
   return (
     <FloatingPanel
       placement="left-start"
@@ -419,7 +428,7 @@ export const BindingPopover = ({
                       event.preventDefault();
                       // inline variables and close dialog
                       const evaluatedValue = evaluateExpressionWithinScope(
-                        value,
+                        normalizedValue,
                         scope
                       );
 
@@ -443,7 +452,7 @@ export const BindingPopover = ({
           scope={scope}
           aliases={aliases}
           valueError={valueError}
-          value={value}
+          value={normalizedValue}
           onChange={() => {
             hasUnsavedChange.current = true;
           }}
@@ -467,7 +476,11 @@ export const BindingPopover = ({
         />
       }
     >
-      <BindingButton variant={variant} error={valueError} value={value} />
+      <BindingButton
+        variant={variant}
+        error={valueError}
+        value={normalizedValue}
+      />
     </FloatingPanel>
   );
 };

apps/builder/app/builder/shared/expression-editor.test.ts

@@ -5,6 +5,7 @@ import { javascript } from "@codemirror/lang-javascript";
 import { allowedArrayMethods, allowedStringMethods } from "@webstudio-is/sdk";
 import {
   completionPath,
+  formatValue,
   generateCompletionOptions,
   getCompletionTarget,
   getCompletionReplacement,
@@ -75,6 +76,19 @@ const getCompletionPath = (value: string) => {
   return completionPath(new CompletionContext(state, value.length, true));
 };
 
+describe("formatValue", () => {
+  test("formats missing preview values as an empty editor string", () => {
+    expect(formatValue(undefined)).toEqual("");
+    expect(typeof formatValue(undefined)).toEqual("string");
+  });
+
+  test("always returns a string for editor values", () => {
+    expect(formatValue(() => {})).toEqual("");
+    expect(formatValue("text")).toEqual('"text"');
+    expect(formatValue(0)).toEqual("0");
+  });
+});
+
 describe("generateCompletionOptions", () => {
   describe("object properties", () => {
     test("returns object properties with preview values", () => {

apps/builder/app/builder/shared/expression-editor.tsx

@@ -40,6 +40,7 @@ import {
   EditorDialogButton,
   EditorDialogControl,
   type EditorApi,
+  normalizeEditorValue,
 } from "~/shared/code-editor-base";
 import {
   decodeDataVariableName,
@@ -50,8 +51,11 @@ import {
 
 export type { EditorApi };
 
-export const formatValue = (value: unknown) => {
+export const formatValue = (value: unknown): string => {
   try {
+    if (value === undefined) {
+      return "";
+    }
     if (Array.isArray(value)) {
       // format arrays as multiline
       return JSON.stringify(value, null, 2);
@@ -61,7 +65,7 @@ export const formatValue = (value: unknown) => {
       // syntax highlighting as expression instead of block
       return `(${JSON.stringify(value, null, 2)})`;
     }
-    return JSON.stringify(value);
+    return JSON.stringify(value) ?? "";
   } catch {
     // show nothing when value is invalid
     return "";
@@ -574,10 +578,11 @@ export const ExpressionEditor = ({
   color?: "error";
   autoFocus?: boolean;
   readOnly?: boolean;
-  value: string;
+  value?: string;
   onChange: (value: string) => void;
   onChangeComplete: (value: string) => void;
 }) => {
+  const normalizedValue = normalizeEditorValue(value);
   const { nameById, idByName } = useMemo(() => {
     const nameById = new Map();
     const idByName = new Map();
@@ -592,10 +597,10 @@ export const ExpressionEditor = ({
   }, [aliases]);
   const expressionWithUnsetVariables = useMemo(() => {
     return unsetExpressionVariables({
-      expression: value,
+      expression: normalizedValue,
       unsetNameById: nameById,
     });
-  }, [value, nameById]);
+  }, [normalizedValue, nameById]);
   const scopeWithUnsetVariables = useMemo(() => {
     const newScope: typeof scope = {};
     for (const [identifier, value] of Object.entries(scope)) {

apps/builder/app/shared/code-editor-base.test.ts

@@ -1,6 +1,6 @@
 import { EditorSelection } from "@codemirror/state";
 import { expect, test } from "vitest";
-import { clampEditorSelection } from "./code-editor-base";
+import { clampEditorSelection, normalizeEditorValue } from "./code-editor-base";
 
 test("clampEditorSelection preserves selection within document length", () => {
   const selection = EditorSelection.create([
@@ -35,3 +35,8 @@ test("clampEditorSelection clamps selection to document length", () => {
   ]);
   expect(clampedSelection.mainIndex).toBe(selection.mainIndex);
 });
+
+test("normalizeEditorValue defaults undefined to empty string", () => {
+  expect(normalizeEditorValue(undefined)).toBe("");
+  expect(normalizeEditorValue("value")).toBe("value");
+});

apps/builder/app/shared/code-editor-base.tsx

@@ -71,6 +71,8 @@ export const clampEditorSelection = (
   );
 };
 
+export const normalizeEditorValue = (value: undefined | string) => value ?? "";
+
 export const getCodeEditorCssVars = ({
   minHeight,
   maxHeight,
@@ -239,7 +241,7 @@ type EditorContentProps = {
   autoFocus?: boolean;
   invalid?: boolean;
   showShortcuts?: boolean;
-  value: string;
+  value?: string;
   onChange: (value: string) => void;
   onChangeComplete: (value: string) => void;
 };
@@ -385,15 +387,16 @@ export const EditorContent = ({
     if (view === undefined) {
       return;
     }
+    const nextValue = normalizeEditorValue(value);
     // prevent updating when editor has the same state
     // and can be the source of new value
-    if (value === view.state.doc.toString()) {
+    if (nextValue === view.state.doc.toString()) {
       return;
     }
 
     view.dispatch({
-      changes: { from: 0, to: view.state.doc.length, insert: value },
-      selection: clampEditorSelection(view.state.selection, value.length),
+      changes: { from: 0, to: view.state.doc.length, insert: nextValue },
+      selection: clampEditorSelection(view.state.selection, nextValue.length),
       annotations: [ExternalChange.of(true)],
     });
   }, [value]);