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

Breaking change to Validate in 6.5.1 #265

Closed
2 tasks done
rynoV opened this issue May 10, 2024 · 9 comments
Closed
2 tasks done

Breaking change to Validate in 6.5.1 #265

rynoV opened this issue May 10, 2024 · 9 comments

Comments

@rynoV
Copy link

rynoV commented May 10, 2024

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Thanks for making this!

I'm mostly just filing this for documentation: there was a breaking change to the types in version 6.5.1, in this commit 75d70f4. When using the rules or validation props on some elements there might be a new type error like:

error TS2322: Type '(value: string) => Promise<ValidateResult>' is not assignable to type 'Validate<string | number | boolean | string[]>'.
  Types of parameters 'value' and 'value' are incompatible.
    Type 'string | number | boolean | string[]' is not assignable to type 'string'.
      Type 'number' is not assignable to type 'string'.

289           async parse(value: string): Promise<ValidateResult> {

Expected behavior 🤔

No response

Steps to reproduce 🕹

Steps:

@rynoV
Copy link
Author

rynoV commented May 10, 2024

It seems like the issue is specific to the validate key within ControllerProps.rules. The referenced commit added the TFieldValues generic to ControllerProps['rules'], but the TName isn't present, so the Validate type isn't being narrowed to the type of the field specified by TName, and instead is defaulting to a union of all types of fields within the form.

I don't have any pressing need to upgrade the version so my solution for now is just to lock the version to 6.5.0

@dohomi
Copy link
Owner

dohomi commented May 13, 2024

could you provide a repo / codesandbox which higlights the issue that would be helpful

@sadik-malik
Copy link
Contributor

@rynoV have you tested it with v6.8.0 or the latest version v7.0.0?

@rynoV
Copy link
Author

rynoV commented May 13, 2024

@rynoV have you tested it with v6.8.0 or the latest version v7.0.0?

I see the issue on both of these versions

Here's a sandbox: https://codesandbox.io/p/sandbox/react-hook-form-mui-zh4mxg

It looks like somewhere past v6.5.1 the TName generic was added (at least for the Autocomplete), so I was able to fix it in 6.8.0 by specifying the TName generic with Extract<FieldPath<FormValues>, 'field'> (which works for nested fields as well, unlike my original attempt FieldPath<Pick<FormValues, 'field'>>)

I couldn't find a way to make type inference work to find the most specific TName from the name property, so I had to add the generic manually

@dohomi
Copy link
Owner

dohomi commented May 13, 2024

@rynoV you would never need to provide it in the way you did in, here is an updated version:
https://codesandbox.io/p/sandbox/react-hook-form-mui-forked-8nhq9h?file=%2Fsrc%2FApp.tsx%3A39%2C45

you can also have a look at https://github.com/dohomi/react-hook-form-mui/blob/master/apps/storybook/stories/FormContainer.stories.tsx#L145 which is meant to have strict typings example.

Basically if you provide control of useForm and you set your default defaultValues the control prop knows which names are allowed within the form context.

@dohomi
Copy link
Owner

dohomi commented May 14, 2024

I add a typesafe example to the landing page of Github https://github.com/dohomi/react-hook-form-mui?tab=readme-ov-file#typesafe-form-setup

@dohomi dohomi closed this as completed May 14, 2024
@rynoV
Copy link
Author

rynoV commented May 16, 2024

@dohomi Thanks for the example in the docs. I couldn't access the sandbox you sent, but I tried to update my sandbox based on the example in the docs and the type error is still present until I specify the second generic: https://codesandbox.io/p/sandbox/react-hook-form-mui-zh4mxg?file=%2Fsrc%2FApp.tsx%3A10%2C32

Re the other stories link, I didn't see any examples there with the rules.validate prop, which seems to be the main affected area of the breaking change (or at least the only area I've run into issues with)

Please let me know if I'm missing anything

@dohomi
Copy link
Owner

dohomi commented May 17, 2024

you should not pass any types because control is taking care of it, do you see this example:

https://codesandbox.io/p/devbox/h3spsq?file=%2Fsrc%2FApp.tsx

import { FieldPath, useForm } from "react-hook-form";
import { AutocompleteElement } from "react-hook-form-mui";
import "./styles.css";

interface FormValues {
  field: string;
  otherField: number;
}

export default function App() {
  const { control, handleSubmit } = useForm<FormValues>({
    defaultValues: {
      field: "",
      otherField: 0,
    },
  });
  return (
    <div className="App">
      <h1>Hello CodeSandbox</h1>
      <h2>Start editing to see some magic happen!</h2>
      <AutocompleteElement
        control={control}
        name={"field"}
        options={[]}
        rules={{
          validate: {
            parse(value) {
              console.log(value);
              return true;
            },
          },
        }}
      />
    </div>
  );
}

@rynoV
Copy link
Author

rynoV commented May 17, 2024

Ah okay I see now, don't specify any generics and pass a control. I wasn't passing control before because I was relying on FormContainer, but I suppose using useFormContext and passing the returned control is what the component is doing anyways so it should be equivalent.

It would be nice if I didn't need to change the code purely to affect things at the type level, but it seems like this would require partial type inference to stop typescript from using the default value for the TName generic. The only other solution I could see is a dummy/phantom parameter like fieldValuesType={undefined as MyFieldValues}, which I might prefer but I'm not that experienced with typescript API design

Appreciate the help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants