[Form lib] Memoize form hook object and fix hook array deps (#71237)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com>
elastic · Jul 15, 2020 · 99255d8 · 99255d8
1 parent 1ac56d7
commit 99255d8
Show file tree

Hide file tree

Showing 26 changed files with 783 additions and 687 deletions.
diff --git a/src/plugins/es_ui_shared/public/forms/multi_content/multi_content_context.tsx b/src/plugins/es_ui_shared/public/forms/multi_content/multi_content_context.tsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import React, { useEffect, useCallback, createContext, useContext } from 'react';
+import React, { useEffect, useCallback, createContext, useContext, useRef } from 'react';
 
 import { useMultiContent, HookProps, Content, MultiContent } from './use_multi_content';
 
@@ -55,7 +55,14 @@ export function useMultiContentContext<T extends object = { [key: string]: any }
  * @param contentId The content id to be added to the "contents" map
  */
 export function useContent<T extends object, K extends keyof T>(contentId: K) {
-  const { updateContentAt, saveSnapshotAndRemoveContent, getData } = useMultiContentContext<T>();
+  const isMounted = useRef(false);
+  const defaultValue = useRef<T[K] | undefined>(undefined);
+  const {
+    updateContentAt,
+    saveSnapshotAndRemoveContent,
+    getData,
+    getSingleContentData,
+  } = useMultiContentContext<T>();
 
   const updateContent = useCallback(
     (content: Content) => {
@@ -71,12 +78,22 @@ export function useContent<T extends object, K extends keyof T>(contentId: K) {
     };
   }, [contentId, saveSnapshotAndRemoveContent]);
 
-  const data = getData();
-  const defaultValue = data[contentId];
+  useEffect(() => {
+    if (isMounted.current === false) {
+      isMounted.current = true;
+    }
+  }, []);
+
+  if (isMounted.current === false) {
+    // Only read the default value once, on component mount to avoid re-rendering the
+    // consumer each time the multi-content validity ("isValid") changes.
+    defaultValue.current = getSingleContentData(contentId);
+  }
 
   return {
-    defaultValue,
+    defaultValue: defaultValue.current!,
     updateContent,
     getData,
+    getSingleContentData,
   };
 }
diff --git a/src/plugins/es_ui_shared/public/forms/multi_content/use_multi_content.ts b/src/plugins/es_ui_shared/public/forms/multi_content/use_multi_content.ts
@@ -45,6 +45,7 @@ export interface MultiContent<T extends object> {
   updateContentAt: (id: keyof T, content: Content) => void;
   saveSnapshotAndRemoveContent: (id: keyof T) => void;
   getData: () => T;
+  getSingleContentData: <K extends keyof T>(contentId: K) => T[K];
   validate: () => Promise<boolean>;
   validation: Validation<T>;
 }
@@ -109,9 +110,22 @@ export function useMultiContent<T extends object>({
     };
   }, [stateData, validation]);
 
+  /**
+   * Read a single content data.
+   */
+  const getSingleContentData = useCallback(
+    <K extends keyof T>(contentId: K): T[K] => {
+      if (contents.current[contentId]) {
+        return contents.current[contentId].getData();
+      }
+      return stateData[contentId];
+    },
+    [stateData]
+  );
+
   const updateContentValidity = useCallback(
     (updatedData: { [key in keyof T]?: boolean | undefined }): boolean | undefined => {
-      let allContentValidity: boolean | undefined;
+      let isAllContentValid: boolean | undefined = validation.isValid;
 
       setValidation((prev) => {
         if (
@@ -120,7 +134,7 @@ export function useMultiContent<T extends object>({
           )
         ) {
           // No change in validation, nothing to update
-          allContentValidity = prev.isValid;
+          isAllContentValid = prev.isValid;
           return prev;
         }
 
@@ -129,21 +143,21 @@ export function useMultiContent<T extends object>({
           ...updatedData,
         };
 
-        allContentValidity = Object.values(nextContentsValidityState).some(
+        isAllContentValid = Object.values(nextContentsValidityState).some(
           (_isValid) => _isValid === undefined
         )
           ? undefined
           : Object.values(nextContentsValidityState).every(Boolean);
 
         return {
-          isValid: allContentValidity,
+          isValid: isAllContentValid,
           contents: nextContentsValidityState,
         };
       });
 
-      return allContentValidity;
+      return isAllContentValid;
     },
-    []
+    [validation.isValid]
   );
 
   /**
@@ -163,7 +177,7 @@ export function useMultiContent<T extends object>({
     }
 
     return Boolean(updateContentValidity(updatedValidation));
-  }, [updateContentValidity]);
+  }, [validation.isValid, updateContentValidity]);
 
   /**
    * Update a content. It replaces the content in our "contents" map and update
@@ -186,7 +200,7 @@ export function useMultiContent<T extends object>({
         });
       }
     },
-    [updateContentValidity, onChange]
+    [updateContentValidity, onChange, getData, validate]
   );
 
   /**
@@ -211,6 +225,7 @@ export function useMultiContent<T extends object>({
 
   return {
     getData,
+    getSingleContentData,
     validate,
     validation,
     updateContentAt,

diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/components/form_data_provider.ts b/src/plugins/es_ui_shared/static/forms/hook_form_lib/components/form_data_provider.ts
@@ -29,6 +29,7 @@ interface Props {
 
 export const FormDataProvider = React.memo(({ children, pathsToWatch }: Props) => {
   const form = useFormContext();
+  const { subscribe } = form;
   const previousRawData = useRef<FormData>(form.__getFormData$().value);
   const [formData, setFormData] = useState<FormData>(previousRawData.current);
 
@@ -54,9 +55,9 @@ export const FormDataProvider = React.memo(({ children, pathsToWatch }: Props) =
   );
 
   useEffect(() => {
-    const subscription = form.subscribe(onFormData);
+    const subscription = subscribe(onFormData);
     return subscription.unsubscribe;
-  }, [form.subscribe, onFormData]);
+  }, [subscribe, onFormData]);
 
   return children(formData);
 });
diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/components/use_array.ts b/src/plugins/es_ui_shared/static/forms/hook_form_lib/components/use_array.ts
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import { useState, useEffect, useRef } from 'react';
+import { useState, useEffect, useRef, useCallback } from 'react';
 
 import { useFormContext } from '../form_context';
 
@@ -83,14 +83,18 @@ export const UseArray = ({
 
   const [items, setItems] = useState<ArrayItem[]>(initialState);
 
-  const updatePaths = (_rows: ArrayItem[]) =>
-    _rows.map(
-      (row, index) =>
-        ({
-          ...row,
-          path: `${path}[${index}]`,
-        } as ArrayItem)
-    );
+  const updatePaths = useCallback(
+    (_rows: ArrayItem[]) => {
+      return _rows.map(
+        (row, index) =>
+          ({
+            ...row,
+            path: `${path}[${index}]`,
+          } as ArrayItem)
+      );
+    },
+    [path]
+  );
 
   const addItem = () => {
     setItems((previousItems) => {
@@ -108,11 +112,13 @@ export const UseArray = ({
 
   useEffect(() => {
     if (didMountRef.current) {
-      setItems(updatePaths(items));
+      setItems((prev) => {
+        return updatePaths(prev);
+      });
     } else {
       didMountRef.current = true;
     }
-  }, [path]);
+  }, [path, updatePaths]);
 
   return children({ items, addItem, removeItem });
 };
diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/components/use_field.test.tsx b/src/plugins/es_ui_shared/static/forms/hook_form_lib/components/use_field.test.tsx
@@ -30,8 +30,9 @@ describe('<UseField />', () => {
 
     const TestComp = ({ onData }: { onData: OnUpdateHandler }) => {
       const { form } = useForm();
+      const { subscribe } = form;
 
-      useEffect(() => form.subscribe(onData).unsubscribe, [form]);
+      useEffect(() => subscribe(onData).unsubscribe, [subscribe, onData]);
 
       return (
         <Form form={form}>