feat: allow saving expressions with unset variables

apps/builder/app/builder/features/pages/custom-metadata.tsx

@@ -13,11 +13,11 @@ import {
 import { TrashIcon, PlusIcon } from "@webstudio-is/icons";
 import { isFeatureEnabled } from "@webstudio-is/feature-flags";
 import { isLiteralExpression } from "@webstudio-is/sdk";
+import { computeExpression } from "~/shared/data-variables";
 import {
   BindingControl,
   BindingPopover,
 } from "~/builder/shared/binding-popover";
-import { computeExpression } from "~/shared/nano-states";
 import { $pageRootScope } from "./page-utils";
 
 type Meta = {

apps/builder/app/builder/features/pages/page-settings.tsx

@@ -74,7 +74,6 @@ import {
   $instances,
   $pages,
   $dataSources,
-  computeExpression,
   $dataSourceVariables,
   $publishedOrigin,
   $project,
@@ -112,6 +111,7 @@ import type { UserPlanFeatures } from "~/shared/db/user-plan-features.server";
 import { useUnmount } from "~/shared/hook-utils/use-mount";
 import { Card } from "../marketplace/card";
 import { selectInstance } from "~/shared/awareness";
+import { computeExpression } from "~/shared/data-variables";
 
 const fieldDefaultValues = {
   name: "Untitled",

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

@@ -442,6 +442,7 @@ BooleanForm.displayName = "BooleanForm";
 
 const validateJsonValue = (expression: string) => {
   const diagnostics = lintExpression({ expression });
+  // prevent saving with any message including unset variable
   return diagnostics.length > 0 ? "error" : "";
 };
 

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

@@ -220,8 +220,10 @@ const VariablesItem = ({
         id={variable.id}
         index={index}
         label={
-          <Flex align="center" css={{}}>
-            <Label color={source}>{variable.name}</Label>
+          <Flex align="center">
+            <Label tag="label" color={source}>
+              {variable.name}
+            </Label>
             {value !== undefined && (
               <span className={variableLabelStyle.toString()}>
                 &nbsp;

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

@@ -38,16 +38,13 @@ import {
   getExpressionIdentifiers,
   lintExpression,
 } from "@webstudio-is/sdk";
+import { $dataSourceVariables, $isDesignMode } from "~/shared/nano-states";
+import { computeExpression } from "~/shared/data-variables";
 import {
   ExpressionEditor,
   formatValuePreview,
   type EditorApi,
 } from "./expression-editor";
-import {
-  $dataSourceVariables,
-  $isDesignMode,
-  computeExpression,
-} from "~/shared/nano-states";
 
 export const evaluateExpressionWithinScope = (
   expression: string,
@@ -94,7 +91,9 @@ const BindingPanel = ({
       expression,
       availableVariables: new Set(aliases.keys()),
     });
-    setErrorsCount(diagnostics.length);
+    // prevent saving expression only with syntax error
+    const errors = diagnostics.filter((item) => item.severity === "error");
+    setErrorsCount(errors.length);
   };
 
   const updateExpression = (newExpression: string) => {

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

@@ -122,6 +122,10 @@ const editorContentStyle = css({
     textDecoration: "underline wavy red",
     backgroundColor: "rgba(255, 0, 0, 0.1)",
   },
+  ".cm-lintRange-warning": {
+    textDecoration: "underline wavy orange",
+    backgroundColor: "rgba(255, 0, 0, 0.1)",
+  },
   ".cm-gutters": {
     backgroundColor: "transparent",
     border: 0,

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

@@ -1,5 +1,6 @@
-import { expect, test } from "vitest";
+import { expect, test, vi } from "vitest";
 import {
+  computeExpression,
   decodeDataVariableName,
   encodeDataVariableName,
   restoreExpressionVariables,
@@ -56,3 +57,59 @@ test("restore expression variables", () => {
     })
   ).toEqual("$ws$dataSource$myId + missingVariable");
 });
+
+test("compute expression with decoded ids", () => {
+  expect(
+    computeExpression("$ws$dataSource$myId", new Map([["myId", "value"]]))
+  ).toEqual("value");
+});
+
+test("compute expression with decoded names", () => {
+  expect(
+    computeExpression("My$32$Name", new Map([["My Name", "value"]]))
+  ).toEqual("value");
+});
+
+test("compute expression when invalid syntax", () => {
+  // prevent error message in test report
+  const spy = vi.spyOn(console, "error");
+  spy.mockImplementationOnce(() => {});
+  expect(computeExpression("https://github.com", new Map())).toEqual(undefined);
+  expect(spy).toHaveBeenCalledOnce();
+  spy.mockRestore();
+});
+
+test("compute expression with nested field of undefined without error", () => {
+  const spy = vi.spyOn(console, "error");
+  const variables = new Map([["myVariable", undefined]]);
+  expect(computeExpression("myVariable.field", variables)).toEqual(undefined);
+  expect(spy).not.toHaveBeenCalled();
+  spy.mockRestore();
+});
+
+test("compute literal expression when variable is json object", () => {
+  const jsonObject = { hello: "world", subObject: { world: "hello" } };
+  const variables = new Map([["jsonVariable", jsonObject]]);
+  expect(computeExpression("`${jsonVariable}`", variables)).toEqual(
+    `{"hello":"world","subObject":{"world":"hello"}}`
+  );
+  expect(computeExpression("`${jsonVariable.subObject}`", variables)).toEqual(
+    `{"world":"hello"}`
+  );
+});
+
+test("compute literal expression when object is frozen", () => {
+  const jsonObject = Object.freeze({
+    hello: "world",
+    subObject: { world: "hello" },
+  });
+  const variables = new Map([["jsonVariable", jsonObject]]);
+  expect(computeExpression("`${jsonVariable.subObject}`", variables)).toEqual(
+    `{"world":"hello"}`
+  );
+});
+
+test("compute unset variables as undefined", () => {
+  expect(computeExpression(`a`, new Map())).toEqual(undefined);
+  expect(computeExpression("`${a}`", new Map())).toEqual("undefined");
+});

apps/builder/app/shared/data-variables.ts

@@ -4,6 +4,10 @@ import {
   encodeDataVariableId,
   transpileExpression,
 } from "@webstudio-is/sdk";
+import {
+  createJsonStringifyProxy,
+  isPlainObject,
+} from "@webstudio-is/sdk/runtime";
 
 const allowedJsChars = /[A-Za-z_]/;
 
@@ -92,3 +96,80 @@ export const restoreExpressionVariables = ({
     return expression;
   }
 };
+
+export const computeExpression = (
+  expression: string,
+  variables: Map<DataSource["name"], unknown>
+) => {
+  try {
+    const usedVariables = new Map();
+    const transpiled = transpileExpression({
+      expression,
+      executable: true,
+      replaceVariable: (identifier) => {
+        const id = decodeDataVariableId(identifier);
+        if (id) {
+          usedVariables.set(identifier, id);
+        } else {
+          // access all variable values from specified map
+          const name = decodeDataVariableName(identifier);
+          usedVariables.set(identifier, name);
+        }
+      },
+    });
+    let code = "";
+    // add only used variables in expression and get values
+    // from variables map without additional serializing of these values
+    for (const [identifier, name] of usedVariables) {
+      code += `let ${identifier} = _variables.get(${JSON.stringify(name)});\n`;
+    }
+    code += `return (${transpiled})`;
+
+    /**
+     *
+     * We are using structuredClone on frozen values because, for some reason,
+     * the Proxy example below throws a cryptic error:
+     * TypeError: 'get' on proxy: property 'data' is a read-only and non-configurable
+     * data property on the proxy target, but the proxy did not return its actual value
+     * (expected '[object Array]' but got '[object Array]').
+     *
+     * ```
+     * const createJsonStringifyProxy = (target) => {
+     *   return new Proxy(target, {
+     *     get(target, prop, receiver) {
+     *
+     *       console.log((prop in target), prop)
+     *
+     *       const value = Reflect.get(target, prop, receiver);
+     *
+     *       if (typeof value === "object" && value !== null) {
+     *         return createJsonStringifyProxy(value);
+     *       }
+     *
+     *       return value;
+     *     },
+     *   });
+     * };
+     * const obj = Object.freeze({ data: [1, 2, 3, 4] });
+     * const proxy = createJsonStringifyProxy(obj)
+     * proxy.data
+     *
+     * ```
+     */
+    const proxiedVariables = new Map(
+      [...variables.entries()].map(([key, value]) => [
+        key,
+        isPlainObject(value)
+          ? createJsonStringifyProxy(
+              Object.isFrozen(value) ? structuredClone(value) : value
+            )
+          : value,
+      ])
+    );
+
+    const result = new Function("_variables", code)(proxiedVariables);
+    return result;
+  } catch (error) {
+    console.error(error);
+  }
+};

apps/builder/app/shared/nano-states/props.test.ts

@@ -2,17 +2,12 @@ import { beforeEach, expect, test } from "vitest";
 import { cleanStores } from "nanostores";
 import { createDefaultPages } from "@webstudio-is/project-build";
 import { setEnv } from "@webstudio-is/feature-flags";
-import {
-  type Instance,
-  encodeDataSourceVariable,
-  collectionComponent,
-} from "@webstudio-is/sdk";
+import { type Instance, collectionComponent } from "@webstudio-is/sdk";
 import { textContentAttribute } from "@webstudio-is/react-sdk";
 import { $instances } from "./instances";
 import {
   $propValuesByInstanceSelector,
   $variableValuesByInstanceSelector,
-  computeExpression,
 } from "./props";
 import { $pages } from "./pages";
 import { $assets, $dataSources, $props, $resources } from "./misc";
@@ -978,24 +973,3 @@ test("prefill default system variable value", () => {
 
   cleanStores($variableValuesByInstanceSelector);
 });
-
-test("compute expression when invalid syntax", () => {
-  expect(computeExpression("https://github.com", new Map())).toEqual(undefined);
-});
-
-test("compute literal expression when variable is json object", () => {
-  const variableName = "jsonVariable";
-  const encVariableName = encodeDataSourceVariable(variableName);
-  const jsonObject = { hello: "world", subObject: { world: "hello" } };
-  const variables = new Map([[variableName, jsonObject]]);
-  const expression = "`${" + encVariableName + "}`";
-
-  expect(computeExpression(expression, variables)).toEqual(
-    JSON.stringify(jsonObject)
-  );
-
-  const subObjectExpression = "`${" + encVariableName + ".subObject}`";
-  expect(computeExpression(subObjectExpression, variables)).toEqual(
-    JSON.stringify(jsonObject.subObject)
-  );
-});

apps/builder/app/shared/nano-states/props.ts

@@ -15,10 +15,6 @@ import {
   collectionComponent,
   portalComponent,
 } from "@webstudio-is/sdk";
-import {
-  createJsonStringifyProxy,
-  isPlainObject,
-} from "@webstudio-is/sdk/runtime";
 import { normalizeProps, textContentAttribute } from "@webstudio-is/react-sdk";
 import { isFeatureEnabled } from "@webstudio-is/feature-flags";
 import { mapGroupBy } from "~/shared/shim";
@@ -44,6 +40,7 @@ import {
 import { uploadingFileDataToAsset } from "~/builder/shared/assets/asset-utils";
 import { fetch } from "~/shared/fetch.client";
 import { $selectedPage, getInstanceKey } from "../awareness";
+import { computeExpression } from "../data-variables";
 
 export const assetBaseUrl = "/cgi/asset/";
 
@@ -204,79 +201,6 @@ const $loaderVariableValues = computed(
   }
 );
 
-export const computeExpression = (
-  expression: string,
-  variables: Map<DataSource["id"], unknown>
-) => {
-  try {
-    const usedVariables = new Map();
-    const transpiled = transpileExpression({
-      expression,
-      executable: true,
-      replaceVariable: (identifier) => {
-        const id = decodeDataSourceVariable(identifier);
-        if (id) {
-          usedVariables.set(identifier, id);
-        }
-      },
-    });
-    let code = "";
-    // add only used variables in expression and get values
-    // from variables map without additional serializing of these values
-    for (const [identifier, id] of usedVariables) {
-      code += `let ${identifier} = _variables.get("${id}");\n`;
-    }
-    code += `return (${transpiled})`;
-
-    /**
-     *
-     * We are using structuredClone on frozen values because, for some reason,
-     * the Proxy example below throws a cryptic error:
-     * TypeError: 'get' on proxy: property 'data' is a read-only and non-configurable
-     * data property on the proxy target, but the proxy did not return its actual value
-     * (expected '[object Array]' but got '[object Array]').
-     *
-     * ```
-     * const createJsonStringifyProxy = (target) => {
-     *   return new Proxy(target, {
-     *     get(target, prop, receiver) {
-     *
-     *       console.log((prop in target), prop)
-     *
-     *       const value = Reflect.get(target, prop, receiver);
-     *
-     *       if (typeof value === "object" && value !== null) {
-     *         return createJsonStringifyProxy(value);
-     *       }
-     *
-     *       return value;
-     *     },
-     *   });
-     * };
-     * const obj = Object.freeze({ data: [1, 2, 3, 4] });
-     * const proxy = createJsonStringifyProxy(obj)
-     * proxy.data
-     *
-     * ```
-     */
-    const proxiedVariables = new Map(
-      [...variables.entries()].map(([key, value]) => [
-        key,
-        isPlainObject(value)
-          ? createJsonStringifyProxy(
-              Object.isFrozen(value) ? structuredClone(value) : value
-            )
-          : value,
-      ])
-    );
-
-    const result = new Function("_variables", code)(proxiedVariables);
-    return result;
-  } catch (error) {
-    console.error(error);
-  }
-};
-
 /**
  * compute prop values within context of instance ancestors
  * like a dry-run of rendering and accessing react contexts deep in the tree