Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion apps/builder/app/builder/shared/binding-popover.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { expect, test } from "vitest";
import { encodeDataSourceVariable } from "@webstudio-is/sdk";
import { evaluateExpressionWithinScope } from "./binding-popover";
import {
evaluateExpressionWithinScope,
validateExpressionScope,
} from "./binding-popover";

test("evaluateExpressionWithinScope works", () => {
const variableName = "jsonVariable";
Expand All @@ -13,3 +16,15 @@ test("evaluateExpressionWithinScope works", () => {
})
).toEqual(2);
});

test("validateExpressionScope reports unavailable variables", () => {
const availableVariable = encodeDataSourceVariable("available");
const missingVariable = encodeDataSourceVariable("missing");

expect(
validateExpressionScope(
`${availableVariable} + ${missingVariable}`,
new Map([[availableVariable, "available"]])
)
).toEqual(`"${missingVariable}" is not defined in the scope`);
});
15 changes: 14 additions & 1 deletion apps/builder/app/builder/shared/binding-popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,17 @@ export const evaluateExpressionWithinScope = (
return computeExpression(expression, variables);
};

export const validateExpressionScope = (
expression: string,
aliases: Map<string, string>
) => {
const diagnostics = lintExpression({
expression,
availableVariables: new Set(aliases.keys()),
});
return diagnostics[0]?.message;
};

const BindingPanel = ({
scope,
aliases,
Expand Down Expand Up @@ -385,7 +396,9 @@ export const BindingPopover = ({
return;
}

const valueError = validate?.(evaluateExpressionWithinScope(value, scope));
const valueError =
validateExpressionScope(value, aliases) ??
validate?.(evaluateExpressionWithinScope(value, scope));
return (
<FloatingPanel
placement="left-start"
Expand Down
14 changes: 8 additions & 6 deletions apps/builder/app/shared/data-variables.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ test("find global variables", () => {
]);
});

test("find global variables in slots", () => {
test("find parent variables in slots", () => {
const globalVariable = new Variable("globalVariable", "");
const bodyVariable = new Variable("bodyVariable", "");
const boxVariable = new Variable("boxVariable", "");
Expand All @@ -128,6 +128,7 @@ test("find global variables in slots", () => {
).toEqual([
expect.objectContaining({ name: "system", id: SYSTEM_VARIABLE_ID }),
expect.objectContaining({ name: "globalVariable" }),
expect.objectContaining({ name: "bodyVariable" }),
expect.objectContaining({ name: "boxVariable" }),
]);
});
Expand Down Expand Up @@ -569,7 +570,7 @@ test("preserve other variables when rebind", () => {
]);
});

test("prevent rebinding tree variables from slots", () => {
test("rebind tree variables from slot ancestors", () => {
const bodyVariable = new Variable("myVariable", "one value of body");
const data = renderData(
<$.Body ws:id="bodyId" data-body-vars={expression`${bodyVariable}`}>
Expand All @@ -585,8 +586,9 @@ test("prevent rebinding tree variables from slots", () => {
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
const [bodyVariableId] = data.dataSources.keys();
expect(data.instances.get("boxId")?.children).toEqual([
{ type: "expression", value: "myVariable" },
{ type: "expression", value: encodeDataVariableId(bodyVariableId) },
]);
});

Expand Down Expand Up @@ -744,7 +746,7 @@ test("rebind expressions with parent variable when delete variable on child", ()
]);
});

test("prevent rebinding with variables outside of slot content scope", () => {
test("rebind with variables outside of slot content scope", () => {
const bodyVariable = new Variable("myVariable", "one value of body");
const boxVariable = new Variable("myVariable", "one value of body");
const data = renderData(
Expand All @@ -762,10 +764,10 @@ test("prevent rebinding with variables outside of slot content scope", () => {
expect.objectContaining({ scopeInstanceId: "bodyId" }),
expect.objectContaining({ scopeInstanceId: "boxId" }),
]);
const [_bodyVariableId, boxVariableId] = data.dataSources.keys();
const [bodyVariableId, boxVariableId] = data.dataSources.keys();
deleteVariableMutable(data, boxVariableId);
expect(data.instances.get("textId")?.children).toEqual([
{ type: "expression", value: "myVariable" },
{ type: "expression", value: encodeDataVariableId(bodyVariableId) },
]);
});

Expand Down
4 changes: 0 additions & 4 deletions apps/builder/app/shared/data-variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,6 @@ export const computeExpression = (
const getParentInstanceById = (instances: Instances) => {
const parentInstanceById = new Map<Instance["id"], Instance["id"]>();
for (const instance of instances.values()) {
// interrupt lookup because slot variables cannot be passed to slot content
if (instance.component === "Slot") {
continue;
}
for (const child of instance.children) {
if (child.type === "id") {
parentInstanceById.set(child.value, instance.id);
Expand Down
10 changes: 5 additions & 5 deletions apps/builder/app/shared/nano-states/props.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ test("compute resource variable values", () => {
).toEqual("my-value");
});

test("stop variables lookup outside of slots", () => {
test("inherit variables from outside of slots", () => {
const bodyVariable = new Variable("bodyVariable", "body");
const slotVariable = new Variable("slotVariable", "slot");
const boxVariable = new Variable("boxVariable", "box");
Expand All @@ -1041,7 +1041,7 @@ test("stop variables lookup outside of slots", () => {
values.get(
getInstanceKey(["fragmentId", "slotId", "bodyId", ROOT_INSTANCE_ID])
)?.size
).toEqual(1);
).toEqual(3);
expect(
values.get(
getInstanceKey([
Expand All @@ -1052,8 +1052,7 @@ test("stop variables lookup outside of slots", () => {
ROOT_INSTANCE_ID,
])
)?.size
// global system and box variable
).toEqual(2);
).toEqual(4);
});

test("compute parameter and resource variables without values to make it available in scope", () => {
Expand Down Expand Up @@ -1259,7 +1258,7 @@ test("inherit variables from global root inside slots", () => {
$instances.set(data.instances);
$dataSources.set(data.dataSources);
$props.set(data.props);
const [rootVariableId, _bodyVariableId, boxVariableId] =
const [rootVariableId, bodyVariableId, boxVariableId] =
data.dataSources.keys();
selectPageRoot("bodyId");
expect(
Expand All @@ -1278,6 +1277,7 @@ test("inherit variables from global root inside slots", () => {
new Map<string, unknown>([
[SYSTEM_VARIABLE_ID, initialSystem],
[rootVariableId, "root"],
[bodyVariableId, "body"],
[boxVariableId, "box"],
])
);
Expand Down
7 changes: 0 additions & 7 deletions apps/builder/app/shared/nano-states/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
encodeDataSourceVariable,
transpileExpression,
collectionComponent,
portalComponent,
ROOT_INSTANCE_ID,
SYSTEM_VARIABLE_ID,
findTreeInstanceIds,
Expand Down Expand Up @@ -548,12 +547,6 @@ export const $variableValuesByInstanceSelector = computed(
}
return;
}
// reset values for slot children to let slots behave as isolated components
if (instance.component === portalComponent) {
// allow accessing global variables in slots
variableValues = globalVariableValues;
variableNames = globalVariableNames;
}
for (const child of instance.children) {
if (child.type === "id") {
traverseInstances(
Expand Down
Loading