fix(form): initialize form on first render [skip ci]

packages/form/__tests__/form.test.tsx

@@ -1,10 +1,17 @@
 /**
  * @jest-environment jsdom
  */
-import React, { useState } from "react";
+import React from "react";
 import { render, screen, waitFor, fireEvent, act } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { Form, useBind, FormProps as BaseFormProps, BindComponentProps, FormAPI } from "~/index";
+import {
+    Form,
+    useBind,
+    FormProps as BaseFormProps,
+    BindComponentProps,
+    FormAPI,
+    Bind
+} from "~/index";
 import { validation } from "@webiny/validation";
 
 type FormProps = Omit<BaseFormProps, "children" | "onSubmit"> & {
@@ -13,8 +20,28 @@ type FormProps = Omit<BaseFormProps, "children" | "onSubmit"> & {
     children?: React.ReactNode;
 };
 
+const EmptyForm = ({ children, data, onSubmit, imperativeHandle, ...props }: FormProps) => {
+    const formData = data;
+
+    return (
+        <Form
+            ref={imperativeHandle}
+            data={formData}
+            onSubmit={data => onSubmit && onSubmit(data)}
+            {...props}
+        >
+            {({ form }) => (
+                <div>
+                    {children}
+                    <button onClick={form.submit}>Submit</button>
+                </div>
+            )}
+        </Form>
+    );
+};
+
 const FormViewWithBind = ({ children, data, onSubmit, imperativeHandle, ...props }: FormProps) => {
-    const [formData] = useState(data || { name: "empty name" });
+    const formData = data || { name: "empty name" };
 
     return (
         <Form
@@ -289,20 +316,42 @@ describe("Form", () => {
         const user = userEvent.setup();
         const onSubmit = jest.fn();
 
-        const { rerender } = render(<FormViewWithHooks onSubmit={onSubmit} data={{}} />);
+        const { rerender } = render(<EmptyForm onSubmit={onSubmit} data={{}} />);
 
         const submitBtn = screen.getByRole("button", { name: /submit/i });
         await user.click(submitBtn);
 
         expect(onSubmit).toHaveBeenLastCalledWith({});
 
-        rerender(<FormViewWithHooks onSubmit={onSubmit} data={{ email: "test@example.com" }} />);
+        rerender(<EmptyForm onSubmit={onSubmit} data={{ email: "test@example.com" }} />);
         await user.click(submitBtn);
 
         await waitFor(() => onSubmit.mock.calls.length > 0);
         expect(onSubmit).toHaveBeenLastCalledWith({ email: "test@example.com" });
     });
 
+    test("should set default field value on first render cycle", async () => {
+        const ref = React.createRef<FormAPI>();
+        const onSubmit = jest.fn();
+
+        render(
+            <EmptyForm onSubmit={onSubmit} imperativeHandle={ref}>
+                <Bind name={"folder"} defaultValue={{ id: "root" }}>
+                    {({ value }) => <div data-testid={"folderId"}>{value.id}</div>}
+                </Bind>
+            </EmptyForm>
+        );
+
+        // Assert
+        await act(() => ref.current?.submit());
+        await waitFor(() => onSubmit.mock.calls.length > 0);
+        expect(onSubmit).toHaveBeenLastCalledWith({ folder: { id: "root" } });
+
+        const folderDiv = screen.getByTestId("folderId");
+        expect(folderDiv).toBeTruthy();
+        expect(folderDiv.innerHTML).toEqual("root");
+    });
+
     test("should submit the form using imperative handle", async () => {
         const user = userEvent.setup();
         const ref = React.createRef<FormAPI>();

packages/form/src/Form.tsx

@@ -1,6 +1,8 @@
-import React, { useEffect, useImperativeHandle, useMemo } from "react";
+import React, { useEffect, useImperativeHandle, useMemo, useRef } from "react";
 import { observer } from "mobx-react-lite";
 import lodashNoop from "lodash/noop";
+import isEqual from "lodash/isEqual";
+
 import { Bind } from "./Bind";
 import { FormProps, GenericFormData } from "~/types";
 import { FormContext } from "./FormContext";
@@ -11,7 +13,22 @@ function FormInner<T extends GenericFormData = GenericFormData>(
     props: FormProps<T>,
     ref: React.ForwardedRef<any>
 ) {
-    const presenter = useMemo(() => new FormPresenter<T>(), []);
+    const dataRef = useRef(props.data);
+
+    const presenter = useMemo(() => {
+        const presenter = new FormPresenter<T>();
+        presenter.init({
+            data: (props.data || {}) as T,
+            onChange: data => {
+                if (typeof props.onChange === "function") {
+                    props.onChange(data, formApi);
+                }
+            },
+            onInvalid: props.onInvalid
+        });
+        return presenter;
+    }, []);
+
     const formApi = useMemo(() => {
         return new FormAPI(presenter, {
             onSubmit: props.onSubmit ?? lodashNoop,
@@ -28,24 +45,21 @@ function FormInner<T extends GenericFormData = GenericFormData>(
         });
     }, [props.onSubmit, props.disabled, props.validateOnFirstSubmit]);
 
-    useEffect(() => {
-        presenter.init({
-            data: (props.data || {}) as T,
-            onChange: data => {
-                if (typeof props.onChange === "function") {
-                    props.onChange(data, formApi);
-                }
-            },
-            onInvalid: props.onInvalid
-        });
-    }, []);
-
     useEffect(() => {
         presenter.setInvalidFields(props.invalidFields || {});
     }, [props.invalidFields]);
 
     useEffect(() => {
+        // We only set form's data if props.data has changed.
+        if (isEqual(dataRef.current, props.data)) {
+            return;
+        }
+
+        // Set the new form data.
         presenter.setData(props.data as T);
+
+        // Keep the new props.data for future comparison.
+        dataRef.current = props.data;
     }, [props.data]);
 
     useImperativeHandle(ref, () => ({

packages/form/src/FormPresenter.ts

@@ -143,31 +143,24 @@ export class FormPresenter<T extends GenericFormData = GenericFormData> {
     registerField(props: BindComponentProps) {
         const existingField = this.formFields.get(props.name);
 
-        let field;
+        let field: FormField;
         if (existingField) {
             field = FormField.createFrom(existingField, props);
         } else {
             field = FormField.create(props);
         }
 
-        this.formFields.set(props.name, field);
-
         // We only want to handle default field value for new fields.
-        if (existingField) {
-            return;
+        if (!existingField) {
+            const fieldName = field.getName();
+            const currentFieldValue = lodashGet(this.data, fieldName);
+            const defaultValue = field.getDefaultValue();
+            if (currentFieldValue === undefined && defaultValue !== undefined) {
+                lodashSet(this.data, fieldName, defaultValue);
+            }
         }
 
-        // Set field's default value.
-        const fieldName = field.getName();
-        const currentFieldValue = lodashGet(this.data, fieldName);
-        const defaultValue = field.getDefaultValue();
-        if (currentFieldValue === undefined && defaultValue !== undefined) {
-            // We need to postpone the state update, because `registerField` is called within a render cycle.
-            // You can't set a new state, while the previous state is being rendered.
-            requestAnimationFrame(() => {
-                this.setFieldValue(fieldName, defaultValue);
-            });
-        }
+        this.formFields.set(props.name, field);
     }
 
     unregisterField(name: string) {

packages/form/src/useBind.ts

@@ -1,6 +1,5 @@
 import { useEffect } from "react";
 import { makeDecoratable } from "@webiny/react-composition";
-import lodashGet from "lodash/get";
 import { BindComponentProps, UseBindHook } from "~/types";
 import { useBindPrefix } from "~/BindPrefix";
 import { useForm } from "./FormContext";
@@ -14,10 +13,6 @@ export const useBind = makeDecoratable((props: BindComponentProps): UseBindHook
     const bindName = [bindPrefix, props.name].filter(Boolean).join(".");
 
     useEffect(() => {
-        if (props.defaultValue !== undefined && lodashGet(form.data, bindName) === undefined) {
-            form.setValue(bindName, props.defaultValue);
-        }
-
         return () => {
             form.unregisterField(props.name);
         };