From 7fa4df32a1b4ed35e0e07609e520a4e1d99517c4 Mon Sep 17 00:00:00 2001 From: Apeksha Bhosale <7846888+ApekshaBhosale@users.noreply.github.com> Date: Fri, 24 Jun 2022 09:36:52 +0530 Subject: [PATCH] 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 --- .../DataTree/dataTreeJSAction.test.ts | 20 +++ .../src/entities/DataTree/dataTreeJSAction.ts | 2 + app/client/src/utils/JSPaneUtils.tsx | 15 +++ .../DataTreeEvaluator/DataTreeEvaluator.ts | 126 +++++++++++------- app/client/src/workers/ast.test.ts | 84 +++++++++++- app/client/src/workers/ast.ts | 26 +++- app/client/src/workers/evaluationUtils.ts | 38 +++++- 7 files changed, 261 insertions(+), 50 deletions(-) diff --git a/app/client/src/entities/DataTree/dataTreeJSAction.test.ts b/app/client/src/entities/DataTree/dataTreeJSAction.test.ts index 1cbbdd64b8..5627882634 100644 --- a/app/client/src/entities/DataTree/dataTreeJSAction.test.ts +++ b/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", }, }; diff --git a/app/client/src/entities/DataTree/dataTreeJSAction.ts b/app/client/src/entities/DataTree/dataTreeJSAction.ts index e9bc72401a..f8e7427d16 100644 --- a/app/client/src/entities/DataTree/dataTreeJSAction.ts +++ b/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 = {}; diff --git a/app/client/src/utils/JSPaneUtils.tsx b/app/client/src/utils/JSPaneUtils.tsx index 1aeddab0ad..66f4033547 100644 --- a/app/client/src/utils/JSPaneUtils.tsx +++ b/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, diff --git a/app/client/src/workers/DataTreeEvaluator/DataTreeEvaluator.ts b/app/client/src/workers/DataTreeEvaluator/DataTreeEvaluator.ts index 651877a05c..a9309ac6bf 100644 --- a/app/client/src/workers/DataTreeEvaluator/DataTreeEvaluator.ts +++ b/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 = []; @@ -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 { jsSnippets } = getDynamicBindings(unevalPropValue, entity); + const unevalPropValue = _.get(entity, propertyPath); + 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 = + _.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 actions: ParsedJSSubAction[] = []; - const variables: any = []; - Object.keys(result).forEach((unEvalFunc) => { - const unEvalValue = result[unEvalFunc]; - if (typeof unEvalValue === "function") { - const params = getParams(unEvalValue); - const functionString = unEvalValue.toString(); - _.set( - this.resolvedFunctions, - `${entityName}.${unEvalFunc}`, - unEvalValue, - ); - _.set( - this.currentJSCollectionState, - `${entityName}.${unEvalFunc}`, - functionString, - ); - actions.push({ - name: unEvalFunc, - body: functionString, - arguments: params, - parsedFunction: unEvalValue, - isAsync: false, - }); - } else { + const parsedObject = parseJSObjectWithAST(body); + const actions: any = []; + const variables: any = []; + if (!!parsedObject) { + parsedObject.forEach((parsedElement: any) => { + if ( + parsedElement.type === "ArrowFunctionExpression" || + parsedElement.type === "FunctionExpression" + ) { + try { + const { result } = evaluateSync( + parsedElement.value, + unEvalDataTree, + {}, + true, + ); + if (!!result) { + const params = getParams(result); + const functionString = parsedElement.value; + _.set( + this.resolvedFunctions, + `${entityName}.${parsedElement.key}`, + result, + ); + _.set( + 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, - value: result[unEvalFunc], + name: parsedElement.key, + value: parsedElement.value, }); _.set( this.currentJSCollectionState, - `${entityName}.${unEvalFunc}`, - unEvalValue, + `${entityName}.${parsedElement.key}`, + parsedElement.value, ); } }); - const parsedBody = { body: entity.body, actions: actions, @@ -1097,16 +1139,8 @@ export default class DataTreeEvaluator { id: entity.actionId, }); } - } catch (error) { - const errors = { - type: EvalErrorTypes.PARSE_JS_ERROR, - context: { - entity: entity, - propertyPath: entity.name + ".body", - }, - message: (error as Error).message, - }; - this.errors.push(errors); + } catch (e) { + //if we need to push error as popup in case } } 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[] = []; diff --git a/app/client/src/workers/ast.test.ts b/app/client/src/workers/ast.test.ts index 4fa3ab6faf..6f556eff60 100644 --- a/app/client/src/workers/ast.test.ts +++ b/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); + }); +}); diff --git a/app/client/src/workers/ast.ts b/app/client/src/workers/ast.ts index 7f490eb85a..708e350bd6 100644 --- a/app/client/src/workers/ast.ts +++ b/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; +}; diff --git a/app/client/src/workers/evaluationUtils.ts b/app/client/src/workers/evaluationUtils.ts index 0145fe9a2b..40fb9e8157 100644 --- a/app/client/src/workers/evaluationUtils.ts +++ b/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 = []; + let newVarList: Array = 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) {