inertia icon indicating copy to clipboard operation
inertia copied to clipboard

Fix React's useForm hook that returns new references for functions like setData, clearErrors, etc on every re-render

Open Hasan-Mir opened this issue 1 year ago • 3 comments

When using the useForm() in a React component that has some heavy pure components(components using React.memo) as children and passing some methods returned by useForm() like setData, clearErrors, etc to them, on every re-render, these methods returned by useForm() are getting new references so new props are passed to these React.memo()ed children and they will be re-rendered unnecessarily. This pull request fixes this issue by using useCallback for those methods in the useForm hook.

Hasan-Mir avatar Jul 10 '23 06:07 Hasan-Mir

I just want to flag that this PR (mostly) fixed some issues that we were running into. I have ended up taking a copy of this useForm implementation and referencing it directly in our application. So just giving a +1 that this PR could be prioritised to bring in.

I also needed to add data as a dependency of the transformFunction callback to ensure that it worked correctly for how we are using the hook:

  const transformFunction = useCallback(
    (callback) => {
      transform = callback;
    },
    // This dependency was added
    [data]
  );

jameshulse avatar Aug 06 '23 22:08 jameshulse

Hi @jameshulse, thank you for the hint, I've fixed it in the latest commit by using the useRef instead of declaring the transform as let transform = (data) => data. This way the transform function maintains the same reference between re-renders.

Furthermore, using useRef addresses another issue in the current useForm implementation. If we use a state inside of the callback that we pass to the transform function and we update that state without updating data (calling setData), the transform function will not utilize the updated state. Instead, it will use the stale version of it.

function GalleryForm(props) {
  const { data, submit, errors, setData, processing, transform } = useForm({
    picture: null,
    picurl: gallery?.type === 0 ? gallery?.url : "",
  });

  const [pictureSourceType, setPictureSourceType] = useState(
    data.picurl ? "pic_url" : "pic_upload"
  );

  transform((prevData) => ({
    ...prevData,
    picture: prevData.type === 0 || pictureSourceType === "pic_url" ? null : prevData.picture,
    picurl: prevData.type === 1 || pictureSourceType === "pic_upload" ? null : prevData.picurl,
  }));

  function handelSubmit(e) {
    e.preventDefault();
    submit("post", "gallery", {
      forceFormData: true,
    });
  }

  return (
    <form>
      {/* ... */}

      <TabButton
        onClick={() => {
          setPictureSourceType("pic_upload");
        }}
      >
        upload image
      </TabButton>
      <TabButton
        onClick={() => {
          setPictureSourceType("pic_url");
        }}
      >
        use image url
      </TabButton>

      {/* ... */}
    </form>
  );
}

Here when I update the pictureSourceType (for example from "pic_url" to "pic_upload") and then try to submit the form, the value of pictureSourceType remains as "pic_url" when the transform function executes! This is due to the dependency list of the submit function, which is [data, setErrors]. As long as the data remains unchanged, the submit function maintains the same function reference from the last re-render where data was modified and received a new reference. So the value of transform function belongs to that render. To address this issue without using useRef we need to call setData wherever we update that state that we use inside of the transform's callback (note that I don't want to set picture or picurl to null when I change the pictureSourceType tab). This approach ensures that the submit function employs the most recent value of the transform.

function GalleryForm(props) {
 // ...

 return (
   <form>
     {/* ... */}

     <TabButton
       onClick={() => {
         setPictureSourceType("pic_upload");
+        setData((prevData) => ({ ...prevData })); 
       }}
     >
       upload image
     </TabButton>
     <TabButton
       onClick={() => {
         setPictureSourceType("pic_url");
+        setData((prevData) => ({ ...prevData })); 
       }}
     >
       use image url
     </TabButton>

     {/* ... */}
   </form>
 );
}

Hasan-Mir avatar Aug 07 '23 12:08 Hasan-Mir

Anyone has an idea on what this PR is waiting? Or could I patch package this somehow?

kendepelchin avatar Dec 28 '23 12:12 kendepelchin