From 0d735c2bf8917fe6887478a2bd63706878ea47cd Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Mon, 6 Nov 2023 13:14:34 +0300 Subject: [PATCH] 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 --- .../props-section/use-props-logic.ts | 3 +- .../webstudio-component.tsx | 6 +- apps/builder/app/shared/instance-utils.ts | 3 +- .../app/__generated__/[form]._index.tsx | 24 +-- .../react-sdk/src/component-generator.test.ts | 2 +- packages/react-sdk/src/component-generator.ts | 20 ++- packages/react-sdk/src/expression.test.ts | 35 +++- packages/react-sdk/src/expression.ts | 153 +++++++----------- .../src/__generated__/accordion.stories.tsx | 4 +- .../src/__generated__/checkbox.stories.tsx | 4 +- .../src/__generated__/collapsible.stories.tsx | 4 +- .../src/__generated__/dialog.stories.tsx | 4 +- .../__generated__/navigation-menu.stories.tsx | 4 +- .../src/__generated__/popover.stories.tsx | 4 +- .../src/__generated__/radio-group.stories.tsx | 4 +- .../src/__generated__/select.stories.tsx | 8 +- .../src/__generated__/sheet.stories.tsx | 4 +- .../src/__generated__/switch.stories.tsx | 4 +- .../src/__generated__/tabs.stories.tsx | 4 +- .../src/__generated__/tooltip.stories.tsx | 4 +- 20 files changed, 152 insertions(+), 146 deletions(-) diff --git a/apps/builder/app/builder/features/settings-panel/props-section/use-props-logic.ts b/apps/builder/app/builder/features/settings-panel/props-section/use-props-logic.ts index a5680a67fa..a62cf11150 100644 --- a/apps/builder/app/builder/features/settings-panel/props-section/use-props-logic.ts +++ b/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 []; } diff --git a/apps/builder/app/canvas/features/webstudio-component/webstudio-component.tsx b/apps/builder/app/canvas/features/webstudio-component/webstudio-component.tsx index c73c3c980c..b3cb5f5f3f 100644 --- a/apps/builder/app/canvas/features/webstudio-component/webstudio-component.tsx +++ b/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; } diff --git a/apps/builder/app/shared/instance-utils.ts b/apps/builder/app/shared/instance-utils.ts index bb550af84b..48fec784d7 100644 --- a/apps/builder/app/shared/instance-utils.ts +++ b/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, diff --git a/fixtures/webstudio-remix-vercel/app/__generated__/[form]._index.tsx b/fixtures/webstudio-remix-vercel/app/__generated__/[form]._index.tsx index 45202a83c7..95f766ca38 100644 --- a/fixtures/webstudio-remix-vercel/app/__generated__/[form]._index.tsx +++ b/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("initial"); let [formState_1, set$formState_1] = useState("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 (
{ state={formState_1} onStateChange={onStateChange_1} > - {formInitial_1 && ( + {datawsshow_3 && ( { )} - {formSuccess_1 && ( + {datawsshow_4 && ( {"Thank you for getting in touch!"} )} - {formError_1 && ( + {datawsshow_5 && ( {"Sorry, something went wrong."} @@ -147,7 +147,7 @@ const Page = (props: { scripts?: ReactNode }) => { method={"get"} action={"/custom"} > - {formInitial && ( + {datawsshow && ( { )} - {formSuccess && ( + {datawsshow_1 && ( {"Thank you for getting in touch!"} )} - {formError && ( + {datawsshow_2 && ( {"Sorry, something went wrong."} diff --git a/packages/react-sdk/src/component-generator.test.ts b/packages/react-sdk/src/component-generator.test.ts index b7a1ddf5b7..b5e24f1a06 100644 --- a/packages/react-sdk/src/component-generator.test.ts +++ b/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} /> `) diff --git a/packages/react-sdk/src/component-generator.ts b/packages/react-sdk/src/component-generator.ts index 4f691d40c0..db810cdc55 100644 --- a/packages/react-sdk/src/component-generator.ts +++ b/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") { diff --git a/packages/react-sdk/src/expression.test.ts b/packages/react-sdk/src/expression.test.ts index afbe0d01d8..c3a0be72df 100644 --- a/packages/react-sdk/src/expression.test.ts +++ b/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"], ]) ); }); diff --git a/packages/react-sdk/src/expression.ts b/packages/react-sdk/src/expression.ts index 4cb8fa0b8d..3f1f28fba4 100644 --- a/packages/react-sdk/src/expression.ts +++ b/packages/react-sdk/src/expression.ts @@ -161,26 +161,6 @@ export const validateExpression = ( return generateCode(expression, true, effectful, transformIdentifier); }; -const sortTopologically = ( - list: Set, - depsById: Map>, - explored = new Set(), - 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, expressionId: string, @@ -293,39 +273,8 @@ export const generateDataSources = ({ let body = ""; const output = new Map(); - // collect each data source depndencies to sort topologically - // and replace data sources with scoped names - const depsById = new Map>(); - const codeById = new Map(); for (const dataSource of dataSources.values()) { - if (dataSource.type === "expression") { - const deps = new Set(); - 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(); - 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(); + 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 { diff --git a/packages/sdk-components-react-radix/src/__generated__/accordion.stories.tsx b/packages/sdk-components-react-radix/src/__generated__/accordion.stories.tsx index 5486582d45..90533d0fac 100644 --- a/packages/sdk-components-react-radix/src/__generated__/accordion.stories.tsx +++ b/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("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} > { let [checkboxChecked, set$checkboxChecked] = useState(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 }) => { { let [collapsibleOpen, set$collapsibleOpen] = useState(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 }) => { { let [dialogOpen, set$dialogOpen] = useState(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 }) => { diff --git a/packages/sdk-components-react-radix/src/__generated__/navigation-menu.stories.tsx b/packages/sdk-components-react-radix/src/__generated__/navigation-menu.stories.tsx index 6cb8e7817e..55a187a6fd 100644 --- a/packages/sdk-components-react-radix/src/__generated__/navigation-menu.stories.tsx +++ b/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(""); - let expression = menuValue; + let value = menuValue; let onValueChange = (value: any) => { menuValue = value; set$menuValue(menuValue); @@ -29,7 +29,7 @@ const Page = (props: { scripts?: ReactNode }) => { { let [popoverOpen, set$popoverOpen] = useState(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 }) => { diff --git a/packages/sdk-components-react-radix/src/__generated__/radio-group.stories.tsx b/packages/sdk-components-react-radix/src/__generated__/radio-group.stories.tsx index db81a792bc..87bfbf10aa 100644 --- a/packages/sdk-components-react-radix/src/__generated__/radio-group.stories.tsx +++ b/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(""); - let expression = radioGroupValue; + let value = radioGroupValue; let onValueChange = (value: any) => { radioGroupValue = value; set$radioGroupValue(radioGroupValue); @@ -23,7 +23,7 @@ const Page = (props: { scripts?: ReactNode }) => {