From 70f8777afda9cd0d1e3c11ea7e95070e04ce0f4a Mon Sep 17 00:00:00 2001 From: Ashit Rath Date: Mon, 15 Jul 2024 21:01:42 +0530 Subject: [PATCH] chore: Compute default value for jsaction params (#34708) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR adds the values to jsArguments. The logic for this is - If the value is string then it is kept as is - For non-strings they are wrapped with `{{ }}` do maintain the data type integrity when evaluated. This property is currently not used anywhere in the platform and this is intended to be used by js modules to identify the default values of parameters and provide support to alter then in a UI in the app. This PR also splits `workers/Evaluation/getJSActionForEvalContext.ts` to override in the EE for modules PR for https://github.com/appsmithorg/appsmith-ee/pull/4612 ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: c6ab372477fb3fd2f1ce171729af4fa64ac2a487 > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Sat, 13 Jul 2024 12:16:08 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Added support for additional node types (`RestElement`, `ObjectPattern`, `ArrayPattern`) in our AST processing. - Introduced `addPropertiesToJSObjectCode` function to enhance JavaScript object property management. - **Updates** - Enhanced `myFun2` function with new parameters and default values to improve flexibility and usage. - Improved `parseJSObject` function with additional parameters for better functionality. - **Tests** - Added a new test suite for `addPropertiesToJSObjectCode` function to ensure robust property management in JavaScript objects. --- app/client/packages/ast/index.ts | 7 +- app/client/packages/ast/src/constants/ast.ts | 3 + app/client/packages/ast/src/index.test.ts | 39 +++-- app/client/packages/ast/src/index.ts | 93 +++++++++++- .../packages/ast/src/jsObject/index.test.ts | 137 ++++++++++++++++++ app/client/packages/ast/src/jsObject/index.ts | 62 +++++++- .../Evaluation/getEntityForEvalContextMap.ts | 2 +- .../Evaluation/getJSActionForEvalContext.ts | 0 .../Evaluation/getJSActionForEvalContext.ts | 1 + 9 files changed, 316 insertions(+), 28 deletions(-) create mode 100644 app/client/packages/ast/src/jsObject/index.test.ts rename app/client/src/{ => ce}/workers/Evaluation/getJSActionForEvalContext.ts (100%) create mode 100644 app/client/src/ee/workers/Evaluation/getJSActionForEvalContext.ts diff --git a/app/client/packages/ast/index.ts b/app/client/packages/ast/index.ts index e5fe99afd0..1387a20da0 100644 --- a/app/client/packages/ast/index.ts +++ b/app/client/packages/ast/index.ts @@ -35,7 +35,11 @@ import type { JSVarProperty, JSFunctionProperty, } from "./src/jsObject"; -import { parseJSObject, isJSFunctionProperty } from "./src/jsObject"; +import { + parseJSObject, + isJSFunctionProperty, + addPropertiesToJSObjectCode, +} from "./src/jsObject"; // action creator import { @@ -139,4 +143,5 @@ export { isFunctionPresent, PeekOverlayExpressionIdentifier, getMemberExpressionObjectFromProperty, + addPropertiesToJSObjectCode, }; diff --git a/app/client/packages/ast/src/constants/ast.ts b/app/client/packages/ast/src/constants/ast.ts index ec021a68a0..dc3c833681 100644 --- a/app/client/packages/ast/src/constants/ast.ts +++ b/app/client/packages/ast/src/constants/ast.ts @@ -16,6 +16,9 @@ export enum NodeTypes { AssignmentPattern = "AssignmentPattern", Literal = "Literal", Property = "Property", + RestElement = "RestElement", + ObjectPattern = "ObjectPattern", + ArrayPattern = "ArrayPattern", // Declaration - https://github.com/estree/estree/blob/master/es5.md#declarations FunctionDeclaration = "FunctionDeclaration", ExportDefaultDeclaration = "ExportDefaultDeclaration", diff --git a/app/client/packages/ast/src/index.test.ts b/app/client/packages/ast/src/index.test.ts index ab53bce6a7..fbc64410d5 100644 --- a/app/client/packages/ast/src/index.test.ts +++ b/app/client/packages/ast/src/index.test.ts @@ -568,7 +568,7 @@ describe("parseJSObjectWithAST", () => { it("parse js object with params of all types", () => { const body = `export default{ - myFun2: async (a,b = Array(1,2,3),c = "", d = [], e = this.myVar1, f = {}, g = function(){}, h = Object.assign({}), i = String(), j = storeValue()) => { + myFun2: async (a,b = Array(1,2,3),c = "", d = [], e = this.myVar1, f = {}, g = function(){}, h = Object.assign({}), i = String(), j = storeValue(), k = "Hello", l = 10, m = null, n = "hello" + 500, o = true, p = () => "arrow function", { o1 = 20, o2 }, [ a1, a2 = 30 ], { k1 = 20, k2 = 40 } = { k1: 500, k2: 600 }, [ g1 = 5, g2 ] = [], ...rest) => { //use async-await or promises }, }`; @@ -576,10 +576,12 @@ describe("parseJSObjectWithAST", () => { const expectedParsedObject = [ { key: "myFun2", - value: - 'async (a, b = Array(1, 2, 3), c = "", d = [], e = this.myVar1, f = {}, g = function () {}, h = Object.assign({}), i = String(), j = storeValue()) => {}', + value: `async (a, b = Array(1, 2, 3), c = \"\", d = [], e = this.myVar1, f = {}, g = function () {}, h = Object.assign({}), i = String(), j = storeValue(), k = \"Hello\", l = 10, m = null, n = \"hello\" + 500, o = true, p = () => \"arrow function\", {o1 = 20, o2}, [a1, a2 = 30], {k1 = 20, k2 = 40} = { + k1: 500, + k2: 600 +}, [g1 = 5, g2] = [], ...rest) => {}`, rawContent: - 'myFun2: async (a,b = Array(1,2,3),c = "", d = [], e = this.myVar1, f = {}, g = function(){}, h = Object.assign({}), i = String(), j = storeValue()) => {\n' + + 'myFun2: async (a,b = Array(1,2,3),c = "", d = [], e = this.myVar1, f = {}, g = function(){}, h = Object.assign({}), i = String(), j = storeValue(), k = "Hello", l = 10, m = null, n = "hello" + 500, o = true, p = () => "arrow function", { o1 = 20, o2 }, [ a1, a2 = 30 ], { k1 = 20, k2 = 40 } = { k1: 500, k2: 600 }, [ g1 = 5, g2 ] = [], ...rest) => {\n' + " //use async-await or promises\n" + " }", type: "ArrowFunctionExpression", @@ -595,15 +597,26 @@ describe("parseJSObjectWithAST", () => { }, arguments: [ { paramName: "a", defaultValue: undefined }, - { paramName: "b", defaultValue: undefined }, - { paramName: "c", defaultValue: undefined }, - { paramName: "d", defaultValue: undefined }, - { paramName: "e", defaultValue: undefined }, - { paramName: "f", defaultValue: undefined }, - { paramName: "g", defaultValue: undefined }, - { paramName: "h", defaultValue: undefined }, - { paramName: "i", defaultValue: undefined }, - { paramName: "j", defaultValue: undefined }, + { paramName: "b", defaultValue: "{{Array(1,2,3)}}" }, + { paramName: "c", defaultValue: "" }, + { paramName: "d", defaultValue: "{{[]}}" }, + { paramName: "e", defaultValue: "{{this.myVar1}}" }, + { paramName: "f", defaultValue: "{{{}}}" }, + { paramName: "g", defaultValue: "{{function(){}}}" }, + { paramName: "h", defaultValue: "{{Object.assign({})}}" }, + { paramName: "i", defaultValue: "{{String()}}" }, + { paramName: "j", defaultValue: "{{storeValue()}}" }, + { paramName: "k", defaultValue: "Hello" }, + { paramName: "l", defaultValue: "{{10}}" }, + { paramName: "m", defaultValue: "{{null}}" }, + { paramName: "n", defaultValue: '{{"hello" + 500}}' }, + { paramName: "o", defaultValue: "{{true}}" }, + { paramName: "p", defaultValue: '{{() => "arrow function"}}' }, + { paramName: "", defaultValue: "{{{}}}" }, + { paramName: "", defaultValue: "{{[]}}" }, + { paramName: "", defaultValue: undefined }, + { paramName: "", defaultValue: undefined }, + { paramName: "rest", defaultValue: undefined }, ], isMarkedAsync: true, }, diff --git a/app/client/packages/ast/src/index.ts b/app/client/packages/ast/src/index.ts index 83a9f93541..6e6213189b 100644 --- a/app/client/packages/ast/src/index.ts +++ b/app/client/packages/ast/src/index.ts @@ -26,7 +26,12 @@ import { generate } from "astring"; * */ -type Pattern = IdentifierNode | AssignmentPatternNode; +type Pattern = + | IdentifierNode + | AssignmentPatternNode + | ArrayPatternNode + | ObjectPatternNode + | RestElementNode; type Expression = Node; export type ArgumentTypes = | LiteralNode @@ -59,6 +64,31 @@ export interface IdentifierNode extends Node { name: string; } +export interface ArrayPatternNode extends Node { + type: NodeTypes.ArrayPattern; + elements: Array; +} + +export interface AssignmentProperty extends Node { + type: NodeTypes.Property; + key: Expression; + value: Pattern; + kind: "init"; + method: false; + shorthand: boolean; + computed: boolean; +} + +export interface RestElementNode extends Node { + type: NodeTypes.RestElement; + argument: Pattern; +} + +export interface ObjectPatternNode extends Node { + type: NodeTypes.ObjectPattern; + properties: Array; +} + //Using this to handle the Variable property refactor interface RefactorIdentifierNode extends Node { type: NodeTypes.Identifier; @@ -104,6 +134,7 @@ export interface ObjectExpression extends Expression { interface AssignmentPatternNode extends Node { type: NodeTypes.AssignmentPattern; left: Pattern; + right: ArgumentTypes; } // doc: https://github.com/estree/estree/blob/master/es5.md#literal @@ -256,6 +287,18 @@ const isAssignmentPatternNode = (node: Node): node is AssignmentPatternNode => { return node.type === NodeTypes.AssignmentPattern; }; +export const isArrayPatternNode = (node: Node): node is ArrayPatternNode => { + return node.type === NodeTypes.ArrayPattern; +}; + +export const isObjectPatternNode = (node: Node): node is ObjectPatternNode => { + return node.type === NodeTypes.ObjectPattern; +}; + +export const isRestElementNode = (node: Node): node is RestElementNode => { + return node.type === NodeTypes.RestElement; +}; + export const isLiteralNode = (node: Node): node is LiteralNode => { return node.type === NodeTypes.Literal; }; @@ -519,6 +562,7 @@ export const getFunctionalParamsFromNode = ( | FunctionExpressionNode | ArrowFunctionExpressionNode, needValue = false, + code = "", ): Set => { const functionalParams = new Set(); node.params.forEach((paramNode) => { @@ -530,15 +574,50 @@ export const getFunctionalParamsFromNode = ( } else if (isAssignmentPatternNode(paramNode)) { if (isIdentifierNode(paramNode.left)) { const paramName = paramNode.left.name; - if (!needValue) { + if (!needValue || !code) { functionalParams.add({ paramName, defaultValue: undefined }); } else { - // figure out how to get value of paramNode.right for each node type - // currently we don't use params value, hence skipping it - // functionalParams.add({ - // defaultValue: paramNode.right.value, - // }); + const defaultValueInString = code.slice( + paramNode.right.start, + paramNode.right.end, + ); + const defaultValue = + paramNode.right.type === "Literal" && + typeof paramNode.right.value === "string" + ? paramNode.right.value + : `{{${defaultValueInString}}}`; + functionalParams.add({ + paramName, + defaultValue, + }); } + } else if ( + isObjectPatternNode(paramNode.left) || + isArrayPatternNode(paramNode.left) + ) { + functionalParams.add({ + paramName: "", + defaultValue: undefined, + }); + } + // The below computations are very basic and can be evolved into nested + // parsing logic to find param and it's default value. + } else if (isObjectPatternNode(paramNode)) { + functionalParams.add({ + paramName: "", + defaultValue: `{{{}}}`, + }); + } else if (isArrayPatternNode(paramNode)) { + functionalParams.add({ + paramName: "", + defaultValue: "{{[]}}", + }); + } else if (isRestElementNode(paramNode)) { + if ("name" in paramNode.argument) { + functionalParams.add({ + paramName: paramNode.argument.name, + defaultValue: undefined, + }); } } }); diff --git a/app/client/packages/ast/src/jsObject/index.test.ts b/app/client/packages/ast/src/jsObject/index.test.ts new file mode 100644 index 0000000000..5a688d0dc4 --- /dev/null +++ b/app/client/packages/ast/src/jsObject/index.test.ts @@ -0,0 +1,137 @@ +import { parse } from "acorn"; +import { simple } from "acorn-walk"; +import { addPropertiesToJSObjectCode } from "."; + +describe("addPropertiesToJSObjectCode", () => { + const parseAST = (code: string) => + parse(code, { sourceType: "module", ecmaVersion: 2020 }); + + const findProperty = (properties: any[] | undefined, key: string) => + properties?.find((property) => property.key.name === key); + + it("should add new properties to the object", () => { + const body = ` + export default { + myVar1: [], + myVar2: {}, + myFun1 () { + // write code here + // this.myVar1 = [1,2,3] + }, + async myFun2 () { + // use async-await or promises + // await storeValue('varName', 'hello world') + } + }`; + const obj = { + inputs: "Module1.inputs", + newProp: "42", + }; + + const result = addPropertiesToJSObjectCode(body, obj); + + const ast = parseAST(result); + let properties; + simple(ast, { + ExportDefaultDeclaration(node) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + properties = node.declaration.properties; + }, + }); + + const inputsProperty = findProperty(properties, "inputs"); + const newPropProperty = findProperty(properties, "newProp"); + + expect(inputsProperty).toBeDefined(); + expect(newPropProperty).toBeDefined(); + expect(inputsProperty.value.type).toBe("MemberExpression"); + expect(newPropProperty.value.value).toBe(42); + }); + + it("should replace existing properties", () => { + const body = ` + export default { + myVar1: [], + myVar2: {}, + inputs: 'oldValue', + myFun1 () { + // write code here + // this.myVar1 = [1,2,3] + }, + async myFun2 () { + // use async-await or promises + // await storeValue('varName', 'hello world') + } + }`; + const obj = { + inputs: "Module1.inputs", + myVar1: "[1, 2, 3]", + }; + + const result = addPropertiesToJSObjectCode(body, obj); + + const ast = parseAST(result); + let properties; + simple(ast, { + ExportDefaultDeclaration(node) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + properties = node.declaration.properties; + }, + }); + + const inputsProperty = findProperty(properties, "inputs"); + const myVar1Property = findProperty(properties, "myVar1"); + + expect(inputsProperty).toBeDefined(); + expect(myVar1Property).toBeDefined(); + expect(inputsProperty.value.type).toBe("MemberExpression"); + expect(myVar1Property.value.type).toBe("ArrayExpression"); + expect( + myVar1Property.value.elements.map((e: { value: any }) => e.value), + ).toEqual([1, 2, 3]); + }); + + it("should handle empty object input without errors", () => { + const body = `export default { + myVar1: [], + myVar2: {}, + myFun1() { + }, + async myFun2() { + } +};`; + const obj = {}; + + const result = addPropertiesToJSObjectCode(body, obj); + + expect(result).toBe(body); + }); + + it("should handle empty string input without errors", () => { + const body = ``; + const obj = { + inputs: "Module1.inputs", + }; + + const result = addPropertiesToJSObjectCode(body, obj); + + expect(result).toBe(body); + }); + + it("should handle missing export default declaration gracefully", () => { + const body = `const myVar1 = []; +const myVar2 = {}; +function myFun1() { +} +async function myFun2() { +}`; + const obj = { + inputs: "Module1.inputs", + }; + + const result = addPropertiesToJSObjectCode(body, obj); + expect(result).toEqual(body); + }); +}); diff --git a/app/client/packages/ast/src/jsObject/index.ts b/app/client/packages/ast/src/jsObject/index.ts index 5b819396ba..589df0bc67 100644 --- a/app/client/packages/ast/src/jsObject/index.ts +++ b/app/client/packages/ast/src/jsObject/index.ts @@ -1,4 +1,4 @@ -import type { Node } from "acorn"; +import { parseExpressionAt, type Node } from "acorn"; import { simple } from "acorn-walk"; import type { IdentifierNode, @@ -15,8 +15,8 @@ import { import { generate } from "astring"; import type { functionParam } from "../index"; import { getFunctionalParamsFromNode, isPropertyAFunctionNode } from "../index"; -import { SourceType } from "../../index"; -import { attachComments } from "escodegen"; +import { ECMA_VERSION, SourceType } from "../../index"; +import escodegen, { attachComments } from "escodegen"; import { extractContentByPosition } from "../utils"; const jsObjectVariableName = @@ -52,6 +52,10 @@ export type JSVarProperty = BaseJSProperty; export type TParsedJSProperty = JSVarProperty | JSFunctionProperty; +interface Property extends PropertyNode { + key: IdentifierNode; +} + export const isJSFunctionProperty = ( t: TParsedJSProperty, ): t is JSFunctionProperty => { @@ -122,9 +126,7 @@ export const parseJSObject = (code: string) => { }; if (isPropertyAFunctionNode(node.value)) { - // if in future we need default values of each param, we could implement that in getFunctionalParamsFromNode - // currently we don't consume it anywhere hence avoiding to calculate that. - const params = getFunctionalParamsFromNode(node.value); + const params = getFunctionalParamsFromNode(node.value, true, code); property = { ...property, arguments: [...params], @@ -137,3 +139,51 @@ export const parseJSObject = (code: string) => { return { parsedObject: [...parsedObjectProperties], success: true }; }; + +export const addPropertiesToJSObjectCode = ( + code: string, + obj: Record, +) => { + try { + const ast = getAST(code, { sourceType: "module" }); + + simple(ast, { + ExportDefaultDeclaration(node: any) { + const properties: Property[] = node?.declaration?.properties; + + Object.entries(obj).forEach(([key, value]) => { + // Check if a property with the same name already exists + const existingPropertyIndex = properties.findIndex( + (property) => property.key.name === key, + ); + + const astValue = parseExpressionAt(value, 0, { + ecmaVersion: ECMA_VERSION, + }); + + // Create a new property + const newProperty = { + type: "Property", + key: { type: "Identifier", name: key }, + value: astValue, + kind: "init", + method: false, + shorthand: false, + computed: false, + } as unknown as Property; + + if (existingPropertyIndex >= 0) { + // Replace the existing property + properties[existingPropertyIndex] = newProperty; + } else { + // Add the new property + properties.push(newProperty); + } + }); + }, + }); + return escodegen.generate(ast); + } catch (e) { + return code; + } +}; diff --git a/app/client/src/ce/workers/Evaluation/getEntityForEvalContextMap.ts b/app/client/src/ce/workers/Evaluation/getEntityForEvalContextMap.ts index 8f9c9f3b77..42b0158fc5 100644 --- a/app/client/src/ce/workers/Evaluation/getEntityForEvalContextMap.ts +++ b/app/client/src/ce/workers/Evaluation/getEntityForEvalContextMap.ts @@ -1,6 +1,6 @@ import { ENTITY_TYPE } from "entities/DataTree/dataTreeFactory"; import type { DataTreeEntity } from "entities/DataTree/dataTreeTypes"; -import { getJSActionForEvalContext } from "workers/Evaluation/getJSActionForEvalContext"; +import { getJSActionForEvalContext } from "@appsmith/workers/Evaluation/getJSActionForEvalContext"; export const getEntityForEvalContextMap: Record< string, diff --git a/app/client/src/workers/Evaluation/getJSActionForEvalContext.ts b/app/client/src/ce/workers/Evaluation/getJSActionForEvalContext.ts similarity index 100% rename from app/client/src/workers/Evaluation/getJSActionForEvalContext.ts rename to app/client/src/ce/workers/Evaluation/getJSActionForEvalContext.ts diff --git a/app/client/src/ee/workers/Evaluation/getJSActionForEvalContext.ts b/app/client/src/ee/workers/Evaluation/getJSActionForEvalContext.ts new file mode 100644 index 0000000000..b997a691ab --- /dev/null +++ b/app/client/src/ee/workers/Evaluation/getJSActionForEvalContext.ts @@ -0,0 +1 @@ +export * from "ce/workers/Evaluation/getJSActionForEvalContext";