fix: optimize block variables and lexical state updates (#3689)

webiny · Nov 10, 2023 · 1f3e8a1 · 1f3e8a1
1 parent a78f96a
commit 1f3e8a1
Show file tree

Hide file tree

Showing 32 changed files with 191 additions and 132 deletions.
diff --git a/packages/app-page-builder/src/blockEditor/components/elementSettingsTab/VariableSettings.tsx b/packages/app-page-builder/src/blockEditor/components/elementSettingsTab/VariableSettings.tsx
@@ -25,7 +25,7 @@ const VariableSettings = ({ element }: { element: PbEditorElement }) => {
             (variable: PbBlockVariable) => variable.id.split(".")[0] === element?.data?.variableId
         );
 
-        return variables;
+        return variables ?? [];
     }, [block, element]);
 
     const onChange = useCallback(
@@ -67,7 +67,8 @@ const VariableSettings = ({ element }: { element: PbEditorElement }) => {
         () =>
             showConfirmation(() => {
                 if (block && block.id) {
-                    const updatedVariables = block.data.variables.filter(
+                    const variables = block.data.variables ?? [];
+                    const updatedVariables = variables.filter(
                         (variable: PbBlockVariable) =>
                             variable.id.split(".")[0] !== element?.data?.variableId
                     );
@@ -99,7 +100,7 @@ const VariableSettings = ({ element }: { element: PbEditorElement }) => {
     return (
         <>
             <FormWrapper>
-                {elementVariables?.map((variable: PbBlockVariable, index: string) => (
+                {elementVariables.map((variable, index) => (
                     <TextInput
                         key={index}
                         label={`${capitalize(variable.type)} ${capitalize(

diff --git a/packages/app-page-builder/src/blockEditor/components/elementSettingsTab/VariablesList.tsx b/packages/app-page-builder/src/blockEditor/components/elementSettingsTab/VariablesList.tsx
@@ -143,7 +143,8 @@ const VariablesList = ({ block }: { block: PbEditorElement }) => {
             showConfirmation(async () => {
                 if (block && block.id) {
                     // remove variable from block
-                    const updatedVariables = block.data.variables.filter(
+                    const variables = block.data.variables ?? [];
+                    const updatedVariables = variables.filter(
                         (variable: PbBlockVariable) => variable.id !== variableId
                     );
                     updateElement({

diff --git a/...ages/app-page-builder/src/blockEditor/components/elementSettingsTab/variablesListHooks.ts b/...ages/app-page-builder/src/blockEditor/components/elementSettingsTab/variablesListHooks.ts
@@ -9,7 +9,7 @@ interface UseMoveVariable {
 export const useMoveVariable = (element: PbEditorElement): UseMoveVariable => {
     const updateElement = useUpdateElement();
     const move = (current: number, next: number) => {
-        const reorderedVariables = moveInPlace(element?.data?.variables, current, next);
+        const reorderedVariables = moveInPlace(element?.data?.variables ?? [], current, next);
 
         updateElement({
             ...element,

diff --git a/packages/app-page-builder/src/editor/hooks/useElementVariableValue.ts b/packages/app-page-builder/src/editor/hooks/useElementVariableValue.ts
@@ -22,7 +22,7 @@ export function useElementVariables(element: PbEditorElement | null) {
         }
     }, [block, element]);
 
-    return variableValue;
+    return variableValue ?? [];
 }
 
 export function useElementVariableValue(element: PbEditorElement | null) {

diff --git a/packages/app-page-builder/src/editor/plugins/elementSettings/delete/DeleteAction.ts b/packages/app-page-builder/src/editor/plugins/elementSettings/delete/DeleteAction.ts
@@ -9,7 +9,9 @@ import { useUpdateElement } from "~/editor/hooks/useUpdateElement";
 import { useParentBlock } from "~/editor/hooks/useParentBlock";
 
 const removeVariableFromBlock = (block: PbEditorElement, variableId: string) => {
-    const updatedVariables = block.data.variables.filter(
+    const variables = block.data.variables ?? [];
+
+    const updatedVariables = variables.filter(
         (variable: PbBlockVariable) => variable.id.split(".")[0] !== variableId
     );
 

diff --git a/packages/app-page-builder/src/editor/plugins/elementSettings/variable/VariableSettings.tsx b/packages/app-page-builder/src/editor/plugins/elementSettings/variable/VariableSettings.tsx
@@ -1,4 +1,4 @@
-import React from "react";
+import React, { useCallback, useMemo } from "react";
 import { css } from "emotion";
 import { plugins } from "@webiny/plugins";
 import { useActiveElement } from "~/editor/hooks/useActiveElement";
@@ -21,21 +21,41 @@ const labelStyle = css({
 const VariableSettings: React.FC = () => {
     const [element] = useActiveElement();
 
-    const elementVariableRendererPlugins =
-        plugins.byType<PbEditorPageElementVariableRendererPlugin>(
+    const variableRenderers = useMemo(() => {
+        return plugins.byType<PbEditorPageElementVariableRendererPlugin>(
             "pb-editor-page-element-variable-renderer"
         );
+    }, []);
+
+    const renderVariableInput = useCallback(
+        (variable: PbBlockVariable) => {
+            const renderer = variableRenderers.find(plugin => plugin.elementType === variable.type);
+            if (!renderer) {
+                return null;
+            }
+            return renderer.renderVariableInput(variable.id);
+        },
+        [variableRenderers]
+    );
+
+    if (!element) {
+        return null;
+    }
+
+    const variables = element.data.variables;
+
+    if (!variables) {
+        return null;
+    }
 
     return (
         <div className={wrapperStyle}>
-            {element?.data?.variables?.map((variable: PbBlockVariable, index: number) => (
+            {variables.map((variable, index) => (
                 <div key={index}>
                     <div className={labelStyle}>
                         <Typography use={"body2"}>{variable.label}</Typography>
                     </div>
-                    {elementVariableRendererPlugins
-                        .find(plugin => plugin.elementType === variable?.type)
-                        ?.renderVariableInput(variable.id)}
+                    {renderVariableInput(variable)}
                 </div>
             ))}
         </div>

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/basic/heading/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/basic/heading/index.tsx
@@ -10,7 +10,7 @@ export default {
     elementType: "heading",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return (

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/basic/icon/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/basic/icon/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "icon",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return <IconVariableInput variableId={variableId} />;

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/basic/image/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/basic/image/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "image",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return <SingleImageVariableInput variableId={variableId} />;

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/basic/images-list/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/basic/images-list/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "images-list",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return <MultipleImageVariableInput variableId={variableId} />;

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/basic/list/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/basic/list/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "list",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return <RichVariableInput variableId={variableId} />;

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/basic/paragraph/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/basic/paragraph/index.tsx
@@ -10,7 +10,11 @@ export default {
     elementType: "paragraph",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        if (!variables) {
+            return null;
+        }
+
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return (

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/basic/quote/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/basic/quote/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "quote",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return <RichVariableInput variableId={variableId} />;

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/code/codesandbox/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/code/codesandbox/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "codesandbox",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return (

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/embeds/iframe/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/embeds/iframe/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "iframe",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return (

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/embeds/soundcloud/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/embeds/soundcloud/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "soundcloud",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return (

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/embeds/vimeo/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/embeds/vimeo/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "vimeo",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return (

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/embeds/youtube/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/embeds/youtube/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "youtube",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return (

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/social/pinterest/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/social/pinterest/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "pinterest",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return (

diff --git a/packages/app-page-builder/src/editor/plugins/elementVariables/social/twitter/index.tsx b/packages/app-page-builder/src/editor/plugins/elementVariables/social/twitter/index.tsx
@@ -9,7 +9,7 @@ export default {
     elementType: "twitter",
     getVariableValue(element) {
         const variables = useElementVariables(element);
-        return variables?.length > 0 ? variables[0].value : null;
+        return variables.length > 0 ? variables[0].value : null;
     },
     renderVariableInput(variableId: string) {
         return (

diff --git a/packages/app-page-builder/src/hooks/useVariable.ts b/packages/app-page-builder/src/hooks/useVariable.ts
@@ -1,44 +1,53 @@
-import { useCallback } from "react";
+import { useCallback, useEffect, useRef } from "react";
 import { useUpdateElement } from "~/editor/hooks/useUpdateElement";
 import { useActiveElement } from "~/editor/hooks/useActiveElement";
 import { PbBlockVariable } from "~/types";
 
 export function useVariable<TValue = any>(variableId: string) {
     const [element] = useActiveElement();
+    const elementRef = useRef(element);
     const updateElement = useUpdateElement();
     const variable = element?.data?.variables?.find(
         (variable: PbBlockVariable) => variable.id === variableId
     );
 
+    // We're storing a reference to an element, so we don't have to recreate the `onChange` callback on every update.
+    useEffect(() => {
+        elementRef.current = element;
+    }, [element]);
+
     const onChange = useCallback(
         (value: TValue, history = false) => {
-            if (element) {
-                const newVariables = element?.data?.variables?.map((variable: PbBlockVariable) => {
-                    if (variable?.id === variableId) {
-                        return {
-                            ...variable,
-                            value
-                        };
-                    }
+            const element = elementRef.current;
+            if (!element) {
+                return;
+            }
 
-                    return variable;
-                });
-
-                updateElement(
-                    {
-                        ...element,
-                        data: {
-                            ...element.data,
-                            variables: newVariables
-                        }
-                    },
-                    {
-                        history
+            const newVariables = element.data.variables?.map(variable => {
+                if (variable?.id === variableId) {
+                    return {
+                        ...variable,
+                        value
+                    };
+                }
+
+                return variable;
+            });
+
+            updateElement(
+                {
+                    ...element,
+                    data: {
+                        ...element.data,
+                        variables: newVariables
                     }
-                );
-            }
+                },
+                {
+                    history
+                }
+            );
         },
-        [element, variableId, updateElement]
+        [variableId, updateElement]
     );
 
     const onBlur = useCallback(() => {

diff --git a/...es/app-page-builder/src/pageEditor/config/eventActions/saveRevision/saveRevisionAction.ts b/...es/app-page-builder/src/pageEditor/config/eventActions/saveRevision/saveRevisionAction.ts
@@ -32,10 +32,11 @@ const syncTemplateVariables = (content: PbElement) => {
     const templateVariables = [];
 
     for (const block of content.elements) {
-        if (block.data.variables?.length > 0) {
+        const variables = block.data.variables ?? [];
+        if (variables.length > 0) {
             templateVariables.push({
                 blockId: block.data.templateBlockId,
-                variables: block.data.variables
+                variables: variables
             });
         }
     }
@@ -50,7 +51,7 @@ const removeTemplateBlockIds = (content: PbElement) => {
         // we need to drop templateBlockId property, when block is no longer inside template
         // we also remove variables from unlinked blocks, since they don't need them outside template
         // eslint-disable-next-line @typescript-eslint/no-unused-vars
-        const { templateBlockId, variables, ...blockData } = block.data;
+        const { templateBlockId, variables = [], ...blockData } = block.data;
         if (block.data.blockId) {
             return {
                 ...block,

diff --git a/packages/app-page-builder/src/pageEditor/plugins/elementSettings/UnlinkBlockAction.ts b/packages/app-page-builder/src/pageEditor/plugins/elementSettings/UnlinkBlockAction.ts
@@ -18,7 +18,7 @@ const UnlinkBlockAction: React.FC<UnlinkBlockActionPropsType> = ({ children }) =
         if (element) {
             // we need to drop blockId and variables properties when unlinking, so they are separated from all other element data
             // eslint-disable-next-line @typescript-eslint/no-unused-vars
-            const { blockId, variables, ...newData } = element.data;
+            const { blockId, variables = [], ...newData } = element.data;
             const pbElement = (await getElementTree({
                 element: { ...element, data: newData }
             })) as PbElement;

diff --git a/packages/app-page-builder/src/templateEditor/plugins/elementSettings/UnlinkBlockAction.ts b/packages/app-page-builder/src/templateEditor/plugins/elementSettings/UnlinkBlockAction.ts
@@ -18,7 +18,7 @@ const UnlinkBlockAction: React.FC<UnlinkBlockActionPropsType> = ({ children }) =
         if (element) {
             // we need to drop blockId and variables properties when unlinking, so they are separated from all other element data
             // eslint-disable-next-line @typescript-eslint/no-unused-vars
-            const { blockId, variables, ...newData } = element.data;
+            const { blockId, variables = [], ...newData } = element.data;
             const pbElement = (await getElementTree({
                 element: { ...element, data: newData }
             })) as PbElement;

diff --git a/packages/app-page-builder/src/types.ts b/packages/app-page-builder/src/types.ts
@@ -177,6 +177,7 @@ export interface PbElementDataTypeSource {
 
 export type PbElementDataType = {
     blockId?: string;
+    variables?: PbBlockVariable[];
     action?: {
         href: string;
         newTab: boolean;