fix: Saving variables as string rather than values in backend (#12999)

* parsing of js object has changed

* added test for parsing

* parse object with literal

* added variable to reactive paths and solved some PR comments

* test fixed

* small fix

* removed restricting functions to be added if object body has errors

Co-authored-by: Aishwarya UR <aishwarya@appsmith.com>

app/client/src/entities/DataTree/dataTreeJSAction.test.ts

@@ -158,11 +158,19 @@ describe("generateDataTreeJSAction", () => {
         body: "SMART_SUBSTITUTE",
         myFun2: "SMART_SUBSTITUTE",
         myFun1: "SMART_SUBSTITUTE",
+        myVar1: "SMART_SUBSTITUTE",
+        myVar2: "SMART_SUBSTITUTE",
       },
       dynamicBindingPathList: [
         {
           key: "body",
         },
+        {
+          key: "myVar1",
+        },
+        {
+          key: "myVar2",
+        },
         {
           key: "myFun2",
         },
@@ -186,6 +194,8 @@ describe("generateDataTreeJSAction", () => {
         body: "SMART_SUBSTITUTE",
         myFun1: "SMART_SUBSTITUTE",
         myFun2: "SMART_SUBSTITUTE",
+        myVar1: "SMART_SUBSTITUTE",
+        myVar2: "SMART_SUBSTITUTE",
       },
     };
     const result = generateDataTreeJSAction(jsCollection);
@@ -347,11 +357,19 @@ describe("generateDataTreeJSAction", () => {
         body: "SMART_SUBSTITUTE",
         myFun2: "SMART_SUBSTITUTE",
         myFun1: "SMART_SUBSTITUTE",
+        myVar1: "SMART_SUBSTITUTE",
+        myVar2: "SMART_SUBSTITUTE",
       },
       dynamicBindingPathList: [
         {
           key: "body",
         },
+        {
+          key: "myVar1",
+        },
+        {
+          key: "myVar2",
+        },
         {
           key: "myFun2",
         },
@@ -375,6 +393,8 @@ describe("generateDataTreeJSAction", () => {
         body: "SMART_SUBSTITUTE",
         myFun1: "SMART_SUBSTITUTE",
         myFun2: "SMART_SUBSTITUTE",
+        myVar1: "SMART_SUBSTITUTE",
+        myVar2: "SMART_SUBSTITUTE",
       },
     };
 

app/client/src/entities/DataTree/dataTreeJSAction.ts

@@ -28,6 +28,8 @@ export const generateDataTreeJSAction = (
       const variable = variables[i];
       variableList[variable.name] = variable.value;
       listVariables.push(variable.name);
+      dynamicBindingPathList.push({ key: variable.name });
+      bindingPaths[variable.name] = EvaluationSubstitutionType.SMART_SUBSTITUTE;
     }
   }
   const dependencyMap: DependencyMap = {};

app/client/src/utils/JSPaneUtils.tsx

@@ -155,6 +155,21 @@ export const getDifferenceInJSCollection = (
   } else {
     changedVariables = jsAction.variables;
   }
+  //delete variable
+  if (varList && varList.length > 0 && parsedBody.variables) {
+    for (let i = 0; i < varList.length; i++) {
+      const preVar = varList[i];
+      const existed = parsedBody.variables.find(
+        (jsVar: Variable) => jsVar.name === preVar.name,
+      );
+      if (!existed) {
+        const newvarList = varList.filter(
+          (deletedVar) => deletedVar.name !== preVar.name,
+        );
+        changedVariables = changedVariables.concat(newvarList);
+      }
+    }
+  }
   return {
     newActions: toBeAddedActions,
     updateActions: toBeUpdatedActions,

app/client/src/workers/DataTreeEvaluator/DataTreeEvaluator.ts

@@ -82,9 +82,9 @@ import {
   ActionValidationConfigMap,
   ValidationConfig,
 } from "constants/PropertyControlConstants";
+import { parseJSObjectWithAST } from "workers/ast";
 import { klona } from "klona/full";
 import { EvalMetaUpdates } from "./types";
-
 export default class DataTreeEvaluator {
   dependencyMap: DependencyMap = {};
   sortedDependencies: Array<string> = [];
@@ -573,8 +573,13 @@ export default class DataTreeEvaluator {
         Object.keys(entity.reactivePaths).forEach((propertyPath) => {
           const existingDeps =
             dependencies[`${entityName}.${propertyPath}`] || [];
-          const unevalPropValue = _.get(entity, propertyPath).toString();
+          const unevalPropValue = _.get(entity, propertyPath);
-          const { jsSnippets } = getDynamicBindings(unevalPropValue, entity);
+          const unevalPropValueString =
+            !!unevalPropValue && unevalPropValue.toString();
+          const { jsSnippets } = getDynamicBindings(
+            unevalPropValueString,
+            entity,
+          );
           dependencies[`${entityName}.${propertyPath}`] = existingDeps.concat(
             jsSnippets.filter((jsSnippet) => !!jsSnippet),
           );
@@ -730,6 +735,30 @@ export default class DataTreeEvaluator {
             _.set(currentTree, fullPropertyPath, evalPropertyValue);
             return currentTree;
           } else if (isJSAction(entity)) {
+            const variableList: Array<string> =
+              _.get(entity, "variables") || [];
+            if (variableList.indexOf(propertyPath) > -1) {
+              const currentEvaluatedValue = _.get(
+                currentTree,
+                getEvalValuePath(fullPropertyPath, {
+                  isPopulated: true,
+                  fullPath: true,
+                }),
+              );
+              if (!currentEvaluatedValue) {
+                _.set(
+                  currentTree,
+                  getEvalValuePath(fullPropertyPath, {
+                    isPopulated: true,
+                    fullPath: true,
+                  }),
+                  evalPropertyValue,
+                );
+                _.set(currentTree, fullPropertyPath, evalPropertyValue);
+              } else {
+                _.set(currentTree, fullPropertyPath, currentEvaluatedValue);
+              }
+            }
             return currentTree;
           } else {
             return _.set(currentTree, fullPropertyPath, evalPropertyValue);
@@ -1041,47 +1070,60 @@ export default class DataTreeEvaluator {
     if (correctFormat) {
       const body = entity.body.replace(/export default/g, "");
       try {
-        const { result } = evaluateSync(body, unEvalDataTree, {}, true);
         delete this.resolvedFunctions[`${entityName}`];
         delete this.currentJSCollectionState[`${entityName}`];
-        if (result) {
+        const parsedObject = parseJSObjectWithAST(body);
-          const actions: ParsedJSSubAction[] = [];
+        const actions: any = [];
-          const variables: any = [];
+        const variables: any = [];
-          Object.keys(result).forEach((unEvalFunc) => {
+        if (!!parsedObject) {
-            const unEvalValue = result[unEvalFunc];
+          parsedObject.forEach((parsedElement: any) => {
-            if (typeof unEvalValue === "function") {
+            if (
-              const params = getParams(unEvalValue);
+              parsedElement.type === "ArrowFunctionExpression" ||
-              const functionString = unEvalValue.toString();
+              parsedElement.type === "FunctionExpression"
-              _.set(
+            ) {
-                this.resolvedFunctions,
+              try {
-                `${entityName}.${unEvalFunc}`,
+                const { result } = evaluateSync(
-                unEvalValue,
+                  parsedElement.value,
-              );
+                  unEvalDataTree,
-              _.set(
+                  {},
-                this.currentJSCollectionState,
+                  true,
-                `${entityName}.${unEvalFunc}`,
+                );
-                functionString,
+                if (!!result) {
-              );
+                  const params = getParams(result);
-              actions.push({
+                  const functionString = parsedElement.value;
-                name: unEvalFunc,
+                  _.set(
-                body: functionString,
+                    this.resolvedFunctions,
-                arguments: params,
+                    `${entityName}.${parsedElement.key}`,
-                parsedFunction: unEvalValue,
+                    result,
-                isAsync: false,
+                  );
-              });
+                  _.set(
-            } else {
+                    this.currentJSCollectionState,
+                    `${entityName}.${parsedElement.key}`,
+                    functionString,
+                  );
+                  actions.push({
+                    name: parsedElement.key,
+                    body: functionString,
+                    arguments: params,
+                    parsedFunction: result,
+                    isAsync: false,
+                  });
+                }
+              } catch {
+                // in case we need to handle error state
+              }
+            } else if (parsedElement.type !== "literal") {
               variables.push({
-                name: unEvalFunc,
+                name: parsedElement.key,
-                value: result[unEvalFunc],
+                value: parsedElement.value,
               });
               _.set(
                 this.currentJSCollectionState,
-                `${entityName}.${unEvalFunc}`,
+                `${entityName}.${parsedElement.key}`,
-                unEvalValue,
+                parsedElement.value,
               );
             }
           });
-
           const parsedBody = {
             body: entity.body,
             actions: actions,
@@ -1097,16 +1139,8 @@ export default class DataTreeEvaluator {
             id: entity.actionId,
           });
         }
-      } catch (error) {
+      } catch (e) {
-        const errors = {
+        //if we need to push error as popup in case
-          type: EvalErrorTypes.PARSE_JS_ERROR,
-          context: {
-            entity: entity,
-            propertyPath: entity.name + ".body",
-          },
-          message: (error as Error).message,
-        };
-        this.errors.push(errors);
       }
     } else {
       const errors = {
@@ -1503,7 +1537,7 @@ export default class DataTreeEvaluator {
         if (!entity) {
           continue;
         }
-        if (!isAction(entity) && !isWidget(entity)) {
+        if (!isAction(entity) && !isWidget(entity) && !isJSAction(entity)) {
           continue;
         }
         let entityDynamicBindingPaths: string[] = [];

app/client/src/workers/ast.test.ts

@@ -1,4 +1,4 @@
-import { extractIdentifiersFromCode } from "workers/ast";
+import { extractIdentifiersFromCode, parseJSObjectWithAST } from "workers/ast";
 
 describe("getAllIdentifiers", () => {
   it("works properly", () => {
@@ -228,3 +228,85 @@ describe("getAllIdentifiers", () => {
     });
   });
 });
+
+describe("parseJSObjectWithAST", () => {
+  it("parse js object", () => {
+    const body = `{
+	myVar1: [],
+	myVar2: {},
+	myFun1: () => {
+		//write code here
+	},
+	myFun2: async () => {
+		//use async-await or promises
+	}
+}`;
+    const parsedObject = [
+      {
+        key: "myVar1",
+        value: "[]",
+        type: "ArrayExpression",
+      },
+      {
+        key: "myVar2",
+        value: "{}",
+        type: "ObjectExpression",
+      },
+      {
+        key: "myFun1",
+        value: "() => {}",
+        type: "ArrowFunctionExpression",
+      },
+      {
+        key: "myFun2",
+        value: "async () => {}",
+        type: "ArrowFunctionExpression",
+      },
+    ];
+    const resultParsedObject = parseJSObjectWithAST(body);
+    expect(resultParsedObject).toStrictEqual(parsedObject);
+  });
+  it("parse js object with literal", () => {
+    const body = `{
+	myVar1: [],
+	myVar2: {
+		"a": "app",
+	},
+	myFun1: () => {
+		//write code here
+	},
+	myFun2: async () => {
+		//use async-await or promises
+	}
+}`;
+    const parsedObject = [
+      {
+        key: "myVar1",
+        value: "[]",
+        type: "ArrayExpression",
+      },
+      {
+        key: '"a"',
+        value: '"app"',
+        type: "Literal",
+      },
+      {
+        key: "myVar2",
+        value: '{\n  "a": "app"\n}',
+        type: "ObjectExpression",
+      },
+      {
+        key: "myFun1",
+        value: "() => {}",
+        type: "ArrowFunctionExpression",
+      },
+      {
+        key: "myFun2",
+        value: "async () => {}",
+        type: "ArrowFunctionExpression",
+      },
+    ];
+    const resultParsedObject = parseJSObjectWithAST(body);
+    expect(resultParsedObject).toStrictEqual(parsedObject);
+  });
+});

app/client/src/workers/ast.ts

@@ -1,8 +1,9 @@
 import { parse, Node } from "acorn";
-import { ancestor } from "acorn-walk";
+import { ancestor, simple } from "acorn-walk";
 import { ECMA_VERSION, NodeTypes } from "constants/ast";
 import _ from "lodash";
 import { sanitizeScript } from "./evaluate";
+import { generate } from "astring";
 
 /*
  * Valuable links:
@@ -298,3 +299,26 @@ const getPropertyAccessor = (propertyNode: IdentifierNode | LiteralNode) => {
     return `[${propertyNode.value}]`;
   }
 };
+
+export const parseJSObjectWithAST = (jsObjectBody: string) => {
+  const createString = `var jsObj = ${jsObjectBody}`;
+  const ast = parse(createString, { ecmaVersion: ECMA_VERSION });
+  const rawKeyValues: any = [];
+  const parsedObject: any = [];
+  simple(ast, {
+    Property(node: any) {
+      rawKeyValues.push({
+        key: node.key,
+        value: node.value,
+      });
+    },
+  });
+  rawKeyValues.forEach((rawKeyValue: any) => {
+    parsedObject.push({
+      key: generate(rawKeyValue.key),
+      value: generate(rawKeyValue.value),
+      type: rawKeyValue.value.type,
+    });
+  });
+  return parsedObject;
+};

app/client/src/workers/evaluationUtils.ts

@@ -822,6 +822,22 @@ export const updateJSCollectionInDataTree = (
         }
       } else {
         varList.push(newVar.name);
+        const reactivePaths = jsCollection.reactivePaths;
+        reactivePaths[newVar.name] =
+          EvaluationSubstitutionType.SMART_SUBSTITUTE;
+        _.set(
+          modifiedDataTree,
+          `${jsCollection.name}.reactivePaths`,
+          reactivePaths,
+        );
+        const dynamicBindingPathList = jsCollection.dynamicBindingPathList;
+        dynamicBindingPathList.push({ key: newVar.name });
+        _.set(
+          modifiedDataTree,
+          `${jsCollection.name}.dynamicBindingPathList`,
+          dynamicBindingPathList,
+        );
+
         _.set(modifiedDataTree, `${jsCollection.name}.variables`, varList);
         _.set(
           modifiedDataTree,
@@ -830,15 +846,33 @@ export const updateJSCollectionInDataTree = (
         );
       }
     }
-    let newVarList: Array<string> = [];
+    let newVarList: Array<string> = varList;
     for (let i = 0; i < varList.length; i++) {
       const varListItem = varList[i];
       const existsInParsed = parsedBody.variables.find(
         (item) => item.name === varListItem,
       );
       if (!existsInParsed) {
+        const reactivePaths = jsCollection.reactivePaths;
+        delete reactivePaths[varListItem];
+        _.set(
+          modifiedDataTree,
+          `${jsCollection.name}.reactivePaths`,
+          reactivePaths,
+        );
+
+        let dynamicBindingPathList = jsCollection.dynamicBindingPathList;
+        dynamicBindingPathList = dynamicBindingPathList.filter(
+          (path) => path["key"] !== varListItem,
+        );
+        _.set(
+          modifiedDataTree,
+          `${jsCollection.name}.dynamicBindingPathList`,
+          dynamicBindingPathList,
+        );
+
+        newVarList = newVarList.filter((item) => item !== varListItem);
         delete modifiedDataTree[`${jsCollection.name}`][`${varListItem}`];
-        newVarList = varList.filter((item) => item !== varListItem);
       }
     }
     if (newVarList.length) {