refinedev / refine

A React Framework for building internal tools, admin panels, dashboards & B2B apps with unmatched flexibility.
https://refine.dev
MIT License
28.61k stars 2.24k forks source link

[FEAT] Improving Type Parameter Specification in useForm Hook #6137

Open noritsune opened 4 months ago

noritsune commented 4 months ago

Is your feature request related to a problem? Please describe.

I'm developing a user information editing page using Refine, and I'm facing difficulties in clearly distinguishing between the form structure and the data type submitted to the DataProvider.

If you know of some way, please let me know.

Describe alternatives you've considered

If I don't specify types in the useForm function, this issue doesn't occur. However, that would negate the purpose of using TypeScript.

Additional context

Current Status and Investigation

I've extensively reviewed documentation and advanced tutorials on how to specify useForm's type parameters effectively, but haven't found a solution yet.

Context

Here is the project setup and code where I encountered the issue.

Framework Structure

Routing: Next.js (App Router) Data Provider: SimpleRest UI: Material UI

The Flow of Data

DataFlow

Problematic Code

I think it's beneficial that the FormProps type specified as the third type parameter of useForm is used by the reset and handleSubmit functions. This is because they rely on the form structure.

However, the same type is also enforced for the argument of the onFinish function. This causes a type error when trying to convert the FormProps type to the User type inside the handleSubmitInner function during submission.

"use client";

import { HttpError } from "@refinedev/core";
import { Edit } from "@refinedev/mui";
import { useForm } from "@refinedev/react-hook-form";
import { useEffect } from "react";

interface User {
  email: string;
  user_metadata: {
    display_name: string;
  };
}

interface FormProps {
  email: string;
  userName: string;
}

export default function UserEdit() {
  const {
    refineCore: { queryResult, onFinish },
    reset,
    handleSubmit,
  } = useForm<User, HttpError, FormProps>();

  const user = queryResult?.data?.data;

  // Called when the page is loaded
  useEffect(() => {
    // Map the user information received from the API to the form
    // Calling reset sets the values entered in the form all at once
    reset({
      email: user?.email,
      userName: user?.user_metadata?.display_name,
    });
  }, [reset, user]);

  // Called when submitting the form
  const handleSubmitInner = (formProps: FormProps) => {
    // Map the information entered in the form to the API
    onFinish({
      email: formProps.email,
      // A type error is output at this line.
      // This is because the argument type of the onFinish function becomes FormProps,
      // which is specified as the third type parameter of useForm.
      user_metadata: {
        display_name: formProps.userName,
      },
    });
  };

  return (
    <Edit
      saveButtonProps={{
        onClick: handleSubmit(handleSubmitInner),
      }}
    >
      {/* ... */}
    </Edit>
  );
}

Describe the thing to improve

To resolve this issue, consider adding an additional type parameter like SubmissionData to useForm. This would clarify distinctions between form structure and submission data types, ensuring a smoother TypeScript-driven development process.

"use client";

import { HttpError } from "@refinedev/core";
import { Edit } from "@refinedev/mui";
import { useForm } from "@refinedev/react-hook-form";
import { useEffect } from "react";

interface User {
  email: string;
  user_metadata: {
    display_name: string;
  };
}

interface FormProps {
  email: string;
  userName: string;
}

export default function UserEdit() {
  const {
    refineCore: { queryResult, onFinish },
    reset,
    handleSubmit,
    // The 4th type parameter is the type of the submission.
    // It will be a argument of onFinish, so it should be User.
  } = useForm<User, HttpError, FormProps, User>();

  const user = queryResult?.data?.data;

  // Called when the page is loaded
  useEffect(() => {
    // Map the user information received from the API to the form
    // Calling reset sets the values entered in the form all at once
    reset({
      email: user?.email,
      userName: user?.user_metadata?.display_name,
    });
  }, [reset, user]);

  // Called when submitting the form
  const handleSubmitInner = (formProps: FormProps) => {
    // Map the information entered in the form to the API
    onFinish({
      email: formProps.email,
      user_metadata: {
        display_name: formProps.userName,
      },
    });
  };

  return (
    <Edit
      saveButtonProps={{
        onClick: handleSubmit(handleSubmitInner),
      }}
    >
      {/* ... */}
    </Edit>
  );
}
aliemir commented 4 months ago

Hey @noritsune thank you for the issue! I think I understand the problem here. While the useForm hook from @refinedev/core exposes an onFinish handler to submit values and doesn't integrate with forms automatically, having a single generic TVariables is enough but when we extend the useForm with form libraries (such as react-hook-form) it's understandable to require two generics, lets say TVariables and TSubmission, one is working with the form elements and other can be used to type onSubmit function to make sure the specs are met.

The 4th generic can be the same with the 3rd by default and it will also only trigger errors if onSubmit (or equivalent) is used manually. We can use type casting to prevent errors internally and assume users know what they're doing when they provide the 4th generic 🤔

Do I understand it correctly?

BTW, as a workaround I remember that one time I used a union type as 3rd that matches both the form and submission type and handled types in the custom onFinish/handleSubmit handler but having an option of a 4th generic looks better to me 😅

noritsune commented 4 months ago

@aliemir Thank you for your response. I think your understanding is correct. Also, I've mostly grasped what you're saying. So, are you suggesting that useForm should allow specifying the type of the onFinish function's argument (i.e., the type of data sent to the DataProvider) as the 4th type option?

stale[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 1 week ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

chhaysotheara commented 1 day ago

+1

It's really hard to use TS without proper typing in here. Also, thanks to @aliemir for the workaround.

I think it's better to use union type rather than "any"

Hopefully we can get an enhancement on this soon.