Skip to content

Commit c1ec5be

Browse files
authored
fix: rebind nested collection item properly (#5159)
Before parent collection item on nested collection was rebind with nested collection item which lead to broken state ``` <Collection id="parent" data={[]} item="collectionItem"> <Collection id="nested" data={collectionItem} item="collectionItem"> </Collection </Collection ``` ``` collection.map(collectionItem => // oops collectionItem2.map(collectionItem2 => { }) }) ```
1 parent 3826ddd commit c1ec5be

2 files changed

Lines changed: 72 additions & 0 deletions

File tree

apps/builder/app/shared/data-variables.test.tsx

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
$,
44
ActionValue,
55
expression,
6+
Parameter,
67
renderData,
78
ResourceValue,
89
Variable,
@@ -574,6 +575,60 @@ test("prevent rebinding tree variables from slots", () => {
574575
]);
575576
});
576577

578+
test("prevent rebinding with nested collection item", () => {
579+
const collectionItem = new Parameter("collectionITem");
580+
const nestedCollectionItem = new Parameter("collectionITem");
581+
const data = renderData(
582+
<$.Body ws:id="bodyId">
583+
<ws.collection ws:id="collectionId" data={[]} item={collectionItem}>
584+
<ws.collection
585+
ws:id="nestedCollectionId"
586+
data={expression`${collectionItem}`}
587+
item={nestedCollectionItem}
588+
>
589+
<ws.element ws:id="divId" ws:tag="div">
590+
{expression`${nestedCollectionItem}`}
591+
</ws.element>
592+
</ws.collection>
593+
</ws.collection>
594+
</$.Body>
595+
);
596+
expect(Array.from(data.dataSources.values())).toEqual([
597+
expect.objectContaining({ scopeInstanceId: "collectionId" }),
598+
expect.objectContaining({ scopeInstanceId: "nestedCollectionId" }),
599+
]);
600+
const [collectionItemId, nestedCollectionItemId] = data.dataSources.keys();
601+
rebindTreeVariablesMutable({
602+
startingInstanceId: "bodyId",
603+
pages: createDefaultPages({ rootInstanceId: "bodyId" }),
604+
...data,
605+
});
606+
expect(Array.from(data.props.values())).toEqual([
607+
expect.objectContaining({
608+
instanceId: "collectionId",
609+
name: "data",
610+
}),
611+
expect.objectContaining({
612+
instanceId: "collectionId",
613+
name: "item",
614+
value: collectionItemId,
615+
}),
616+
expect.objectContaining({
617+
instanceId: "nestedCollectionId",
618+
name: "data",
619+
value: encodeDataVariableId(collectionItemId),
620+
}),
621+
expect.objectContaining({
622+
instanceId: "nestedCollectionId",
623+
name: "item",
624+
value: nestedCollectionItemId,
625+
}),
626+
]);
627+
expect(data.instances.get("divId")?.children).toEqual([
628+
{ type: "expression", value: encodeDataVariableId(nestedCollectionItemId) },
629+
]);
630+
});
631+
577632
test("delete variable and unset it in expressions", () => {
578633
const bodyVariable = new Variable("bodyVariable", "one value of body");
579634
const data = renderData(

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
Pages,
1111
ROOT_INSTANCE_ID,
1212
SYSTEM_VARIABLE_ID,
13+
collectionComponent,
1314
decodeDataVariableId,
1415
encodeDataVariableId,
1516
findTreeInstanceIds,
@@ -208,10 +209,12 @@ const getParentInstanceById = (instances: Instances) => {
208209
const findMaskedVariablesByInstanceId = ({
209210
startingInstanceId,
210211
parentInstanceById,
212+
instances,
211213
dataSources,
212214
}: {
213215
startingInstanceId: Instance["id"];
214216
parentInstanceById: Map<Instance["id"], Instance["id"]>;
217+
instances: Instances;
215218
dataSources: DataSources;
216219
}) => {
217220
let currentId: undefined | string = startingInstanceId;
@@ -228,8 +231,19 @@ const findMaskedVariablesByInstanceId = ({
228231
// start from the root to descendant
229232
// so child variables override parent variables
230233
for (const instanceId of instanceIdsPath.reverse()) {
234+
const instance = instances.get(instanceId);
231235
for (const dataSource of dataSources.values()) {
232236
if (dataSource.scopeInstanceId === instanceId) {
237+
// when current instance is collection
238+
// ignore its collection item parameter
239+
// when rebind variables
240+
if (
241+
instanceId === startingInstanceId &&
242+
instance?.component === collectionComponent &&
243+
dataSource.type === "parameter"
244+
) {
245+
continue;
246+
}
233247
maskedVariables.set(dataSource.name, dataSource.id);
234248
}
235249
}
@@ -249,6 +263,7 @@ export const findAvailableVariables = ({
249263
const maskedVariables = findMaskedVariablesByInstanceId({
250264
startingInstanceId,
251265
parentInstanceById: getParentInstanceById(instances),
266+
instances,
252267
dataSources,
253268
});
254269
const availableVariables: DataSource[] = [];
@@ -504,6 +519,7 @@ export const rebindTreeVariablesMutable = ({
504519
const maskedVariables = findMaskedVariablesByInstanceId({
505520
startingInstanceId: instanceId,
506521
parentInstanceById,
522+
instances,
507523
dataSources,
508524
});
509525
let maskedIdByName = new Map(maskedVariables);
@@ -544,6 +560,7 @@ export const deleteVariableMutable = (
544560
const maskedIdByName = findMaskedVariablesByInstanceId({
545561
startingInstanceId,
546562
parentInstanceById: getParentInstanceById(data.instances),
563+
instances: data.instances,
547564
dataSources: data.dataSources,
548565
});
549566
// unset deleted variable in expressions

0 commit comments

Comments
 (0)