From 3cdb2dbbc46daabc28c9377581e36cdf3e701e22 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Sat, 8 Jul 2023 12:15:40 +0300 Subject: [PATCH] Support effectful expressions (#1908) Here added two new utilities for generation and executing individual expressions. Such expressions can use assignment operator to change any variable. This give us less opinionated solution for "set" action. We can just execute any code our parser supports. --- .../app/shared/nano-states/nano-states.ts | 7 +- packages/react-sdk/package.json | 1 + packages/react-sdk/src/embed-template.ts | 8 +- packages/react-sdk/src/expression.test.ts | 118 +++++++++++-- packages/react-sdk/src/expression.ts | 162 +++++++++++++++--- packages/react-sdk/src/index.ts | 6 +- pnpm-lock.yaml | 12 ++ 7 files changed, 269 insertions(+), 45 deletions(-) diff --git a/apps/builder/app/shared/nano-states/nano-states.ts b/apps/builder/app/shared/nano-states/nano-states.ts index 9e266f8e04..246ed4e71b 100644 --- a/apps/builder/app/shared/nano-states/nano-states.ts +++ b/apps/builder/app/shared/nano-states/nano-states.ts @@ -21,7 +21,7 @@ import type { StyleSourceSelections, } from "@webstudio-is/project-build"; import { - executeExpressions, + executeComputingExpressions, encodeDataSourceVariable, decodeDataSourceVariable, } from "@webstudio-is/react-sdk"; @@ -96,7 +96,10 @@ export const dataSourceValuesStore = computed( } } try { - const outputVariables = executeExpressions(variables, expressions); + const outputVariables = executeComputingExpressions( + expressions, + variables + ); for (const [name, value] of outputVariables) { const id = decodeDataSourceVariable(name); if (id !== undefined) { diff --git a/packages/react-sdk/package.json b/packages/react-sdk/package.json index aa626097fd..a2013166d5 100644 --- a/packages/react-sdk/package.json +++ b/packages/react-sdk/package.json @@ -36,6 +36,7 @@ "zod": "^3.19.1" }, "dependencies": { + "@jsep-plugin/assignment": "^1.2.1", "@nanostores/react": "^0.4.1", "@webstudio-is/asset-uploader": "workspace:^", "@webstudio-is/css-data": "workspace:^", diff --git a/packages/react-sdk/src/embed-template.ts b/packages/react-sdk/src/embed-template.ts index 30ce2b4afe..ac44ba9dfe 100644 --- a/packages/react-sdk/src/embed-template.ts +++ b/packages/react-sdk/src/embed-template.ts @@ -250,9 +250,11 @@ export const generateDataFromEmbedTemplate = ( const dataSources: DataSource[] = []; for (const dataSource of dataSourceByRef.values()) { if (dataSource.type === "expression") { - dataSource.code = validateExpression(dataSource.code, (ref) => { - const id = dataSourceByRef.get(ref)?.id ?? ref; - return encodeDataSourceVariable(id); + dataSource.code = validateExpression(dataSource.code, { + transformIdentifier: (ref) => { + const id = dataSourceByRef.get(ref)?.id ?? ref; + return encodeDataSourceVariable(id); + }, }); } dataSources.push(dataSource); diff --git a/packages/react-sdk/src/expression.test.ts b/packages/react-sdk/src/expression.test.ts index 98cede6fef..2b7ed7ff2f 100644 --- a/packages/react-sdk/src/expression.test.ts +++ b/packages/react-sdk/src/expression.test.ts @@ -2,8 +2,10 @@ import { expect, test } from "@jest/globals"; import { decodeDataSourceVariable, encodeDataSourceVariable, - executeExpressions, - generateExpressionsComputation, + executeComputingExpressions, + executeEffectfulExpression, + generateComputingExpressions, + generateEffectfulExpression, validateExpression, } from "./expression"; @@ -17,6 +19,13 @@ test("allow unary and binary expressions", () => { expect(validateExpression(`[-1, 1 + 1]`)).toEqual(`[-1, 1 + 1]`); }); +test("optionally allow assignment expressions", () => { + expect(() => { + validateExpression(`a = 2`); + }).toThrowError(/Cannot use assignment in this expression/); + expect(validateExpression(`a = 2`, { effectful: true })).toEqual(`a = 2`); +}); + test("forbid member expressions", () => { expect(() => { validateExpression("var1 + obj.param"); @@ -44,6 +53,21 @@ test("forbid ternary", () => { }).toThrowError(/Ternary operator is not supported/); }); +test("forbid increment and decrement", () => { + expect(() => { + validateExpression("++var1"); + }).toThrowError(/"\+\+" operator is not supported/); + expect(() => { + validateExpression("var1++"); + }).toThrowError(/"\+\+" operator is not supported/); + expect(() => { + validateExpression("--var1"); + }).toThrowError(/"--" operator is not supported/); + expect(() => { + validateExpression("var1--"); + }).toThrowError(/"--" operator is not supported/); +}); + test("forbid multiple expressions", () => { expect(() => { validateExpression("a b"); @@ -57,12 +81,14 @@ test("forbid multiple expressions", () => { }); test("transform identifiers", () => { - expect(validateExpression(`a + b`, (id) => `$ws$${id}`)).toEqual( - `$ws$a + $ws$b` - ); + expect( + validateExpression(`a + b`, { + transformIdentifier: (id) => `$ws$${id}`, + }) + ).toEqual(`$ws$a + $ws$b`); }); -test("generate expressions computation", () => { +test("generate computing expressions", () => { const variables = new Set(["var0"]); const expressions = new Map([ ["exp3", "exp2 + exp1"], @@ -70,7 +96,7 @@ test("generate expressions computation", () => { ["exp2", "exp1"], ["exp4", "exp2"], ]); - expect(generateExpressionsComputation(variables, expressions)) + expect(generateComputingExpressions(expressions, variables)) .toMatchInlineSnapshot(` "const var0 = _variables.get('var0'); const exp1 = (var0); @@ -86,10 +112,22 @@ test("generate expressions computation", () => { `); }); +test("add only used variables in computing expression", () => { + const expressions = new Map([["exp1", "var0"]]); + expect(generateComputingExpressions(expressions, new Set(["var0", "var1"]))) + .toMatchInlineSnapshot(` + "const var0 = _variables.get('var0'); + const exp1 = (var0); + return new Map([ + ['exp1', exp1], + ]);" + `); +}); + test("execute expression", () => { const variables = new Map(); const expressions = new Map([["exp1", "1 + 1"]]); - expect(executeExpressions(variables, expressions)).toEqual( + expect(executeComputingExpressions(expressions, variables)).toEqual( new Map([["exp1", 2]]) ); }); @@ -97,7 +135,7 @@ test("execute expression", () => { test("execute expression dependent on variables", () => { const variables = new Map([["var1", 5]]); const expressions = new Map([["exp1", "var1 + 1"]]); - expect(executeExpressions(variables, expressions)).toEqual( + expect(executeComputingExpressions(expressions, variables)).toEqual( new Map([["exp1", 6]]) ); }); @@ -108,7 +146,7 @@ test("execute expression dependent on another expressions", () => { ["exp1", "exp0 + 1"], ["exp0", "var1 + 2"], ]); - expect(executeExpressions(variables, expressions)).toEqual( + expect(executeComputingExpressions(expressions, variables)).toEqual( new Map([ ["exp1", 6], ["exp0", 5], @@ -124,7 +162,7 @@ test("forbid circular expressions", () => { ["exp2", "exp1 + 3"], ]); expect(() => { - executeExpressions(variables, expressions); + executeComputingExpressions(expressions, variables); }).toThrowError(/Cannot access 'exp0' before initialization/); }); @@ -132,7 +170,7 @@ test("make sure dependency exists", () => { const variables = new Map(); const expressions = new Map([["exp1", "var1 + 1"]]); expect(() => { - executeExpressions(variables, expressions); + executeComputingExpressions(expressions, variables); }).toThrowError(/Unknown dependency "var1"/); }); @@ -145,3 +183,59 @@ test("encode/decode variable names", () => { ); expect(decodeDataSourceVariable("myVarName")).toEqual(undefined); }); + +test("generate effectful expression", () => { + expect( + generateEffectfulExpression(`var0 = var0 + var1`, new Set(["var0", "var1"])) + ).toMatchInlineSnapshot(` + "let var0 = _variables.get('var0'); + let var1 = _variables.get('var1'); + var0 = var0 + var1; + return new Map([ + ['var0', var0], + ]);" + `); + + expect( + generateEffectfulExpression(`var0 = var1 + 1`, new Set(["var0", "var1"])) + ).toMatchInlineSnapshot(` + "let var1 = _variables.get('var1'); + let var0; + var0 = var1 + 1; + return new Map([ + ['var0', var0], + ]);" + `); +}); + +test("add only used variables in effectful expression", () => { + expect( + generateEffectfulExpression( + `var0 = var1 + 1`, + new Set(["var0", "var1", "var2"]) + ) + ).toMatchInlineSnapshot(` + "let var1 = _variables.get('var1'); + let var0; + var0 = var1 + 1; + return new Map([ + ['var0', var0], + ]);" + `); +}); + +test("forbid not allowed variables in effectful expression", () => { + expect(() => { + generateEffectfulExpression(`var0 = var0 + var1`, new Set(["var0"])); + }).toThrowError(/Unknown dependency "var1"/); +}); + +test("execute effectful expression", () => { + const variables = new Map([ + ["var0", 2], + ["var1", 3], + ]); + expect(executeEffectfulExpression(`var0 = var0 + var1`, variables)).toEqual( + new Map([["var0", 5]]) + ); +}); diff --git a/packages/react-sdk/src/expression.ts b/packages/react-sdk/src/expression.ts index bda360b363..af86e95158 100644 --- a/packages/react-sdk/src/expression.ts +++ b/packages/react-sdk/src/expression.ts @@ -1,27 +1,37 @@ import jsep from "jsep"; +import jsepAssignment from "@jsep-plugin/assignment"; +import type { + UpdateExpression, + AssignmentExpression, +} from "@jsep-plugin/assignment"; -type TransformIdentifier = (id: string) => string; +jsep.plugins.register(jsepAssignment); -type Node = jsep.CoreExpression; +type TransformIdentifier = (id: string, assignee: boolean) => string; + +type Node = jsep.CoreExpression | UpdateExpression | AssignmentExpression; const generateCode = ( node: Node, failOnForbidden: boolean, + effectful: boolean, transformIdentifier: TransformIdentifier ): string => { if (node.type === "Identifier") { - return transformIdentifier(node.name); + return transformIdentifier(node.name, false); } if (node.type === "MemberExpression") { if (failOnForbidden) { const object = generateCode( node.object as Node, false, + effectful, transformIdentifier ); const property = generateCode( node.property as Node, false, + effectful, transformIdentifier ); throw Error(`Cannot access "${property}" of "${object}"`); @@ -29,11 +39,13 @@ const generateCode = ( const object = generateCode( node.object as Node, failOnForbidden, + effectful, transformIdentifier ); const property = generateCode( node.property as Node, failOnForbidden, + effectful, transformIdentifier ); return `${object}.${property}`; @@ -45,6 +57,7 @@ const generateCode = ( const arg = generateCode( node.argument as Node, failOnForbidden, + effectful, transformIdentifier ); return `${node.operator}${arg}`; @@ -53,18 +66,25 @@ const generateCode = ( const left = generateCode( node.left as Node, failOnForbidden, + effectful, transformIdentifier ); const right = generateCode( node.right as Node, failOnForbidden, + effectful, transformIdentifier ); return `${left} ${node.operator} ${right}`; } if (node.type === "ArrayExpression") { const elements = node.elements.map((element) => - generateCode(element as Node, failOnForbidden, transformIdentifier) + generateCode( + element as Node, + failOnForbidden, + effectful, + transformIdentifier + ) ); return `[${elements.join(", ")}]`; } @@ -73,6 +93,7 @@ const generateCode = ( const callee = generateCode( node.callee as Node, false, + effectful, transformIdentifier ); throw Error(`Cannot call "${callee}"`); @@ -80,10 +101,11 @@ const generateCode = ( const callee = generateCode( node.callee as Node, failOnForbidden, + effectful, transformIdentifier ); const args = node.arguments.map((arg) => - generateCode(arg as Node, failOnForbidden, transformIdentifier) + generateCode(arg as Node, failOnForbidden, effectful, transformIdentifier) ); return `${callee}(${args.join(", ")})`; } @@ -99,16 +121,43 @@ const generateCode = ( if (node.type === "Compound") { throw Error("Cannot use multiple expressions"); } + if (node.type === "AssignmentExpression") { + if (node.operator !== "=") { + throw Error(`Only "=" assignment operator is supported`); + } + if (effectful === false) { + throw Error(`Cannot use assignment in this expression`); + } + const left = generateCode( + node.left as Node, + failOnForbidden, + effectful, + // override and mark all identifiers inside of left expression as assignee + (id) => transformIdentifier(id, true) + ); + const right = generateCode( + node.right as Node, + failOnForbidden, + effectful, + transformIdentifier + ); + return `${left} ${node.operator} ${right}`; + } + if (node.type === "UpdateExpression") { + throw Error(`"${node.operator}" operator is not supported`); + } node satisfies never; return ""; }; export const validateExpression = ( code: string, - transformIdentifier: TransformIdentifier = (id) => id + options?: { effectful?: boolean; transformIdentifier?: TransformIdentifier } ) => { + const { effectful = false, transformIdentifier = (id: string) => id } = + options ?? {}; const expression = jsep(code) as Node; - return generateCode(expression, true, transformIdentifier); + return generateCode(expression, true, effectful, transformIdentifier); }; const sortTopologically = ( @@ -135,22 +184,26 @@ const sortTopologically = ( * Generates a function body expecting map as _variables argument * and outputing map of results */ -export const generateExpressionsComputation = ( - variables: Set, - expressions: Map +export const generateComputingExpressions = ( + expressions: Map, + allowedVariables: Set ) => { const depsById = new Map>(); + const inputVariables = new Set(); for (const [id, code] of expressions) { const deps = new Set(); - validateExpression(code, (identifier) => { - if (variables.has(identifier)) { - return identifier; - } - if (expressions.has(identifier)) { - deps.add(identifier); - return identifier; - } - throw Error(`Unknown dependency "${identifier}"`); + validateExpression(code, { + transformIdentifier: (identifier) => { + if (allowedVariables.has(identifier)) { + inputVariables.add(identifier); + return identifier; + } + if (expressions.has(identifier)) { + deps.add(identifier); + return identifier; + } + throw Error(`Unknown dependency "${identifier}"`); + }, }); depsById.set(id, deps); } @@ -163,7 +216,7 @@ export const generateExpressionsComputation = ( // generate code computing all expressions let generatedCode = ""; - for (const id of variables) { + for (const id of inputVariables) { generatedCode += `const ${id} = _variables.get('${id}');\n`; } @@ -184,13 +237,70 @@ export const generateExpressionsComputation = ( return generatedCode; }; -export const executeExpressions = ( - variables: Map, - expressions: Map +export const executeComputingExpressions = ( + expressions: Map, + variables: Map ) => { - const generatedCode = generateExpressionsComputation( - new Set(variables.keys()), - expressions + const generatedCode = generateComputingExpressions( + expressions, + new Set(variables.keys()) + ); + const executeFn = new Function("_variables", generatedCode); + const values = executeFn(variables) as Map; + return values; +}; + +export const generateEffectfulExpression = ( + code: string, + allowedVariables: Set +) => { + const inputVariables = new Set(); + const outputVariables = new Set(); + validateExpression(code, { + effectful: true, + transformIdentifier: (identifier, assignee) => { + if (allowedVariables.has(identifier)) { + if (assignee) { + outputVariables.add(identifier); + } else { + inputVariables.add(identifier); + } + return identifier; + } + throw Error(`Unknown dependency "${identifier}"`); + }, + }); + + // generate code computing all expressions + let generatedCode = ""; + + for (const id of inputVariables) { + generatedCode += `let ${id} = _variables.get('${id}');\n`; + } + for (const id of outputVariables) { + if (inputVariables.has(id) === false) { + generatedCode += `let ${id};\n`; + } + } + + generatedCode += `${code};\n`; + + generatedCode += `return new Map([\n`; + for (const id of outputVariables) { + generatedCode += ` ['${id}', ${id}],\n`; + } + generatedCode += `]);`; + + return generatedCode; +}; + +export const executeEffectfulExpression = ( + code: string, + variables: Map +) => { + const generatedCode = generateEffectfulExpression( + code, + new Set(variables.keys()) ); const executeFn = new Function("_variables", generatedCode); const values = executeFn(variables) as Map; diff --git a/packages/react-sdk/src/index.ts b/packages/react-sdk/src/index.ts index f8d2b90101..5652479769 100644 --- a/packages/react-sdk/src/index.ts +++ b/packages/react-sdk/src/index.ts @@ -22,8 +22,10 @@ export { export { type Params, ReactSdkContext } from "./context"; export { validateExpression, - generateExpressionsComputation, - executeExpressions, + generateComputingExpressions, + executeComputingExpressions, + generateEffectfulExpression, + executeEffectfulExpression, encodeDataSourceVariable, decodeDataSourceVariable, } from "./expression"; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0436f72177..4126126969 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1190,6 +1190,9 @@ importers: packages/react-sdk: dependencies: + '@jsep-plugin/assignment': + specifier: ^1.2.1 + version: 1.2.1(jsep@1.3.8) '@nanostores/react': specifier: ^0.4.1 version: 0.4.1(nanostores@0.7.1)(react@18.2.0) @@ -5316,6 +5319,15 @@ packages: '@jridgewell/resolve-uri': 3.1.0 '@jridgewell/sourcemap-codec': 1.4.14 + /@jsep-plugin/assignment@1.2.1(jsep@1.3.8): + resolution: {integrity: sha512-gaHqbubTi29aZpVbBlECRpmdia+L5/lh2BwtIJTmtxdbecEyyX/ejAOg7eQDGNvGOUmPY7Z2Yxdy9ioyH/VJeA==} + engines: {node: '>= 10.16.0'} + peerDependencies: + jsep: ^0.4.0||^1.0.0 + dependencies: + jsep: 1.3.8 + dev: false + /@juggle/resize-observer@3.4.0: resolution: {integrity: sha512-dfLbk+PwWvFzSxwk3n5ySL0hfBog779o8h68wK/7/APo/7cgyWp5jcXockbxdk5kFRkbeXWm4Fbi9FrdN381sA==} dev: false