From 803a16b8a78c1204101ae1b6ea22002811308a4c Mon Sep 17 00:00:00 2001
From: Strahinja Ajvaz <strahinja.ajvaz@gmail.com>
Date: Wed, 28 Apr 2021 16:24:59 +1000
Subject: [PATCH] fixed issue with  validateData being stale (#215)

---
 src/components/Form.js         | 17 ++++---
 src/components/Form.test.js    | 84 ++++++++++++++++++++++++++++++----
 src/hooks/internal/useField.js | 32 ++++++++++---
 3 files changed, 112 insertions(+), 21 deletions(-)

diff --git a/src/components/Form.js b/src/components/Form.js
index 95461ced..98e0e526 100644
--- a/src/components/Form.js
+++ b/src/components/Form.js
@@ -123,12 +123,17 @@ function Form(_props) {
 
     lastMouseDownInputElement.current = inputElement;
   };
-  const registerField = (name, field) => {
+  const registerField = useCallback((name, field) => {
     fields.current[name] = field;
-  };
-  const unregisterField = (name) => {
+  }, []);
+  const unregisterField = useCallback((name) => {
     delete fields.current[name];
-  };
+    setState((state) =>
+      deletePath(state, `errors.${name}`, {
+        deleteEmptyObjects: { except: ["errors"] },
+      })
+    );
+  }, []);
   const providerValue = {
     state,
     onFocus, // should be called by inputs
@@ -140,7 +145,7 @@ function Form(_props) {
   };
   const getFieldErrors = useCallback((values, name) => {
     const value = getPath(values, name);
-    const field = fields.current[name];
+    const field = fields.current[name].current;
 
     if (
       !field || // See: https://stackoverflow.com/q/65659161/247243
@@ -182,7 +187,7 @@ function Form(_props) {
   );
   const validateField = useCallback(
     (name) => {
-      if (fields.current[name]) {
+      if (fields.current[name].current) {
         setState((state) => {
           const errors = getFieldErrors(state.values, name);
 
diff --git a/src/components/Form.test.js b/src/components/Form.test.js
index 0547de00..34d6b2aa 100644
--- a/src/components/Form.test.js
+++ b/src/components/Form.test.js
@@ -61,6 +61,17 @@ const hungryOptions = [
   },
 ];
 
+const yesNoOptions = [
+  {
+    label: "Yes",
+    value: "yes",
+  },
+  {
+    label: "No",
+    value: "no",
+  },
+];
+
 function SimpleForm({ testId }) {
   const initialValues = {
     name: "",
@@ -334,14 +345,12 @@ describe("Form", () => {
 
     userEvent.type(screen.getByLabelText("Name"), "Helena");
 
-    await waitFor(() => {
-      expect(
-        screen.queryByText("This name is already taken.")
-      ).not.toBeInTheDocument();
-      expect(
-        screen.queryByText("Try to spell it differently.")
-      ).not.toBeInTheDocument();
-    });
+    expect(
+      screen.queryByText("This name is already taken.")
+    ).not.toBeInTheDocument();
+    expect(
+      screen.queryByText("Try to spell it differently.")
+    ).not.toBeInTheDocument();
   });
 
   it("doesn't validate fields when they become disabled", async () => {
@@ -613,3 +622,62 @@ describe("Form", () => {
     });
   });
 });
+
+it("with hidden fields", async () => {
+  const onSubmit = jest.fn();
+  const { container } = render(
+    <Form
+      initialValues={{ hasMiddleName: "", middleName: "" }}
+      onSubmit={onSubmit}
+    >
+      {({ state }) => (
+        <>
+          <RadioGroup
+            name="hasMiddleName"
+            label="Do you have a middle name?"
+            options={yesNoOptions}
+          />
+          {state.values.hasMiddleName === "yes" && (
+            <Input name="middleName" label="Middle name" />
+          )}
+          <Button type="submit">Submit</Button>
+        </>
+      )}
+    </Form>
+  );
+
+  const yesInput = screen.getByLabelText("Yes");
+  const yesLabel = container.querySelector(
+    `label[for="${yesInput.getAttribute("id")}"]`
+  );
+  const noInput = screen.getByLabelText("No");
+  const noLabel = container.querySelector(
+    `label[for="${noInput.getAttribute("id")}"]`
+  );
+
+  userEvent.click(yesLabel);
+
+  const middleNameInput = screen.getByLabelText("Middle name");
+  middleNameInput.focus();
+  middleNameInput.blur();
+
+  // Wait until form state is updated with the error.
+  // Without this await, the test ends before form's state gets the error.
+  await waitFor(() => {
+    expect(screen.getByText("Required")).toBeInTheDocument();
+  });
+
+  userEvent.click(noLabel);
+  screen.getByRole("button", { name: "Submit" }).click();
+
+  await waitFor(() =>
+    expect(onSubmit).toBeCalledWith({
+      errors: {},
+      values: {
+        hasMiddleName: "no",
+        middleName: "",
+      },
+      setErrors: expect.any(Function),
+    })
+  );
+});
diff --git a/src/hooks/internal/useField.js b/src/hooks/internal/useField.js
index a5490f7f..63c89e7c 100644
--- a/src/hooks/internal/useField.js
+++ b/src/hooks/internal/useField.js
@@ -1,4 +1,4 @@
-import { useEffect } from "react";
+import { useEffect, useRef } from "react";
 import useForm from "./useForm";
 import { getPath } from "../../utils/objectPath";
 
@@ -6,7 +6,6 @@ function useField(componentName, { name, disabled, optional, validate, data }) {
   if (typeof name !== "string" || name.trim() === "") {
     throw new Error(`${componentName} component is missing a name prop`);
   }
-
   const {
     state,
     onFocus,
@@ -17,6 +16,16 @@ function useField(componentName, { name, disabled, optional, validate, data }) {
     unregisterField,
   } = useForm(componentName);
 
+  const registerDataRef = useRef({
+    registerField,
+    unregisterField,
+    name,
+    disabled,
+    optional,
+    validate,
+    data,
+  });
+
   if (typeof state.values === "undefined") {
     throw new Error("Form is missing initialValues");
   }
@@ -30,16 +39,17 @@ function useField(componentName, { name, disabled, optional, validate, data }) {
   const errors = getPath(state.errors, name);
   const hasErrors = Array.isArray(errors) && errors.length > 0;
 
+  // we need to store the data in a ref so that if the validateData prop changes, we have a reference
+  // that we can update with the new value.
   useEffect(() => {
-    registerField(name, {
+    registerDataRef.current = {
+      registerField,
+      unregisterField,
+      name,
       disabled,
       optional,
       validate,
       data,
-    });
-
-    return () => {
-      unregisterField(name);
     };
   }, [
     name,
@@ -51,6 +61,14 @@ function useField(componentName, { name, disabled, optional, validate, data }) {
     unregisterField,
   ]);
 
+  useEffect(() => {
+    registerField(name, registerDataRef);
+
+    return () => {
+      unregisterField(name);
+    };
+  }, [registerField, unregisterField, name]);
+
   return {
     value,
     errors,