Skip to content

fix: rebind variables within scope #4927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 27, 2025
Merged

fix: rebind variables within scope #4927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
111 changes: 105 additions & 6 deletions apps/builder/app/shared/data-variables.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,11 @@ test("restore tree variables in children", () => {
</$.Box>
</$.Body>
);
rebindTreeVariablesMutable({ startingInstanceId: "boxId", ...data });
rebindTreeVariablesMutable({
startingInstanceId: "boxId",
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
expect(Array.from(data.dataSources.values())).toEqual([
expect.objectContaining({ scopeInstanceId: "bodyId" }),
expect.objectContaining({ scopeInstanceId: "boxId" }),
Expand Down Expand Up @@ -290,7 +294,11 @@ test("restore tree variables in props", () => {
</$.Box>
</$.Body>
);
rebindTreeVariablesMutable({ startingInstanceId: "boxId", ...data });
rebindTreeVariablesMutable({
startingInstanceId: "boxId",
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
const [_bodyVariableId, boxOneVariableId, boxTwoVariableId] =
data.dataSources.keys();
const boxOneIdentifier = encodeDataVariableId(boxOneVariableId);
Expand Down Expand Up @@ -337,7 +345,11 @@ test("rebind tree variables in props and children", () => {
</$.Box>
</$.Body>
);
rebindTreeVariablesMutable({ startingInstanceId: "boxId", ...data });
rebindTreeVariablesMutable({
startingInstanceId: "boxId",
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
expect(Array.from(data.dataSources.values())).toEqual([
expect.objectContaining({ scopeInstanceId: "bodyId" }),
expect.objectContaining({ scopeInstanceId: "boxId" }),
Expand All @@ -358,6 +370,32 @@ test("rebind tree variables in props and children", () => {
]);
});

test("preserve nested variables with the same name when rebind", () => {
const bodyVariable = new Variable("one", "one value of body");
const textVariable = new Variable("one", "one value of box");
const data = renderData(
<$.Body ws:id="bodyId" data-body-vars={expression`${bodyVariable}`}>
<$.Text ws:id="textId" data-text-vars={expression`${textVariable}`}>
{expression`${textVariable}`}
</$.Text>
</$.Body>
);
rebindTreeVariablesMutable({
startingInstanceId: "bodyId",
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
expect(Array.from(data.dataSources.values())).toEqual([
expect.objectContaining({ scopeInstanceId: "bodyId" }),
expect.objectContaining({ scopeInstanceId: "textId" }),
]);
const [_bodyVariableId, textVariableId] = data.dataSources.keys();
const textIdentifier = encodeDataVariableId(textVariableId);
expect(data.instances.get("textId")?.children).toEqual([
{ type: "expression", value: textIdentifier },
]);
});

test("restore tree variables in resources", () => {
const bodyVariable = new Variable("one", "one value of body");
const boxVariable = new Variable("one", "one value of box");
Expand All @@ -384,7 +422,11 @@ test("restore tree variables in resources", () => {
</$.Box>
</$.Body>
);
rebindTreeVariablesMutable({ startingInstanceId: "boxId", ...data });
rebindTreeVariablesMutable({
startingInstanceId: "boxId",
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
expect(Array.from(data.dataSources.values())).toEqual([
expect.objectContaining({ scopeInstanceId: "bodyId" }),
expect.objectContaining({ scopeInstanceId: "boxId" }),
Expand Down Expand Up @@ -434,7 +476,11 @@ test("rebind tree variables in resources", () => {
</$.Box>
</$.Body>
);
rebindTreeVariablesMutable({ startingInstanceId: "boxId", ...data });
rebindTreeVariablesMutable({
startingInstanceId: "boxId",
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
expect(Array.from(data.dataSources.values())).toEqual([
expect.objectContaining({ scopeInstanceId: "bodyId" }),
expect.objectContaining({ scopeInstanceId: "boxId" }),
Expand All @@ -458,6 +504,55 @@ test("rebind tree variables in resources", () => {
]);
});

test("rebind global variables in resources", () => {
const globalVariable = new Variable("globalVariable", "");
const data = renderData(
<ws.root ws:id={ROOT_INSTANCE_ID} data-vars={expression`${globalVariable}`}>
<$.Body ws:id="bodyId">
<$.Text ws:id="textId">{expression`globalVariable`}</$.Text>
</$.Body>
</ws.root>
);
data.instances.delete(ROOT_INSTANCE_ID);
rebindTreeVariablesMutable({
startingInstanceId: ROOT_INSTANCE_ID,
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
expect(Array.from(data.dataSources.values())).toEqual([
expect.objectContaining({ scopeInstanceId: ROOT_INSTANCE_ID }),
]);
const [globalVariableId] = data.dataSources.keys();
const globalIdentifier = encodeDataVariableId(globalVariableId);
expect(data.instances.get("textId")?.children).toEqual([
{ type: "expression", value: globalIdentifier },
]);
});

test("preserve other variables when rebind", () => {
const bodyVariable = new Variable("globalVariable", "");
const textVariable = new Variable("textVariable", "");
const data = renderData(
<$.Body ws:id="bodyId" data-vars={expression`${bodyVariable}`}>
<$.Text ws:id="textId">{expression`${textVariable}`}</$.Text>
</$.Body>
);
rebindTreeVariablesMutable({
startingInstanceId: "bodyId",
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
expect(Array.from(data.dataSources.values())).toEqual([
expect.objectContaining({ scopeInstanceId: "bodyId" }),
expect.objectContaining({ scopeInstanceId: "textId" }),
]);
const [_globalVariableId, textVariableId] = data.dataSources.keys();
const textIdentifier = encodeDataVariableId(textVariableId);
expect(data.instances.get("textId")?.children).toEqual([
{ type: "expression", value: textIdentifier },
]);
});

test("prevent rebinding tree variables from slots", () => {
const bodyVariable = new Variable("myVariable", "one value of body");
const data = renderData(
Expand All @@ -469,7 +564,11 @@ test("prevent rebinding tree variables from slots", () => {
</$.Slot>
</$.Body>
);
rebindTreeVariablesMutable({ startingInstanceId: "boxId", ...data });
rebindTreeVariablesMutable({
startingInstanceId: "boxId",
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
...data,
});
expect(data.instances.get("boxId")?.children).toEqual([
{ type: "expression", value: "myVariable" },
]);
Expand Down
80 changes: 47 additions & 33 deletions apps/builder/app/shared/data-variables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,11 @@ const traverseExpressions = ({
props: Props;
dataSources: DataSources;
resources: Resources;
update: (expression: string, args?: string[]) => void | string;
update: (
expression: string,
instanceId: Instance["id"],
args?: string[]
) => void | string;
}) => {
const pagesList = pages ? [pages.homePage, ...pages.pages] : [];

Expand All @@ -297,45 +301,51 @@ const traverseExpressions = ({
startingInstanceId === page.rootInstanceId ||
startingInstanceId === ROOT_INSTANCE_ID
) {
page.title = update(page.title) ?? page.title;
const { rootInstanceId } = page;
page.title = update(page.title, rootInstanceId) ?? page.title;
if (page.meta.description) {
page.meta.description =
update(page.meta.description) ?? page.meta.description;
update(page.meta.description, rootInstanceId) ??
page.meta.description;
}
if (page.meta.excludePageFromSearch) {
page.meta.excludePageFromSearch =
update(page.meta.excludePageFromSearch) ??
update(page.meta.excludePageFromSearch, rootInstanceId) ??
page.meta.excludePageFromSearch;
}
if (page.meta.socialImageUrl) {
page.meta.socialImageUrl =
update(page.meta.socialImageUrl) ?? page.meta.socialImageUrl;
update(page.meta.socialImageUrl, rootInstanceId) ??
page.meta.socialImageUrl;
}
if (page.meta.language) {
page.meta.language = update(page.meta.language) ?? page.meta.language;
page.meta.language =
update(page.meta.language, rootInstanceId) ?? page.meta.language;
}
if (page.meta.status) {
page.meta.status = update(page.meta.status) ?? page.meta.status;
page.meta.status =
update(page.meta.status, rootInstanceId) ?? page.meta.status;
}
if (page.meta.redirect) {
page.meta.redirect = update(page.meta.redirect) ?? page.meta.redirect;
page.meta.redirect =
update(page.meta.redirect, rootInstanceId) ?? page.meta.redirect;
}
if (page.meta.custom) {
for (const item of page.meta.custom) {
item.content = update(item.content) ?? item.content;
item.content = update(item.content, rootInstanceId) ?? item.content;
}
}
}
}
const resourceIds = new Set<Resource["id"]>();
const instanceIdByResourceId = new Map<Resource["id"], Instance["id"]>();

for (const instance of instances.values()) {
if (instanceIds.has(instance.id) === false) {
continue;
}
for (const child of instance.children) {
if (child.type === "expression") {
child.value = update(child.value) ?? child.value;
child.value = update(child.value, instance.id) ?? child.value;
}
}
}
Expand All @@ -345,40 +355,40 @@ const traverseExpressions = ({
continue;
}
if (prop.type === "expression") {
prop.value = update(prop.value) ?? prop.value;
prop.value = update(prop.value, prop.instanceId) ?? prop.value;
continue;
}
if (prop.type === "action") {
for (const action of prop.value) {
action.code = update(action.code, action.args) ?? action.code;
action.code =
update(action.code, prop.instanceId, action.args) ?? action.code;
}
continue;
}
if (prop.type === "resource") {
resourceIds.add(prop.value);
instanceIdByResourceId.set(prop.value, prop.instanceId);
continue;
}
}

for (const dataSource of dataSources.values()) {
if (
instanceIds.has(dataSource.scopeInstanceId ?? "") &&
dataSource.type === "resource"
) {
resourceIds.add(dataSource.resourceId);
const instanceId = dataSource.scopeInstanceId ?? "";
if (instanceIds.has(instanceId) && dataSource.type === "resource") {
instanceIdByResourceId.set(dataSource.resourceId, instanceId);
}
}

for (const resource of resources.values()) {
if (resourceIds.has(resource.id) === false) {
const instanceId = instanceIdByResourceId.get(resource.id);
if (instanceId === undefined) {
continue;
}
resource.url = update(resource.url) ?? resource.url;
resource.url = update(resource.url, instanceId) ?? resource.url;
for (const header of resource.headers) {
header.value = update(header.value) ?? header.value;
header.value = update(header.value, instanceId) ?? header.value;
}
if (resource.body) {
resource.body = update(resource.body) ?? resource.body;
resource.body = update(resource.body, instanceId) ?? resource.body;
}
}
};
Expand All @@ -404,7 +414,7 @@ export const findUnsetVariableNames = ({
props,
dataSources,
resources,
update: (expression, args = []) => {
update: (expression, _instanceId, args = []) => {
transpileExpression({
expression,
replaceVariable: (identifier) => {
Expand Down Expand Up @@ -458,34 +468,38 @@ export const findUsedVariables = ({

export const rebindTreeVariablesMutable = ({
startingInstanceId,
pages,
instances,
props,
dataSources,
resources,
}: {
startingInstanceId: Instance["id"];
pages: undefined | Pages;
instances: Instances;
props: Props;
dataSources: DataSources;
resources: Resources;
}) => {
const maskedVariables = findMaskedVariablesByInstanceId({
startingInstanceId,
dataSources,
instances,
});
// unset all variables
const unsetNameById = new Map<DataSource["id"], DataSource["name"]>();
for (const { id, name } of dataSources.values()) {
unsetNameById.set(id, name);
for (const dataSource of dataSources.values()) {
unsetNameById.set(dataSource.id, dataSource.name);
}
traverseExpressions({
startingInstanceId,
pages: undefined,
pages,
instances,
props,
dataSources,
resources,
update: (expression, args) => {
update: (expression, instanceId, args) => {
// restore all masked variables of current scope
const maskedVariables = findMaskedVariablesByInstanceId({
startingInstanceId: instanceId,
dataSources,
instances,
});
let maskedIdByName = new Map(maskedVariables);
if (args) {
maskedIdByName = new Map(maskedIdByName);
Expand Down