refactor: compile prop expressions (#2569)

Ref https://github.com/webstudio-is/webstudio/issues/2532

Here changed accessing expression from data source id to prop id in
builder and data source name to prop name in generated code.

This is another step to migrate expressions from data sources to props.

See generated output for clarification.

Note: review with hidden spaces

apps/builder/app/builder/features/settings-panel/props-section/use-props-logic.ts

@@ -163,7 +163,8 @@ export const usePropsLogic = ({
     // convert data source prop to typed prop
     const dataSourceId = prop.value;
     const dataSource = dataSources.get(dataSourceId);
-    const dataSourceValue = dataSourcesLogic.get(dataSourceId);
+    const dataSourceValue =
+      dataSourcesLogic.get(prop.id) ?? dataSourcesLogic.get(dataSourceId);
     if (dataSource === undefined) {
       return [];
     }

apps/builder/app/canvas/features/webstudio-component/webstudio-component.tsx

@@ -165,7 +165,11 @@ const useInstanceProps = (instanceId: Instance["id"]) => {
           }
           if (prop.type === "dataSource") {
             const dataSourceId = prop.value;
-            const value = dataSourcesLogic.get(dataSourceId);
+            const value =
+              // access expression by prop id
+              dataSourcesLogic.get(prop.id) ??
+              // access variable by data source id
+              dataSourcesLogic.get(dataSourceId);
             if (value !== undefined) {
               instancePropsObject[prop.name] = value;
             }

apps/builder/app/shared/instance-utils.ts

@@ -697,7 +697,8 @@ export const getInstancesSlice = (rootInstanceId: string) => {
     if (prop.type === "dataSource") {
       const dataSourceId = prop.value;
       if (slicedDataSources.has(dataSourceId) === false) {
-        const value = dataSourcesLogic.get(dataSourceId);
+        const value =
+          dataSourcesLogic.get(prop.id) ?? dataSourcesLogic.get(dataSourceId);
         slicedProps.set(prop.id, {
           id: prop.id,
           instanceId: prop.instanceId,

fixtures/webstudio-remix-vercel/app/__generated__/[form]._index.tsx

@@ -73,20 +73,20 @@ export const projectId = "cddc1d44-af37-4cb6-a430-d300cf6f932d";
 const Page = (props: { scripts?: ReactNode }) => {
   let [formState, set$formState] = useState<any>("initial");
   let [formState_1, set$formState_1] = useState<any>("initial");
-  let formInitial = formState === "initial" || formState === "error";
-  let formSuccess = formState === "success";
-  let formError = formState === "error";
-  let formInitial_1 = formState_1 === "initial" || formState_1 === "error";
-  let formSuccess_1 = formState_1 === "success";
-  let formError_1 = formState_1 === "error";
   let onStateChange = (state: any) => {
     formState = state;
     set$formState(formState);
   };
+  let datawsshow = formState === "initial" || formState === "error";
+  let datawsshow_1 = formState === "success";
+  let datawsshow_2 = formState === "error";
   let onStateChange_1 = (state: any) => {
     formState_1 = state;
     set$formState_1(formState_1);
   };
+  let datawsshow_3 = formState_1 === "initial" || formState_1 === "error";
+  let datawsshow_4 = formState_1 === "success";
+  let datawsshow_5 = formState_1 === "error";
   return (
     <Body data-ws-id="a-4nDFkaWy4px1fn38XWJ" data-ws-component="Body">
       <Form
@@ -95,7 +95,7 @@ const Page = (props: { scripts?: ReactNode }) => {
         state={formState_1}
         onStateChange={onStateChange_1}
       >
-        {formInitial_1 && (
+        {datawsshow_3 && (
           <Box data-ws-id="qhnVrmYGlyrMZi3UzqSQA" data-ws-component="Box">
             <Heading
               data-ws-id="YdHHf4u3jrdbRIWpB_VfH"
@@ -128,12 +128,12 @@ const Page = (props: { scripts?: ReactNode }) => {
             </Button>
           </Box>
         )}
-        {formSuccess_1 && (
+        {datawsshow_4 && (
           <Box data-ws-id="966cjxuqP_T99N27-mqWE" data-ws-component="Box">
             {"Thank you for getting in touch!"}
           </Box>
         )}
-        {formError_1 && (
+        {datawsshow_5 && (
           <Box data-ws-id="SYG5hhOz31xFJUN_v9zq6" data-ws-component="Box">
             {"Sorry, something went wrong."}
           </Box>
@@ -147,7 +147,7 @@ const Page = (props: { scripts?: ReactNode }) => {
         method={"get"}
         action={"/custom"}
       >
-        {formInitial && (
+        {datawsshow && (
           <Box data-ws-id="a5YPRc19IJyhTrjjasA_R" data-ws-component="Box">
             <Heading
               data-ws-id="y4pceTmziuBRIDgUBQNLD"
@@ -180,12 +180,12 @@ const Page = (props: { scripts?: ReactNode }) => {
             </Button>
           </Box>
         )}
-        {formSuccess && (
+        {datawsshow_1 && (
           <Box data-ws-id="Gw-ta0R4FNFAGBTVRWKep" data-ws-component="Box">
             {"Thank you for getting in touch!"}
           </Box>
         )}
-        {formError && (
+        {datawsshow_2 && (
           <Box data-ws-id="ewk_WKpu4syHLPABMmvUz" data-ws-component="Box">
             {"Sorry, something went wrong."}
           </Box>

packages/react-sdk/src/component-generator.test.ts

@@ -288,7 +288,7 @@ test("generate jsx element with data sources and action", () => {
       data-ws-id="box"
       data-ws-component="Box"
       variable={variableName}
-      expression={expressionName}
+      expression={expression}
       onClick={onClick}
       onChange={onChange} />
     `)

packages/react-sdk/src/component-generator.ts

@@ -60,7 +60,12 @@ export const generateJsxElement = ({
         if (dataSource === undefined) {
           continue;
         }
-        conditionVariableName = scope.getName(dataSource.id, dataSource.name);
+        if (dataSource.type === "variable") {
+          conditionVariableName = scope.getName(dataSource.id, dataSource.name);
+        }
+        if (dataSource.type === "expression") {
+          conditionVariableName = scope.getName(prop.id, prop.name);
+        }
       }
       // ignore any other values
       continue;
@@ -84,8 +89,17 @@ export const generateJsxElement = ({
       if (dataSource === undefined) {
         continue;
       }
-      const dataSourceVariable = scope.getName(dataSource.id, dataSource.name);
-      generatedProps += `\n${prop.name}={${dataSourceVariable}}`;
+      if (dataSource.type === "variable") {
+        const dataSourceVariable = scope.getName(
+          dataSource.id,
+          dataSource.name
+        );
+        generatedProps += `\n${prop.name}={${dataSourceVariable}}`;
+      }
+      if (dataSource.type === "expression") {
+        const dataSourceVariable = scope.getName(prop.id, prop.name);
+        generatedProps += `\n${prop.name}={${dataSourceVariable}}`;
+      }
       continue;
     }
     if (prop.type === "action") {

packages/react-sdk/src/expression.test.ts

@@ -234,7 +234,7 @@ test("generate variables with actions", () => {
   );
 });
 
-test("generate variables with sorted expressions", () => {
+test("generate variables with expressions", () => {
   const generated = generateDataSources({
     scope: createScope(),
     dataSources: new Map([
@@ -255,7 +255,7 @@ test("generate variables with sorted expressions", () => {
           scopeInstanceId: "instance1",
           type: "expression",
           name: "myExp",
-          code: `$ws$dataSource$dataSource3 + "Name"`,
+          code: `$ws$dataSource$dataSource1 + "Name"`,
         },
       ],
       [
@@ -269,11 +269,32 @@ test("generate variables with sorted expressions", () => {
         },
       ],
     ]),
-    props: new Map(),
+    props: new Map([
+      [
+        "prop1",
+        {
+          id: "prop1",
+          instanceId: "instance1",
+          type: "dataSource",
+          name: "exp",
+          value: "dataSource2",
+        },
+      ],
+      [
+        "prop2",
+        {
+          id: "prop2",
+          instanceId: "instance2",
+          type: "dataSource",
+          name: "exp",
+          value: "dataSource3",
+        },
+      ],
+    ]),
   });
   expect(generated.body).toMatchInlineSnapshot(`
-"let myExp = (myVar + "Value");
-let myExp_1 = (myExp + "Name");
+"let exp = (myVar + "Name");
+let exp_1 = (myVar + "Value");
 "
 `);
   expect(generated.variables).toEqual(
@@ -291,8 +312,8 @@ let myExp_1 = (myExp + "Name");
   expect(generated.output).toEqual(
     new Map([
       ["dataSource1", "myVar"],
-      ["dataSource2", "myExp_1"],
-      ["dataSource3", "myExp"],
+      ["prop1", "exp"],
+      ["prop2", "exp_1"],
     ])
   );
 });

packages/react-sdk/src/expression.ts

@@ -161,26 +161,6 @@ export const validateExpression = (
   return generateCode(expression, true, effectful, transformIdentifier);
 };
 
-const sortTopologically = (
-  list: Set<string>,
-  depsById: Map<string, Set<string>>,
-  explored = new Set<string>(),
-  sorted: string[] = []
-) => {
-  for (const id of list) {
-    if (explored.has(id)) {
-      continue;
-    }
-    explored.add(id);
-    const deps = depsById.get(id);
-    if (deps) {
-      sortTopologically(deps, depsById, explored, sorted);
-    }
-    sorted.push(id);
-  }
-  return sorted;
-};
-
 const computeExpressionDependencies = (
   expressions: Map<string, string>,
   expressionId: string,
@@ -293,39 +273,8 @@ export const generateDataSources = ({
   let body = "";
   const output = new Map<DataSourceOrPropId, VariableName>();
 
-  // collect each data source depndencies to sort topologically
-  // and replace data sources with scoped names
-  const depsById = new Map<DataSourceId, Set<DataSourceId>>();
-  const codeById = new Map<DataSourceId, string>();
   for (const dataSource of dataSources.values()) {
-    if (dataSource.type === "expression") {
-      const deps = new Set<string>();
-      const newCode = validateExpression(dataSource.code, {
-        transformIdentifier: (identifier) => {
-          const depId = decodeDataSourceVariable(identifier);
-          const dep = depId ? dataSources.get(depId) : undefined;
-          if (dep) {
-            deps.add(dep.id);
-            return scope.getName(dep.id, dep.name);
-          }
-          // eslint-disable-next-line no-console
-          console.error(`Unknown dependency "${identifier}"`);
-          return identifier;
-        },
-      });
-      depsById.set(dataSource.id, deps);
-      codeById.set(dataSource.id, newCode);
-    }
-  }
-
-  // sort expressions starting with used ones as entry points
-  const sortedDataSources = sortTopologically(
-    new Set(dataSources.keys()),
-    depsById
-  );
-  for (const dataSourceId of sortedDataSources) {
-    const dataSource = dataSources.get(dataSourceId);
-    if (dataSource?.type === "variable") {
+    if (dataSource.type === "variable") {
       // save variables to generate header and footer depending on environment
       const valueName = scope.getName(dataSource.id, dataSource.name);
       const setterName = scope.getName(
@@ -336,63 +285,79 @@ export const generateDataSources = ({
       output.set(dataSource.id, valueName);
       variables.set(dataSource.id, { valueName, setterName, initialValue });
     }
-    if (dataSource?.type === "expression") {
-      const name = scope.getName(dataSource.id, dataSource.name);
-      const code = codeById.get(dataSourceId);
-      output.set(dataSource.id, name);
-      body += `let ${name} = (${code});\n`;
-    }
   }
 
-  // generate actions assigning variables and invoking their setters
   for (const prop of props.values()) {
-    if (prop.type !== "action") {
-      continue;
-    }
-    const name = scope.getName(prop.id, prop.name);
-    output.set(prop.id, name);
-    const setters = new Set<DataSourceId>();
-    let args: undefined | string[] = undefined;
-    let newCode = "";
-    for (const value of prop.value) {
-      args = value.args;
-      newCode += validateExpression(value.code, {
-        effectful: true,
-        transformIdentifier: (identifier, assignee) => {
-          if (args?.includes(identifier)) {
-            return identifier;
-          }
+    // generate prop expressions
+    if (prop.type === "dataSource") {
+      const dataSource = dataSources.get(prop.value);
+      if (dataSource?.type !== "expression") {
+        continue;
+      }
+      const name = scope.getName(prop.id, prop.name);
+      output.set(prop.id, name);
+      const code = validateExpression(dataSource.code, {
+        transformIdentifier: (identifier) => {
           const depId = decodeDataSourceVariable(identifier);
           const dep = depId ? dataSources.get(depId) : undefined;
           if (dep) {
-            const name = scope.getName(dep.id, dep.name);
-            if (assignee) {
-              setters.add(dep.id);
-            }
-            return name;
+            return scope.getName(dep.id, dep.name);
           }
           // eslint-disable-next-line no-console
           console.error(`Unknown dependency "${identifier}"`);
           return identifier;
         },
       });
-      newCode += `\n`;
+      body += `let ${name} = (${code});\n`;
     }
-    if (args === undefined) {
-      continue;
-    }
-    if (typed) {
-      args = args.map((arg) => `${arg}: any`);
-    }
-    body += `let ${name} = (${args.join(", ")}) => {\n`;
-    body += newCode;
-    for (const dataSourceId of setters.values()) {
-      const variable = variables.get(dataSourceId);
-      if (variable) {
-        body += `${variable.setterName}(${variable.valueName})\n`;
+
+    // generate actions assigning variables and invoking their setters
+    if (prop.type === "action") {
+      const name = scope.getName(prop.id, prop.name);
+      output.set(prop.id, name);
+      const setters = new Set<DataSourceId>();
+      let args: undefined | string[] = undefined;
+      let newCode = "";
+      for (const value of prop.value) {
+        args = value.args;
+        newCode += validateExpression(value.code, {
+          effectful: true,
+          transformIdentifier: (identifier, assignee) => {
+            if (args?.includes(identifier)) {
+              return identifier;
+            }
+            const depId = decodeDataSourceVariable(identifier);
+            const dep = depId ? dataSources.get(depId) : undefined;
+            if (dep) {
+              const name = scope.getName(dep.id, dep.name);
+              if (assignee) {
+                setters.add(dep.id);
+              }
+              return name;
+            }
+            // eslint-disable-next-line no-console
+            console.error(`Unknown dependency "${identifier}"`);
+            return identifier;
+          },
+        });
+        newCode += `\n`;
       }
+      if (args === undefined) {
+        continue;
+      }
+      if (typed) {
+        args = args.map((arg) => `${arg}: any`);
+      }
+      body += `let ${name} = (${args.join(", ")}) => {\n`;
+      body += newCode;
+      for (const dataSourceId of setters.values()) {
+        const variable = variables.get(dataSourceId);
+        if (variable) {
+          body += `${variable.setterName}(${variable.valueName})\n`;
+        }
+      }
+      body += `}\n`;
     }
-    body += `}\n`;
   }
 
   return {

packages/sdk-components-react-radix/src/__generated__/accordion.stories.tsx

@@ -14,7 +14,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [accordionValue, set$accordionValue] = useState<any>("0");
-  let expression = accordionValue;
+  let value = accordionValue;
   let onValueChange = (value: any) => {
     accordionValue = value;
     set$accordionValue(accordionValue);
@@ -25,7 +25,7 @@ const Page = (props: { scripts?: ReactNode }) => {
         data-ws-id="1"
         data-ws-component="Accordion"
         collapsible={true}
-        value={expression}
+        value={value}
         onValueChange={onValueChange}
       >
         <AccordionItem

packages/sdk-components-react-radix/src/__generated__/checkbox.stories.tsx

@@ -12,7 +12,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [checkboxChecked, set$checkboxChecked] = useState<any>(false);
-  let expression = checkboxChecked;
+  let checked = checkboxChecked;
   let onCheckedChange = (checked: any) => {
     checkboxChecked = checked;
     set$checkboxChecked(checkboxChecked);
@@ -23,7 +23,7 @@ const Page = (props: { scripts?: ReactNode }) => {
         <Checkbox
           data-ws-id="3"
           data-ws-component="Checkbox"
-          checked={expression}
+          checked={checked}
           onCheckedChange={onCheckedChange}
         >
           <CheckboxIndicator

packages/sdk-components-react-radix/src/__generated__/collapsible.stories.tsx

@@ -12,7 +12,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [collapsibleOpen, set$collapsibleOpen] = useState<any>(false);
-  let expression = collapsibleOpen;
+  let open = collapsibleOpen;
   let onOpenChange = (open: any) => {
     collapsibleOpen = open;
     set$collapsibleOpen(collapsibleOpen);
@@ -22,7 +22,7 @@ const Page = (props: { scripts?: ReactNode }) => {
       <Collapsible
         data-ws-id="1"
         data-ws-component="Collapsible"
-        open={expression}
+        open={open}
         onOpenChange={onOpenChange}
       >
         <CollapsibleTrigger

packages/sdk-components-react-radix/src/__generated__/dialog.stories.tsx

@@ -17,7 +17,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [dialogOpen, set$dialogOpen] = useState<any>(false);
-  let expression = dialogOpen;
+  let open = dialogOpen;
   let onOpenChange = (open: any) => {
     dialogOpen = open;
     set$dialogOpen(dialogOpen);
@@ -27,7 +27,7 @@ const Page = (props: { scripts?: ReactNode }) => {
       <Dialog
         data-ws-id="1"
         data-ws-component="Dialog"
-        open={expression}
+        open={open}
         onOpenChange={onOpenChange}
       >
         <DialogTrigger data-ws-id="6" data-ws-component="DialogTrigger">

packages/sdk-components-react-radix/src/__generated__/navigation-menu.stories.tsx

@@ -19,7 +19,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [menuValue, set$menuValue] = useState<any>("");
-  let expression = menuValue;
+  let value = menuValue;
   let onValueChange = (value: any) => {
     menuValue = value;
     set$menuValue(menuValue);
@@ -29,7 +29,7 @@ const Page = (props: { scripts?: ReactNode }) => {
       <NavigationMenu
         data-ws-id="1"
         data-ws-component="NavigationMenu"
-        value={expression}
+        value={value}
         onValueChange={onValueChange}
       >
         <NavigationMenuList

packages/sdk-components-react-radix/src/__generated__/popover.stories.tsx

@@ -12,7 +12,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [popoverOpen, set$popoverOpen] = useState<any>(false);
-  let expression = popoverOpen;
+  let open = popoverOpen;
   let onOpenChange = (open: any) => {
     popoverOpen = open;
     set$popoverOpen(popoverOpen);
@@ -22,7 +22,7 @@ const Page = (props: { scripts?: ReactNode }) => {
       <Popover
         data-ws-id="1"
         data-ws-component="Popover"
-        open={expression}
+        open={open}
         onOpenChange={onOpenChange}
       >
         <PopoverTrigger data-ws-id="6" data-ws-component="PopoverTrigger">

packages/sdk-components-react-radix/src/__generated__/radio-group.stories.tsx

@@ -13,7 +13,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [radioGroupValue, set$radioGroupValue] = useState<any>("");
-  let expression = radioGroupValue;
+  let value = radioGroupValue;
   let onValueChange = (value: any) => {
     radioGroupValue = value;
     set$radioGroupValue(radioGroupValue);
@@ -23,7 +23,7 @@ const Page = (props: { scripts?: ReactNode }) => {
       <RadioGroup
         data-ws-id="1"
         data-ws-component="RadioGroup"
-        value={expression}
+        value={value}
         onValueChange={onValueChange}
       >
         <Label data-ws-id="7" data-ws-component="Label">

packages/sdk-components-react-radix/src/__generated__/select.stories.tsx

@@ -17,12 +17,12 @@ import {
 const Page = (props: { scripts?: ReactNode }) => {
   let [selectValue, set$selectValue] = useState<any>("");
   let [selectOpen, set$selectOpen] = useState<any>(false);
-  let expression = selectValue;
-  let expression_1 = selectOpen;
+  let value = selectValue;
   let onValueChange = (value: any) => {
     selectValue = value;
     set$selectValue(selectValue);
   };
+  let open = selectOpen;
   let onOpenChange = (open: any) => {
     selectOpen = open;
     set$selectOpen(selectOpen);
@@ -32,9 +32,9 @@ const Page = (props: { scripts?: ReactNode }) => {
       <Select
         data-ws-id="1"
         data-ws-component="Select"
-        value={expression}
+        value={value}
         onValueChange={onValueChange}
-        open={expression_1}
+        open={open}
         onOpenChange={onOpenChange}
       >
         <SelectTrigger data-ws-id="10" data-ws-component="SelectTrigger">

packages/sdk-components-react-radix/src/__generated__/sheet.stories.tsx

@@ -17,7 +17,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [sheetOpen, set$sheetOpen] = useState<any>(false);
-  let expression = sheetOpen;
+  let open = sheetOpen;
   let onOpenChange = (open: any) => {
     sheetOpen = open;
     set$sheetOpen(sheetOpen);
@@ -27,7 +27,7 @@ const Page = (props: { scripts?: ReactNode }) => {
       <Dialog
         data-ws-id="1"
         data-ws-component="Dialog"
-        open={expression}
+        open={open}
         onOpenChange={onOpenChange}
       >
         <DialogTrigger data-ws-id="6" data-ws-component="DialogTrigger">

packages/sdk-components-react-radix/src/__generated__/switch.stories.tsx

@@ -4,7 +4,7 @@ import { Switch as Switch, SwitchThumb as SwitchThumb } from "../components";
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [switchChecked, set$switchChecked] = useState<any>(false);
-  let expression = switchChecked;
+  let checked = switchChecked;
   let onCheckedChange = (checked: any) => {
     switchChecked = checked;
     set$switchChecked(switchChecked);
@@ -14,7 +14,7 @@ const Page = (props: { scripts?: ReactNode }) => {
       <Switch
         data-ws-id="1"
         data-ws-component="Switch"
-        checked={expression}
+        checked={checked}
         onCheckedChange={onCheckedChange}
       >
         <SwitchThumb data-ws-id="7" data-ws-component="SwitchThumb" />

packages/sdk-components-react-radix/src/__generated__/tabs.stories.tsx

@@ -9,7 +9,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [tabsValue, set$tabsValue] = useState<any>("0");
-  let expression = tabsValue;
+  let value = tabsValue;
   let onValueChange = (value: any) => {
     tabsValue = value;
     set$tabsValue(tabsValue);
@@ -19,7 +19,7 @@ const Page = (props: { scripts?: ReactNode }) => {
       <Tabs
         data-ws-id="1"
         data-ws-component="Tabs"
-        value={expression}
+        value={value}
         onValueChange={onValueChange}
       >
         <TabsList data-ws-id="6" data-ws-component="TabsList">

packages/sdk-components-react-radix/src/__generated__/tooltip.stories.tsx

@@ -12,7 +12,7 @@ import {
 
 const Page = (props: { scripts?: ReactNode }) => {
   let [tooltipOpen, set$tooltipOpen] = useState<any>(false);
-  let expression = tooltipOpen;
+  let open = tooltipOpen;
   let onOpenChange = (open: any) => {
     tooltipOpen = open;
     set$tooltipOpen(tooltipOpen);
@@ -22,7 +22,7 @@ const Page = (props: { scripts?: ReactNode }) => {
       <Tooltip
         data-ws-id="1"
         data-ws-component="Tooltip"
-        open={expression}
+        open={open}
         onOpenChange={onOpenChange}
       >
         <TooltipTrigger data-ws-id="6" data-ws-component="TooltipTrigger">