Skip to content

Commit 6b04c97

Browse files
TrySoundTrySound
authored
feat: allow saving expressions with unset variables (#4792)
Ref #4768 #4753 User can paste expression with not defined variable and should get it saved in case of accidental closing popover. Now such expressions can be executed in builder and will be compiled with undefined instead of variables in published sites. In the future we need to add global diagnostics to let users find such places faster. Also fixed a couple of ui issues - react error about using button inside of button in data variables list - react error about using unknown middlewareData prop from floating-ui <img width="307" alt="Screenshot 2025-01-27 at 16 30 58" src="https://github.com/user-attachments/assets/2ddc9ead-33a2-4438-b641-6943d7472f7e" />
1 parent 0586f6e commit 6b04c97

File tree

15 files changed

+240
-143
lines changed

15 files changed

+240
-143
lines changed

‎apps/builder/app/builder/features/pages/custom-metadata.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ import {
1313
import { TrashIcon, PlusIcon } from "@webstudio-is/icons";
1414
import { isFeatureEnabled } from "@webstudio-is/feature-flags";
1515
import { isLiteralExpression } from "@webstudio-is/sdk";
16+
import { computeExpression } from "~/shared/data-variables";
1617
import {
1718
BindingControl,
1819
BindingPopover,
1920
} from "~/builder/shared/binding-popover";
20-
import { computeExpression } from "~/shared/nano-states";
2121
import { $pageRootScope } from "./page-utils";
2222

2323
type Meta = {

‎apps/builder/app/builder/features/pages/page-settings.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ import {
7474
$instances,
7575
$pages,
7676
$dataSources,
77-
computeExpression,
7877
$dataSourceVariables,
7978
$publishedOrigin,
8079
$project,
@@ -112,6 +111,7 @@ import type { UserPlanFeatures } from "~/shared/db/user-plan-features.server";
112111
import { useUnmount } from "~/shared/hook-utils/use-mount";
113112
import { Card } from "../marketplace/card";
114113
import { selectInstance } from "~/shared/awareness";
114+
import { computeExpression } from "~/shared/data-variables";
115115

116116
const fieldDefaultValues = {
117117
name: "Untitled",

‎apps/builder/app/builder/features/settings-panel/variable-popover.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ BooleanForm.displayName = "BooleanForm";
442442

443443
const validateJsonValue = (expression: string) => {
444444
const diagnostics = lintExpression({ expression });
445+
// prevent saving with any message including unset variable
445446
return diagnostics.length > 0 ? "error" : "";
446447
};
447448

‎apps/builder/app/builder/features/settings-panel/variables-section.tsx

+4-2
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,10 @@ const VariablesItem = ({
220220
id={variable.id}
221221
index={index}
222222
label={
223-
<Flex align="center" css={{}}>
224-
<Label color={source}>{variable.name}</Label>
223+
<Flex align="center">
224+
<Label tag="label" color={source}>
225+
{variable.name}
226+
</Label>
225227
{value !== undefined && (
226228
<span className={variableLabelStyle.toString()}>
227229
&nbsp;

‎apps/builder/app/builder/shared/binding-popover.tsx

+5-6
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,13 @@ import {
3838
getExpressionIdentifiers,
3939
lintExpression,
4040
} from "@webstudio-is/sdk";
41+
import { $dataSourceVariables, $isDesignMode } from "~/shared/nano-states";
42+
import { computeExpression } from "~/shared/data-variables";
4143
import {
4244
ExpressionEditor,
4345
formatValuePreview,
4446
type EditorApi,
4547
} from "./expression-editor";
46-
import {
47-
$dataSourceVariables,
48-
$isDesignMode,
49-
computeExpression,
50-
} from "~/shared/nano-states";
5148

5249
export const evaluateExpressionWithinScope = (
5350
expression: string,
@@ -94,7 +91,9 @@ const BindingPanel = ({
9491
expression,
9592
availableVariables: new Set(aliases.keys()),
9693
});
97-
setErrorsCount(diagnostics.length);
94+
// prevent saving expression only with syntax error
95+
const errors = diagnostics.filter((item) => item.severity === "error");
96+
setErrorsCount(errors.length);
9897
};
9998

10099
const updateExpression = (newExpression: string) => {

‎apps/builder/app/builder/shared/code-editor-base.tsx

+4
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,10 @@ const editorContentStyle = css({
122122
textDecoration: "underline wavy red",
123123
backgroundColor: "rgba(255, 0, 0, 0.1)",
124124
},
125+
".cm-lintRange-warning": {
126+
textDecoration: "underline wavy orange",
127+
backgroundColor: "rgba(255, 0, 0, 0.1)",
128+
},
125129
".cm-gutters": {
126130
backgroundColor: "transparent",
127131
border: 0,

‎apps/builder/app/shared/data-variables.test.ts

+58-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { expect, test } from "vitest";
1+
import { expect, test, vi } from "vitest";
22
import {
3+
computeExpression,
34
decodeDataVariableName,
45
encodeDataVariableName,
56
restoreExpressionVariables,
@@ -56,3 +57,59 @@ test("restore expression variables", () => {
5657
})
5758
).toEqual("$ws$dataSource$myId + missingVariable");
5859
});
60+
61+
test("compute expression with decoded ids", () => {
62+
expect(
63+
computeExpression("$ws$dataSource$myId", new Map([["myId", "value"]]))
64+
).toEqual("value");
65+
});
66+
67+
test("compute expression with decoded names", () => {
68+
expect(
69+
computeExpression("My$32$Name", new Map([["My Name", "value"]]))
70+
).toEqual("value");
71+
});
72+
73+
test("compute expression when invalid syntax", () => {
74+
// prevent error message in test report
75+
const spy = vi.spyOn(console, "error");
76+
spy.mockImplementationOnce(() => {});
77+
expect(computeExpression("https://github.com", new Map())).toEqual(undefined);
78+
expect(spy).toHaveBeenCalledOnce();
79+
spy.mockRestore();
80+
});
81+
82+
test("compute expression with nested field of undefined without error", () => {
83+
const spy = vi.spyOn(console, "error");
84+
const variables = new Map([["myVariable", undefined]]);
85+
expect(computeExpression("myVariable.field", variables)).toEqual(undefined);
86+
expect(spy).not.toHaveBeenCalled();
87+
spy.mockRestore();
88+
});
89+
90+
test("compute literal expression when variable is json object", () => {
91+
const jsonObject = { hello: "world", subObject: { world: "hello" } };
92+
const variables = new Map([["jsonVariable", jsonObject]]);
93+
expect(computeExpression("`${jsonVariable}`", variables)).toEqual(
94+
`{"hello":"world","subObject":{"world":"hello"}}`
95+
);
96+
expect(computeExpression("`${jsonVariable.subObject}`", variables)).toEqual(
97+
`{"world":"hello"}`
98+
);
99+
});
100+
101+
test("compute literal expression when object is frozen", () => {
102+
const jsonObject = Object.freeze({
103+
hello: "world",
104+
subObject: { world: "hello" },
105+
});
106+
const variables = new Map([["jsonVariable", jsonObject]]);
107+
expect(computeExpression("`${jsonVariable.subObject}`", variables)).toEqual(
108+
`{"world":"hello"}`
109+
);
110+
});
111+
112+
test("compute unset variables as undefined", () => {
113+
expect(computeExpression(`a`, new Map())).toEqual(undefined);
114+
expect(computeExpression("`${a}`", new Map())).toEqual("undefined");
115+
});

‎apps/builder/app/shared/data-variables.ts

+81
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import {
44
encodeDataVariableId,
55
transpileExpression,
66
} from "@webstudio-is/sdk";
7+
import {
8+
createJsonStringifyProxy,
9+
isPlainObject,
10+
} from "@webstudio-is/sdk/runtime";
711

812
const allowedJsChars = /[A-Za-z_]/;
913

@@ -92,3 +96,80 @@ export const restoreExpressionVariables = ({
9296
return expression;
9397
}
9498
};
99+
100+
export const computeExpression = (
101+
expression: string,
102+
variables: Map<DataSource["name"], unknown>
103+
) => {
104+
try {
105+
const usedVariables = new Map();
106+
const transpiled = transpileExpression({
107+
expression,
108+
executable: true,
109+
replaceVariable: (identifier) => {
110+
const id = decodeDataVariableId(identifier);
111+
if (id) {
112+
usedVariables.set(identifier, id);
113+
} else {
114+
// access all variable values from specified map
115+
const name = decodeDataVariableName(identifier);
116+
usedVariables.set(identifier, name);
117+
}
118+
},
119+
});
120+
let code = "";
121+
// add only used variables in expression and get values
122+
// from variables map without additional serializing of these values
123+
for (const [identifier, name] of usedVariables) {
124+
code += `let ${identifier} = _variables.get(${JSON.stringify(name)});\n`;
125+
}
126+
code += `return (${transpiled})`;
127+
128+
/**
129+
*
130+
* We are using structuredClone on frozen values because, for some reason,
131+
* the Proxy example below throws a cryptic error:
132+
* TypeError: 'get' on proxy: property 'data' is a read-only and non-configurable
133+
* data property on the proxy target, but the proxy did not return its actual value
134+
* (expected '[object Array]' but got '[object Array]').
135+
*
136+
* ```
137+
* const createJsonStringifyProxy = (target) => {
138+
* return new Proxy(target, {
139+
* get(target, prop, receiver) {
140+
*
141+
* console.log((prop in target), prop)
142+
*
143+
* const value = Reflect.get(target, prop, receiver);
144+
*
145+
* if (typeof value === "object" && value !== null) {
146+
* return createJsonStringifyProxy(value);
147+
* }
148+
*
149+
* return value;
150+
* },
151+
* });
152+
* };
153+
* const obj = Object.freeze({ data: [1, 2, 3, 4] });
154+
* const proxy = createJsonStringifyProxy(obj)
155+
* proxy.data
156+
*
157+
* ```
158+
*/
159+
const proxiedVariables = new Map(
160+
[...variables.entries()].map(([key, value]) => [
161+
key,
162+
isPlainObject(value)
163+
? createJsonStringifyProxy(
164+
Object.isFrozen(value) ? structuredClone(value) : value
165+
)
166+
: value,
167+
])
168+
);
169+
170+
const result = new Function("_variables", code)(proxiedVariables);
171+
return result;
172+
} catch (error) {
173+
console.error(error);
174+
}
175+
};

‎apps/builder/app/shared/nano-states/props.test.ts

+1-27
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,12 @@ import { beforeEach, expect, test } from "vitest";
22
import { cleanStores } from "nanostores";
33
import { createDefaultPages } from "@webstudio-is/project-build";
44
import { setEnv } from "@webstudio-is/feature-flags";
5-
import {
6-
type Instance,
7-
encodeDataSourceVariable,
8-
collectionComponent,
9-
} from "@webstudio-is/sdk";
5+
import { type Instance, collectionComponent } from "@webstudio-is/sdk";
106
import { textContentAttribute } from "@webstudio-is/react-sdk";
117
import { $instances } from "./instances";
128
import {
139
$propValuesByInstanceSelector,
1410
$variableValuesByInstanceSelector,
15-
computeExpression,
1611
} from "./props";
1712
import { $pages } from "./pages";
1813
import { $assets, $dataSources, $props, $resources } from "./misc";
@@ -978,24 +973,3 @@ test("prefill default system variable value", () => {
978973

979974
cleanStores($variableValuesByInstanceSelector);
980975
});
981-
982-
test("compute expression when invalid syntax", () => {
983-
expect(computeExpression("https://github.com", new Map())).toEqual(undefined);
984-
});
985-
986-
test("compute literal expression when variable is json object", () => {
987-
const variableName = "jsonVariable";
988-
const encVariableName = encodeDataSourceVariable(variableName);
989-
const jsonObject = { hello: "world", subObject: { world: "hello" } };
990-
const variables = new Map([[variableName, jsonObject]]);
991-
const expression = "`${" + encVariableName + "}`";
992-
993-
expect(computeExpression(expression, variables)).toEqual(
994-
JSON.stringify(jsonObject)
995-
);
996-
997-
const subObjectExpression = "`${" + encVariableName + ".subObject}`";
998-
expect(computeExpression(subObjectExpression, variables)).toEqual(
999-
JSON.stringify(jsonObject.subObject)
1000-
);
1001-
});

‎apps/builder/app/shared/nano-states/props.ts

+1-77
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,6 @@ import {
1515
collectionComponent,
1616
portalComponent,
1717
} from "@webstudio-is/sdk";
18-
import {
19-
createJsonStringifyProxy,
20-
isPlainObject,
21-
} from "@webstudio-is/sdk/runtime";
2218
import { normalizeProps, textContentAttribute } from "@webstudio-is/react-sdk";
2319
import { isFeatureEnabled } from "@webstudio-is/feature-flags";
2420
import { mapGroupBy } from "~/shared/shim";
@@ -44,6 +40,7 @@ import {
4440
import { uploadingFileDataToAsset } from "~/builder/shared/assets/asset-utils";
4541
import { fetch } from "~/shared/fetch.client";
4642
import { $selectedPage, getInstanceKey } from "../awareness";
43+
import { computeExpression } from "../data-variables";
4744

4845
export const assetBaseUrl = "/cgi/asset/";
4946

@@ -204,79 +201,6 @@ const $loaderVariableValues = computed(
204201
}
205202
);
206203

207-
export const computeExpression = (
208-
expression: string,
209-
variables: Map<DataSource["id"], unknown>
210-
) => {
211-
try {
212-
const usedVariables = new Map();
213-
const transpiled = transpileExpression({
214-
expression,
215-
executable: true,
216-
replaceVariable: (identifier) => {
217-
const id = decodeDataSourceVariable(identifier);
218-
if (id) {
219-
usedVariables.set(identifier, id);
220-
}
221-
},
222-
});
223-
let code = "";
224-
// add only used variables in expression and get values
225-
// from variables map without additional serializing of these values
226-
for (const [identifier, id] of usedVariables) {
227-
code += `let ${identifier} = _variables.get("${id}");\n`;
228-
}
229-
code += `return (${transpiled})`;
230-
231-
/**
232-
*
233-
* We are using structuredClone on frozen values because, for some reason,
234-
* the Proxy example below throws a cryptic error:
235-
* TypeError: 'get' on proxy: property 'data' is a read-only and non-configurable
236-
* data property on the proxy target, but the proxy did not return its actual value
237-
* (expected '[object Array]' but got '[object Array]').
238-
*
239-
* ```
240-
* const createJsonStringifyProxy = (target) => {
241-
* return new Proxy(target, {
242-
* get(target, prop, receiver) {
243-
*
244-
* console.log((prop in target), prop)
245-
*
246-
* const value = Reflect.get(target, prop, receiver);
247-
*
248-
* if (typeof value === "object" && value !== null) {
249-
* return createJsonStringifyProxy(value);
250-
* }
251-
*
252-
* return value;
253-
* },
254-
* });
255-
* };
256-
* const obj = Object.freeze({ data: [1, 2, 3, 4] });
257-
* const proxy = createJsonStringifyProxy(obj)
258-
* proxy.data
259-
*
260-
* ```
261-
*/
262-
const proxiedVariables = new Map(
263-
[...variables.entries()].map(([key, value]) => [
264-
key,
265-
isPlainObject(value)
266-
? createJsonStringifyProxy(
267-
Object.isFrozen(value) ? structuredClone(value) : value
268-
)
269-
: value,
270-
])
271-
);
272-
273-
const result = new Function("_variables", code)(proxiedVariables);
274-
return result;
275-
} catch (error) {
276-
console.error(error);
277-
}
278-
};
279-
280204
/**
281205
* compute prop values within context of instance ancestors
282206
* like a dry-run of rendering and accessing react contexts deep in the tree

0 commit comments

Comments
 (0)