refactor: inline expressions into props (#2603)

Ref https://github.com/webstudio-is/webstudio/issues/2532

Collections will introduce nested scope within same component and some
expressions at component scope will not be able to access collection
item. For example

```ts
const PageComponent = () => {
  let [value, set$value] = useState('')
  let show = value === 'error'
  let arialabel = item + ' label'
  return <Form>
    <ErrorMessage data-ws-show={show}></ErrorMessage>
    {list.map(item =>
      <Button aria-label={arialabel}></Button>
    )}
  </Form>
}
```

So the easiest way to workaround it without complicated scope selecting
is just inline all expressions into props.

```ts
const PageComponent = () => {
  let [value, set$value] = useState('')
  return <Form>
    <ErrorMessage data-ws-show={value === 'error'}></ErrorMessage>
    {list.map(item =>
      <Button aria-label={item + ' label'}></Button>
    )}
  </Form>
}
```

## Steps for reproduction

1. Create `Form`
2. Try to change `state` prop
This commit is contained in:
Bogdan Chadkin authored and GitHub committed 2023-11-12 13:07:14 +03:00
1 parent 6dd5b0687a
commit 4e9986d6e3
21 files changed
+121 -170

No files matched your search

@@ -4,6 +4,7 @@ import { type PropMeta, showAttribute } from "@webstudio-is/react-sdk";
import type { PropValue } from "../shared";
import { useStore } from "@nanostores/react";
import {
computeExpression,
dataSourcesLogicStore,
registeredComponentPropsMetasStore,
} from "~/shared/nano-states";
@@ -162,7 +163,7 @@ export const usePropsLogic = ({
const savedProps = props.flatMap((prop) => {
if (prop.type === "expression") {
// convert expression prop to value prop
const dataSourceValue = dataSourcesLogic.get(prop.id);
const dataSourceValue = computeExpression(prop.value, dataSourcesLogic);
return [
{
id: prop.id,
@@ -26,6 +26,7 @@ import {
getIndexesWithinAncestors,
} from "@webstudio-is/react-sdk";
import {
computeExpression,
dataSourcesLogicStore,
instancesStore,
registeredComponentMetasStore,
@@ -164,7 +165,7 @@ const useInstanceProps = (instanceId: Instance["id"]) => {
continue;
}
if (prop.type === "expression") {
const value = dataSourcesLogic.get(prop.id);
const value = computeExpression(prop.value, dataSourcesLogic);
if (value !== undefined) {
instancePropsObject[prop.name] = value;
}
@@ -18,7 +18,10 @@ import {
type StyleSources,
type StyleSourceSelections,
} from "@webstudio-is/sdk";
import { generateDataSources } from "@webstudio-is/react-sdk";
import {
encodeDataSourceVariable,
generateDataSources,
} from "@webstudio-is/react-sdk";
import type { Style } from "@webstudio-is/css-engine";
import type { DragStartPayload } from "~/canvas/shared/use-drag-drop";
import { shallowComputed } from "../store-utils";
@@ -112,6 +115,29 @@ export const dataSourcesLogicStore = computed(
}
);
export const computeExpression = (
expression: string,
variables: Map<DataSource["id"], unknown>
) => {
let code = "";
for (const [id, value] of variables) {
// @todo pass dedicated map of variables without actions
// skip actions
if (typeof value === "function") {
continue;
}
const identifier = encodeDataSourceVariable(id);
code += `const ${identifier} = ${JSON.stringify(value)};\n`;
}
code += `return (${expression})`;
try {
const result = new Function(code)();
return result;
} catch {
// empty block
}
};
export const stylesStore = atom<Styles>(new Map());
export const useInstanceStyles = (instanceId: undefined | Instance["id"]) => {
@@ -73,31 +73,23 @@ export const projectId = "cddc1d44-af37-4cb6-a430-d300cf6f932d";
const Page = (props: { scripts?: ReactNode }) => {
let [formState, set$formState] = useState<any>("initial");
let [formState_1, set$formState_1] = useState<any>("initial");
let state = formState;
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 state_1 = formState_1;
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 (
<Body data-ws-id="a-4nDFkaWy4px1fn38XWJ" data-ws-component="Body">
<Form
data-ws-id="-1RvizaBcVpHsjvnYxn1c"
data-ws-component="Form"
state={state_1}
state={formState_1}
onStateChange={onStateChange_1}
>
{datawsshow_3 && (
{(formState_1 === "initial" || formState_1 === "error") && (
<Box data-ws-id="qhnVrmYGlyrMZi3UzqSQA" data-ws-component="Box">
<Heading
data-ws-id="YdHHf4u3jrdbRIWpB_VfH"
@@ -130,12 +122,12 @@ const Page = (props: { scripts?: ReactNode }) => {
</Button>
</Box>
)}
{datawsshow_4 && (
{formState_1 === "success" && (
<Box data-ws-id="966cjxuqP_T99N27-mqWE" data-ws-component="Box">
{"Thank you for getting in touch!"}
</Box>
)}
{datawsshow_5 && (
{formState_1 === "error" && (
<Box data-ws-id="SYG5hhOz31xFJUN_v9zq6" data-ws-component="Box">
{"Sorry, something went wrong."}
</Box>
@@ -144,12 +136,12 @@ const Page = (props: { scripts?: ReactNode }) => {
<Form
data-ws-id="isNSM3wXcnHFikwNPlEOL"
data-ws-component="Form"
state={state}
state={formState}
onStateChange={onStateChange}
method={"get"}
action={"/custom"}
>
{datawsshow && (
{(formState === "initial" || formState === "error") && (
<Box data-ws-id="a5YPRc19IJyhTrjjasA_R" data-ws-component="Box">
<Heading
data-ws-id="y4pceTmziuBRIDgUBQNLD"
@@ -182,12 +174,12 @@ const Page = (props: { scripts?: ReactNode }) => {
</Button>
</Box>
)}
{datawsshow_1 && (
{formState === "success" && (
<Box data-ws-id="Gw-ta0R4FNFAGBTVRWKep" data-ws-component="Box">
{"Thank you for getting in touch!"}
</Box>
)}
{datawsshow_2 && (
{formState === "error" && (
<Box data-ws-id="ewk_WKpu4syHLPABMmvUz" data-ws-component="Box">
{"Sorry, something went wrong."}
</Box>
@@ -81,7 +81,6 @@ export const projectId = "cddc1d44-af37-4cb6-a430-d300cf6f932d";
const Page = (props: { scripts?: ReactNode }) => {
let [accordionValue, set$accordionValue] = useState<any>("0");
let value = accordionValue;
let onValueChange = (value: any) => {
accordionValue = value;
set$accordionValue(accordionValue);
@@ -92,7 +91,7 @@ const Page = (props: { scripts?: ReactNode }) => {
data-ws-id="AM9fD6dv2Ftc3Xjcsd7Uc"
data-ws-component="@webstudio-is/sdk-components-react-radix:Accordion"
collapsible={true}
value={value}
value={accordionValue}
onValueChange={onValueChange}
>
<AccordionItem
@@ -281,8 +281,8 @@ test("generate jsx element with data sources and action", () => {
<Box
data-ws-id="box"
data-ws-component="Box"
variable={variable}
expression={expression}
variable={variableName}
expression={variableName + 1}
onClick={onClick}
onChange={onChange} />
`)
@@ -358,7 +358,7 @@ test("generate jsx element with condition based on show prop", () => {
})
).toEqual(
clear(`
{datawsshow &&
{(conditionName) &&
<Box
data-ws-id="box"
data-ws-component="Box" />
@@ -529,7 +529,6 @@ test("generate page component with variables and actions", () => {
clear(`
const Page = (props: { scripts?: ReactNode }) => {
let [variableName, set$variableName] = useState<any>("initial")
let value = (variableName);
let onChange = (value: any) => {
variableName = value
set$variableName(variableName)
@@ -541,7 +540,7 @@ test("generate page component with variables and actions", () => {
data-ws-id="input"
data-ws-component="Input"
data-ws-index="0"
value={value}
value={variableName}
onChange={onChange} />
{props.scripts}
</Body>
+64 -35
View File
@@ -4,6 +4,7 @@ import type {
Props,
Scope,
DataSources,
Prop,
} from "@webstudio-is/sdk";
import { parseComponentName } from "@webstudio-is/sdk";
import {
@@ -12,9 +13,56 @@ import {
indexAttribute,
showAttribute,
} from "./tree/webstudio-component";
import { generateDataSources } from "./expression";
import {
decodeDataSourceVariable,
generateDataSources,
validateExpression,
} from "./expression";
import type { IndexesWithinAncestors } from "./instance-utils";
const generatePropValue = ({
scope,
prop,
dataSources,
}: {
scope: Scope;
prop: Prop;
dataSources: DataSources;
}) => {
// ignore asset and page props which are handled by components internally
if (prop.type === "asset" || prop.type === "page") {
return;
}
if (
prop.type === "string" ||
prop.type === "number" ||
prop.type === "boolean" ||
prop.type === "string[]" ||
prop.type === "json"
) {
return JSON.stringify(prop.value);
}
// inline expression to safely use collection item
if (prop.type === "expression") {
return validateExpression(prop.value, {
// transpile to safely executable member expressions
optional: true,
transformIdentifier: (identifier) => {
const depId = decodeDataSourceVariable(identifier);
const dep = depId ? dataSources.get(depId) : undefined;
if (dep) {
return scope.getName(dep.id, dep.name);
}
return identifier;
},
});
}
if (prop.type === "action") {
return scope.getName(prop.id, prop.name);
}
prop satisfies never;
};
export const generateJsxElement = ({
scope,
instance,
@@ -30,7 +78,7 @@ export const generateJsxElement = ({
indexesWithinAncestors: IndexesWithinAncestors;
children: string;
}) => {
let conditionVariableName: undefined | string;
let conditionValue: undefined | string;
let generatedProps = "";
@@ -48,50 +96,31 @@ export const generateJsxElement = ({
if (prop.instanceId !== instance.id) {
continue;
}
const propValue = generatePropValue({ scope, prop, dataSources });
// show prop controls conditional rendering and need to be handled separately
if (prop.name === showAttribute) {
// prevent instance rendering when hidden
if (prop.type === "boolean" && prop.value === false) {
// prevent generating unnecessary condition
if (propValue === "true") {
continue;
}
// prevent instance rendering when always hidden
if (propValue === "false") {
return "";
}
if (prop.type === "expression") {
conditionVariableName = scope.getName(prop.id, prop.name);
}
// ignore any other values
conditionValue = propValue;
// generate separately
continue;
}
if (
prop.type === "string" ||
prop.type === "number" ||
prop.type === "boolean" ||
prop.type === "string[]" ||
prop.type === "json"
) {
generatedProps += `\n${prop.name}={${JSON.stringify(prop.value)}}`;
continue;
if (propValue !== undefined) {
generatedProps += `\n${prop.name}={${propValue}}`;
}
// ignore asset and page props which are handled by components internally
if (prop.type === "asset" || prop.type === "page") {
continue;
}
if (prop.type === "expression") {
const propVariable = scope.getName(prop.id, prop.name);
generatedProps += `\n${prop.name}={${propVariable}}`;
continue;
}
if (prop.type === "action") {
const propVariable = scope.getName(prop.id, prop.name);
generatedProps += `\n${prop.name}={${propVariable}}`;
continue;
}
prop satisfies never;
}
let generatedElement = "";
// coditionally render instance when show prop is data source
// {dataSourceVariable && <Instance>}
if (conditionVariableName) {
generatedElement += `{${conditionVariableName} &&\n`;
if (conditionValue) {
generatedElement += `{(${conditionValue}) &&\n`;
}
const [_namespace, shortName] = parseComponentName(instance.component);
@@ -104,7 +133,7 @@ export const generateJsxElement = ({
generatedElement += `</${componentVariable}>\n`;
}
if (conditionVariableName) {
if (conditionValue) {
generatedElement += `}\n`;
}
-64
View File
@@ -267,67 +267,3 @@ test("generate variables with actions", () => {
])
);
});
test("generate variables witha prop expressions", () => {
const generated = generateDataSources({
scope: createScope(),
dataSources: new Map([
[
"dataSource1",
{
id: "dataSource1",
scopeInstanceId: "instance1",
type: "variable",
name: "myVar",
value: { type: "string", value: "initial" },
},
],
]),
props: new Map([
[
"prop1",
{
id: "prop1",
instanceId: "instance1",
type: "expression",
name: "exp",
value: `$ws$dataSource$dataSource1 + "Name"`,
},
],
[
"prop2",
{
id: "prop2",
instanceId: "instance2",
type: "expression",
name: "exp",
value: `$ws$dataSource$dataSource1 + "Value"`,
},
],
]),
});
expect(generated.body).toMatchInlineSnapshot(`
"let exp = (myVar + "Name");
let exp_1 = (myVar + "Value");
"
`);
expect(generated.variables).toEqual(
new Map([
[
"dataSource1",
{
initialValue: "initial",
setterName: "set$myVar",
valueName: "myVar",
},
],
])
);
expect(generated.output).toEqual(
new Map([
["dataSource1", "myVar"],
["prop1", "exp"],
["prop2", "exp_1"],
])
);
});
-19
View File
@@ -249,25 +249,6 @@ export const generateDataSources = ({
}
for (const prop of props.values()) {
// generate prop expressions
if (prop.type === "expression") {
const name = scope.getName(prop.id, prop.name);
output.set(prop.id, name);
const code = validateExpression(prop.value, {
transformIdentifier: (identifier) => {
const depId = decodeDataSourceVariable(identifier);
const dep = depId ? dataSources.get(depId) : undefined;
if (dep) {
return scope.getName(dep.id, dep.name);
}
// eslint-disable-next-line no-console
console.error(`Unknown dependency "${identifier}"`);
return identifier;
},
});
body += `let ${name} = (${code});\n`;
}
// generate actions assigning variables and invoking their setters
if (prop.type === "action") {
const name = scope.getName(prop.id, prop.name);
@@ -14,7 +14,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [accordionValue, set$accordionValue] = useState<any>("0");
let value = accordionValue;
let onValueChange = (value: any) => {
accordionValue = value;
set$accordionValue(accordionValue);
@@ -25,7 +24,7 @@ const Page = (props: { scripts?: ReactNode }) => {
data-ws-id="1"
data-ws-component="Accordion"
collapsible={true}
value={value}
value={accordionValue}
onValueChange={onValueChange}
>
<AccordionItem
@@ -12,7 +12,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [checkboxChecked, set$checkboxChecked] = useState<any>(false);
let checked = checkboxChecked;
let onCheckedChange = (checked: any) => {
checkboxChecked = checked;
set$checkboxChecked(checkboxChecked);
@@ -23,7 +22,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<Checkbox
data-ws-id="3"
data-ws-component="Checkbox"
checked={checked}
checked={checkboxChecked}
onCheckedChange={onCheckedChange}
>
<CheckboxIndicator
@@ -12,7 +12,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [collapsibleOpen, set$collapsibleOpen] = useState<any>(false);
let open = collapsibleOpen;
let onOpenChange = (open: any) => {
collapsibleOpen = open;
set$collapsibleOpen(collapsibleOpen);
@@ -22,7 +21,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<Collapsible
data-ws-id="1"
data-ws-component="Collapsible"
open={open}
open={collapsibleOpen}
onOpenChange={onOpenChange}
>
<CollapsibleTrigger
@@ -17,7 +17,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [dialogOpen, set$dialogOpen] = useState<any>(false);
let open = dialogOpen;
let onOpenChange = (open: any) => {
dialogOpen = open;
set$dialogOpen(dialogOpen);
@@ -27,7 +26,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<Dialog
data-ws-id="1"
data-ws-component="Dialog"
open={open}
open={dialogOpen}
onOpenChange={onOpenChange}
>
<DialogTrigger data-ws-id="5" data-ws-component="DialogTrigger">
@@ -19,7 +19,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [menuValue, set$menuValue] = useState<any>("");
let value = menuValue;
let onValueChange = (value: any) => {
menuValue = value;
set$menuValue(menuValue);
@@ -29,7 +28,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<NavigationMenu
data-ws-id="1"
data-ws-component="NavigationMenu"
value={value}
value={menuValue}
onValueChange={onValueChange}
>
<NavigationMenuList
@@ -12,7 +12,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [popoverOpen, set$popoverOpen] = useState<any>(false);
let open = popoverOpen;
let onOpenChange = (open: any) => {
popoverOpen = open;
set$popoverOpen(popoverOpen);
@@ -22,7 +21,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<Popover
data-ws-id="1"
data-ws-component="Popover"
open={open}
open={popoverOpen}
onOpenChange={onOpenChange}
>
<PopoverTrigger data-ws-id="5" data-ws-component="PopoverTrigger">
@@ -13,7 +13,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [radioGroupValue, set$radioGroupValue] = useState<any>("");
let value = radioGroupValue;
let onValueChange = (value: any) => {
radioGroupValue = value;
set$radioGroupValue(radioGroupValue);
@@ -23,7 +22,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<RadioGroup
data-ws-id="1"
data-ws-component="RadioGroup"
value={value}
value={radioGroupValue}
onValueChange={onValueChange}
>
<Label data-ws-id="6" data-ws-component="Label">
@@ -17,12 +17,10 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [selectValue, set$selectValue] = useState<any>("");
let [selectOpen, set$selectOpen] = useState<any>(false);
let value = selectValue;
let onValueChange = (value: any) => {
selectValue = value;
set$selectValue(selectValue);
};
let open = selectOpen;
let onOpenChange = (open: any) => {
selectOpen = open;
set$selectOpen(selectOpen);
@@ -32,9 +30,9 @@ const Page = (props: { scripts?: ReactNode }) => {
<Select
data-ws-id="1"
data-ws-component="Select"
value={value}
value={selectValue}
onValueChange={onValueChange}
open={open}
open={selectOpen}
onOpenChange={onOpenChange}
>
<SelectTrigger data-ws-id="8" data-ws-component="SelectTrigger">
@@ -17,7 +17,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [sheetOpen, set$sheetOpen] = useState<any>(false);
let open = sheetOpen;
let onOpenChange = (open: any) => {
sheetOpen = open;
set$sheetOpen(sheetOpen);
@@ -27,7 +26,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<Dialog
data-ws-id="1"
data-ws-component="Dialog"
open={open}
open={sheetOpen}
onOpenChange={onOpenChange}
>
<DialogTrigger data-ws-id="5" data-ws-component="DialogTrigger">
@@ -4,7 +4,6 @@ import { Switch as Switch, SwitchThumb as SwitchThumb } from "../components";
const Page = (props: { scripts?: ReactNode }) => {
let [switchChecked, set$switchChecked] = useState<any>(false);
let checked = switchChecked;
let onCheckedChange = (checked: any) => {
switchChecked = checked;
set$switchChecked(switchChecked);
@@ -14,7 +13,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<Switch
data-ws-id="1"
data-ws-component="Switch"
checked={checked}
checked={switchChecked}
onCheckedChange={onCheckedChange}
>
<SwitchThumb data-ws-id="6" data-ws-component="SwitchThumb" />
@@ -9,7 +9,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [tabsValue, set$tabsValue] = useState<any>("0");
let value = tabsValue;
let onValueChange = (value: any) => {
tabsValue = value;
set$tabsValue(tabsValue);
@@ -19,7 +18,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<Tabs
data-ws-id="1"
data-ws-component="Tabs"
value={value}
value={tabsValue}
onValueChange={onValueChange}
>
<TabsList data-ws-id="5" data-ws-component="TabsList">
@@ -12,7 +12,6 @@ import {
const Page = (props: { scripts?: ReactNode }) => {
let [tooltipOpen, set$tooltipOpen] = useState<any>(false);
let open = tooltipOpen;
let onOpenChange = (open: any) => {
tooltipOpen = open;
set$tooltipOpen(tooltipOpen);
@@ -22,7 +21,7 @@ const Page = (props: { scripts?: ReactNode }) => {
<Tooltip
data-ws-id="1"
data-ws-component="Tooltip"
open={open}
open={tooltipOpen}
onOpenChange={onOpenChange}
>
<TooltipTrigger data-ws-id="5" data-ws-component="TooltipTrigger">