Skip to content

Commit 191d0ac

Browse files
surajshetty3416surajshetty3416
authored
Merge pull request #539 from frappe/mergify/bp/master/pr-525
fix(Block Props): improved implementation of `getPropValue` (backport #525)
2 parents ae8cad6 + eb4dd08 commit 191d0ac

5 files changed

Lines changed: 93 additions & 120 deletions

File tree

‎frontend/src/components/BlockHTML.vue‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,7 @@ const getDynamicContent = () => {
3333
let value;
3434
if (props.block.getDataKey("comesFrom") === "props") {
3535
// props are checked first as unavailablity of comesFrom means it comes from dataScript (legacy)
36-
value = getPropValue(
37-
props.block.getDataKey("key"),
38-
props.block,
39-
getDataScriptValue,
40-
getBlockDataScriptValue,
41-
props.defaultProps,
42-
);
36+
value = getPropValue(props.block.getDataKey("key"), props.block);
4337
} else if (props.block.getDataKey("comesFrom") === "blockDataScript") {
4438
value = getBlockDataScriptValue(props.block.getDataKey("key"));
4539
} else {
@@ -55,13 +49,7 @@ const getDynamicContent = () => {
5549
?.forEach((dataKeyObj: BlockDataKey) => {
5650
let value;
5751
if (dataKeyObj.comesFrom === "props") {
58-
value = getPropValue(
59-
dataKeyObj.key as string,
60-
props.block,
61-
getDataScriptValue,
62-
getBlockDataScriptValue,
63-
props.defaultProps,
64-
);
52+
value = getPropValue(dataKeyObj.key as string, props.block, props.uid);
6553
} else if (dataKeyObj.comesFrom === "blockDataScript") {
6654
value = getBlockDataScriptValue(dataKeyObj.key as string);
6755
} else {

‎frontend/src/components/BuilderBlock.vue‎

Lines changed: 40 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ const canvasStore = useCanvasStore();
7979
const component = ref<HTMLElement | InstanceType<typeof TextBlock> | null>(null);
8080
const attrs = useAttrs();
8181
const editor = ref<InstanceType<typeof BlockEditor> | null>(null);
82+
const isMounted = ref(false);
83+
8284
const pageStore = usePageStore();
8385
const blockDataStore = useBlockDataStore();
8486
const blockUidStore = useBlockUidStore();
@@ -179,7 +181,7 @@ const attributes = computed(() => {
179181
}
180182
181183
Object.keys(additionalAttributes).forEach((key) => {
182-
if (!RESTRICTED_ATTRIBS.includes(key)) {
184+
if (RESTRICTED_ATTRIBS.includes(key)) {
183185
delete additionalAttributes[key];
184186
}
185187
});
@@ -219,13 +221,7 @@ const attributes = computed(() => {
219221
if (props.block.getDataKey("type") === "attribute") {
220222
let value;
221223
if (props.block.getDataKey("comesFrom") === "props") {
222-
value = getPropValue(
223-
props.block.getDataKey("key") as string,
224-
props.block,
225-
getDataScriptValue,
226-
getBlockDataScriptValue,
227-
props.defaultProps,
228-
);
224+
value = getPropValue(props.block.getDataKey("key") as string, props.block, uidToUse);
229225
} else if (props.block.getDataKey("comesFrom") === "blockDataScript") {
230226
value = getBlockDataScriptValue(props.block.getDataKey("key") as string);
231227
} else {
@@ -243,13 +239,7 @@ const attributes = computed(() => {
243239
const property = dataKeyObj.property as string;
244240
let value;
245241
if (dataKeyObj.comesFrom === "props") {
246-
value = getPropValue(
247-
dataKeyObj.key as string,
248-
props.block,
249-
getDataScriptValue,
250-
getBlockDataScriptValue,
251-
props.defaultProps,
252-
);
242+
value = getPropValue(dataKeyObj.key as string, props.block, uidToUse);
253243
} else if (dataKeyObj.comesFrom === "blockDataScript") {
254244
value = getBlockDataScriptValue(dataKeyObj.key as string);
255245
} else {
@@ -283,13 +273,7 @@ const styles = computed(() => {
283273
if (props.block.getDataKey("type") === "style") {
284274
let value;
285275
if (props.block.getDataKey("comesFrom") === "props") {
286-
value = getPropValue(
287-
props.block.getDataKey("key") as string,
288-
props.block,
289-
getDataScriptValue,
290-
getBlockDataScriptValue,
291-
props.defaultProps,
292-
);
276+
value = getPropValue(props.block.getDataKey("key") as string, props.block, uidToUse);
293277
} else if (props.block.getDataKey("comesFrom") === "blockDataScript") {
294278
value = getBlockDataScriptValue(props.block.getDataKey("key") as string);
295279
} else {
@@ -308,13 +292,7 @@ const styles = computed(() => {
308292
const property = dataKeyObj.property as string;
309293
let value;
310294
if (dataKeyObj.comesFrom === "props") {
311-
value = getPropValue(
312-
dataKeyObj.key as string,
313-
props.block,
314-
getDataScriptValue,
315-
getBlockDataScriptValue,
316-
props.defaultProps,
317-
);
295+
value = getPropValue(dataKeyObj.key as string, props.block, uidToUse);
318296
} else if (dataKeyObj.comesFrom === "blockDataScript") {
319297
value = getBlockDataScriptValue(dataKeyObj.key as string);
320298
} else {
@@ -396,31 +374,34 @@ onMounted(async () => {
396374
}
397375
blockUidStore.registerBlockUid(uidToUse, props.block);
398376
blockUidStore.setParentUid(uidToUse, props.parentBlockUid || "root");
377+
isMounted.value = true;
399378
});
400379
401380
const allResolvedProps = computed(() => {
381+
if (!isMounted.value) {
382+
return {};
383+
}
384+
const defaultProps = Object.entries(props.defaultProps || {}).reduce((acc, [key, value]) => {
385+
acc[key] = value.value;
386+
return acc;
387+
}, {} as Record<string, any>);
388+
389+
const blockProps = Object.entries({
390+
...props.block.getBlockProps(),
391+
}).reduce((acc, [key]) => {
392+
acc[key] = getPropValue(key, props.block, uidToUse);
393+
return acc;
394+
}, {} as Record<string, any>);
395+
396+
const parentProps = Object.entries(getParentProps(props.block, uidToUse)).reduce((acc, [key, value]) => {
397+
acc[key] = getPropValue(key, value.block!, value.blockUid);
398+
return acc;
399+
}, {} as Record<string, any>);
400+
402401
return {
403-
...Object.fromEntries(
404-
Object.entries(props.defaultProps || {}).map(([key, value]) => {
405-
return [key, value.value];
406-
}),
407-
),
408-
...Object.fromEntries(
409-
Object.entries({ ...props.block.getBlockProps(), ...getParentProps(props.block) }).map(
410-
([key, prop]) => {
411-
return [
412-
key,
413-
getPropValue(
414-
key,
415-
props.block,
416-
getDataScriptValue,
417-
(path: string) => getDataForKey({ ...props.blockData }, path), // block props can not refer to own block data items
418-
props.defaultProps,
419-
),
420-
];
421-
},
422-
),
423-
),
402+
...parentProps,
403+
...blockProps,
404+
...defaultProps,
424405
};
425406
});
426407
@@ -497,12 +478,18 @@ watch(
497478
});
498479
});
499480
},
500-
{ immediate: true },
481+
{ immediate: true, deep: true },
501482
);
502483
503484
watchEffect(() => {
504-
blockDataStore.setPageData(uidToUse, props.data || {});
505485
blockDataStore.setBlockData(uidToUse, props.blockData || {}, "passedDown");
486+
});
487+
488+
watchEffect(() => {
489+
blockDataStore.setPageData(uidToUse, props.data || {});
490+
});
491+
492+
watchEffect(() => {
506493
blockDataStore.setBlockDefaultProps(uidToUse, props.defaultProps || {});
507494
});
508495
@@ -526,13 +513,7 @@ const hiddenDueToVisibilityCondition = computed(() => {
526513
const value = getDataScriptValue(key as string);
527514
return !Boolean(value);
528515
} else {
529-
const value = getPropValue(
530-
key as string,
531-
props.block,
532-
getDataScriptValue,
533-
getBlockDataScriptValue,
534-
props.defaultProps,
535-
);
516+
const value = getPropValue(key as string, props.block, uidToUse);
536517
return !Boolean(value);
537518
}
538519
});

‎frontend/src/components/Controls/DynamicValueHandler.vue‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ const filteredBlockProps = computed(() => {
148148
} else {
149149
const currentBlock = props.block || blockController.getFirstSelectedBlock();
150150
if (currentBlock) {
151-
parentProps = Object.keys(getParentProps(currentBlock, {}));
151+
parentProps = Object.keys(getParentProps(currentBlock));
152152
} else {
153153
parentProps = [];
154154
}
@@ -185,15 +185,7 @@ const filteredItems = computed(() => {
185185
(item) =>
186186
item.key.toLowerCase().includes(query) ||
187187
String(getDataScriptValue(item.key)).toLowerCase().includes(query) ||
188-
String(
189-
getPropValue(
190-
item.key,
191-
props.block || blockController.getFirstSelectedBlock(),
192-
getDataScriptValue,
193-
getBlockDataScriptValue,
194-
defaultProps.value,
195-
),
196-
)
188+
String(getPropValue(item.key, props.block || blockController.getFirstSelectedBlock()))
197189
.toLowerCase()
198190
.includes(query),
199191
);
@@ -203,13 +195,7 @@ const selectedItem = ref<DynamicValueItem | null>(props.selectedValue || null);
203195
204196
const getValue = (item: DynamicValueItem): any => {
205197
if (item.comesFrom == "props") {
206-
return getPropValue(
207-
item.key,
208-
props.block || blockController.getFirstSelectedBlock(),
209-
getDataScriptValue,
210-
getBlockDataScriptValue,
211-
defaultProps.value,
212-
);
198+
return getPropValue(item.key, props.block || blockController.getFirstSelectedBlock());
213199
} else if (item.comesFrom == "blockDataScript") {
214200
return getBlockDataScriptValue(item.key);
215201
} else {

‎frontend/src/components/TextBlock.vue‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,7 @@ const getDynamicContent = () => {
130130
let value;
131131
if (props.block.getDataKey("comesFrom") === "props") {
132132
// props are checked first as unavailablity of comesFrom means it comes from dataScript (legacy)
133-
value = getPropValue(
134-
props.block.getDataKey("key"),
135-
props.block,
136-
getDataScriptValue,
137-
getBlockDataScriptValue,
138-
props.defaultProps,
139-
);
133+
value = getPropValue(props.block.getDataKey("key"), props.block, props.uid);
140134
} else if (props.block.getDataKey("comesFrom") === "blockDataScript") {
141135
value = getBlockDataScriptValue(props.block.getDataKey("key"));
142136
} else {
@@ -152,13 +146,7 @@ const getDynamicContent = () => {
152146
?.forEach((dataKeyObj: BlockDataKey) => {
153147
let value;
154148
if (dataKeyObj.comesFrom === "props") {
155-
value = getPropValue(
156-
dataKeyObj.key as string,
157-
props.block,
158-
getDataScriptValue,
159-
getBlockDataScriptValue,
160-
props.defaultProps,
161-
);
149+
value = getPropValue(dataKeyObj.key as string, props.block, props.uid);
162150
} else if (dataKeyObj.comesFrom === "blockDataScript") {
163151
value = getBlockDataScriptValue(dataKeyObj.key as string);
164152
} else {

‎frontend/src/utils/helpers.ts‎

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,19 +1120,33 @@ const getValueForInheritedProp = (
11201120
return undefined;
11211121
};
11221122

1123-
const getParentProps = (baseBlock: Block, baseProps: BlockProps = {}): BlockProps => {
1123+
type BlockPropsWithTraceback = Record<
1124+
string,
1125+
BlockProps[string] & { block?: Block; blockUid?: string | null }
1126+
>;
1127+
1128+
const getParentProps = (
1129+
baseBlock: Block,
1130+
blockUid?: string | null,
1131+
baseProps: BlockPropsWithTraceback = {},
1132+
): BlockPropsWithTraceback => {
11241133
const parentBlock = baseBlock.getParentBlock();
1134+
const parentBlockUid = useBlockUidStore().getParentUid(blockUid || "");
11251135
if (parentBlock) {
1126-
const parentProps: BlockProps = {};
1136+
const parentProps: BlockPropsWithTraceback = {};
11271137
Object.entries(parentBlock.getBlockProps())
11281138
.filter(([_, propDetails]) => {
11291139
return propDetails.isPassedDown;
11301140
})
1131-
.map(([propName, propDetails]) => {
1132-
parentProps[propName] = propDetails;
1141+
.forEach(([propName, propDetails]) => {
1142+
parentProps[propName] = {
1143+
...propDetails,
1144+
block: parentBlock,
1145+
blockUid: parentBlockUid,
1146+
};
11331147
});
11341148
const combinedProps = { ...parentProps, ...baseProps };
1135-
return getParentProps(parentBlock, combinedProps);
1149+
return getParentProps(parentBlock, parentBlockUid, combinedProps);
11361150
} else {
11371151
return baseProps;
11381152
}
@@ -1184,28 +1198,46 @@ const getDefaultPropsList = (block: Block, blockController: any): BlockProps =>
11841198

11851199
const PARSEABLE_STANDARD_TYPES = ["number", "boolean", "object", "array"];
11861200

1187-
const getPropValue = (
1188-
propName: string,
1189-
block: Block,
1190-
getDataScriptValue: (path: string) => any,
1191-
getBlockScriptValue: (path: string) => any,
1192-
defaultProps?: Record<string, any> | null,
1193-
): any => {
1201+
const getPropValue = (propName: string, block: Block, blockUid?: string | null): any => {
1202+
const blockDataStore = useBlockDataStore();
1203+
1204+
const uidToUse = blockUid || block.blockId;
1205+
1206+
const defaultProps = blockDataStore.getDefaultProps(uidToUse);
11941207
// Check default props first
11951208
if (defaultProps?.[propName] !== undefined) {
11961209
return defaultProps[propName].value;
11971210
}
11981211

1212+
let parentProps: BlockPropsWithTraceback | null = null;
1213+
11991214
// Find matching prop from block or parent
12001215
const blockProps = block.getBlockProps();
1201-
const matchingProp = blockProps[propName] ?? getParentProps(block, {})[propName];
1216+
let matchingProp: BlockPropsWithTraceback[string] =
1217+
blockProps[propName] ?? (parentProps = getParentProps(block, blockUid))[propName];
12021218

12031219
if (!matchingProp) {
12041220
return undefined;
12051221
}
12061222

12071223
// Handle dynamic props
12081224
if (matchingProp.isDynamic) {
1225+
if (matchingProp.comesFrom === "props" && matchingProp.value) {
1226+
if (parentProps === null) {
1227+
parentProps = getParentProps(block, blockUid);
1228+
}
1229+
const newMatchingProp = parentProps[matchingProp.value];
1230+
if (!newMatchingProp.block) return undefined;
1231+
return getPropValue(matchingProp.value, newMatchingProp.block, newMatchingProp.blockUid);
1232+
}
1233+
const getDataScriptValue = (path: string) => {
1234+
const pageData = blockDataStore.getPageData(uidToUse) || {};
1235+
return getDataForKey(pageData, path);
1236+
};
1237+
const getBlockScriptValue = (path: string) => {
1238+
const blockData = blockDataStore.getBlockData(uidToUse, "passedDown") || {};
1239+
return getDataForKey(blockData, path);
1240+
};
12091241
if (matchingProp.comesFrom === "dataScript" && matchingProp.value) {
12101242
return getDataScriptValue(matchingProp.value);
12111243
}
@@ -1230,7 +1262,6 @@ const getPropValue = (
12301262
if (PARSEABLE_STANDARD_TYPES.includes(type)) {
12311263
return matchingProp.value ? JSON.parse(matchingProp.value) : defaultValue;
12321264
}
1233-
12341265
return matchingProp.value || defaultValue;
12351266
}
12361267

@@ -1309,9 +1340,8 @@ function executeBlockClientScriptUnrestricted(
13091340
try {
13101341
document.querySelectorAll(`[data-created-by='${blockUid}']`).forEach((el) => el.remove());
13111342
fn.call(thisElement, context);
1312-
console.log("Executed unrestricted user script");
13131343
} catch (e) {
1314-
console.error("Error in user script 2:", e);
1344+
console.error("Error in user script (unrestricted):", e);
13151345
// toast.warning("An error occurred while executing block script: " + (e instanceof Error ? e.message : ""));
13161346
}
13171347
}

0 commit comments

Comments
 (0)