Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
noritsune opened this issue Jul 13, 2024 · 3 comments
Open

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

noritsune opened this issue Jul 13, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@noritsune
Copy link
Contributor

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>
  );
}
@noritsune noritsune added the enhancement New feature or request label Jul 13, 2024
@aliemir
Copy link
Member

aliemir commented Jul 16, 2024

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
Copy link
Contributor Author

@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?

Copy link

stale bot commented Sep 16, 2024

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 stale bot added the wontfix This will not be worked on label Sep 16, 2024
@aliemir aliemir removed the wontfix This will not be worked on label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants