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

[BUG] Cannot use react-hook-form Controller with Refine's useForm #6139

Closed
ritute opened this issue Jul 14, 2024 · 17 comments · Fixed by #6161 or #6154
Closed

[BUG] Cannot use react-hook-form Controller with Refine's useForm #6139

ritute opened this issue Jul 14, 2024 · 17 comments · Fixed by #6161 or #6154
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@ritute
Copy link
Contributor

ritute commented Jul 14, 2024

Describe the bug

I'm following the example for Material UI on this page: https://refine.dev/docs/packages/react-hook-form/introduction/#usage

My imports are:

import { useForm } from "@refinedev/react-hook-form";
import { Controller } from "react-hook-form";

The problem is that it looks we have to use the Edit component from Material UI in order to use Controller, otherwise I get the error:

React Router caught the following error during render TypeError: Cannot read properties of undefined (reading 'subscribe')

When I wrap my form within the Edit component, this error goes away, but I don't want to use Edit from material UI for my form. Is there a way around this? I'm currently having to use register instead, but I need to use some Material components like Autocomplete.

Steps To Reproduce

  1. Follow the steps in this documentation for Material UI: https://refine.dev/docs/packages/react-hook-form/introduction/#usage
  2. Remove the Edit component (don't wrap form with it)

Expected behavior

Shouldn't require the Edit component in order to use Controller in my form.

Packages

  • react-hook-form: "7.30.0"
  • @refinedev/react-hook-form: "^4.8.20"

Additional Context

No response

@ritute ritute added the bug Something isn't working label Jul 14, 2024
@aliemir
Copy link
Member

aliemir commented Jul 16, 2024

Hey @ritute, using a base Refine + Material UI + React Hook Form setup, I've followed the steps to repro the issue. Just copy-pasted the full Material UI code block and removed the <Edit /> component but could not reproduce. Can you try to share the broken code from your source or provide a minimal repro? 🙏

@ritute
Copy link
Contributor Author

ritute commented Jul 17, 2024

https://refine.dev/docs/packages/react-hook-form/introduction/#usage

Hmm, okay I just started with a base sandbox and don't see the issue either. Gonna try to figure out what's different then in my project's setup.

@ritute
Copy link
Contributor Author

ritute commented Jul 17, 2024

@aliemir here's a reproduction

The subscribe error only happens when use useForm from refine, not when using the one from react-hook-form. It actually doesn't work even with the Edit wrapped around in this case.

@ritute ritute changed the title [BUG] Cannot use react-hook-form Controller without Material UI Edit [BUG] Cannot use react-hook-form Controller with Refine's useForm Jul 17, 2024
@alicanerdurmaz
Copy link
Member

Hi @ritute,

I found some syntax problems in your project and fixed them, but I didn't see the issue you're facing. However, I did find another error:

  ➜  Local:   http://localhost:3000/
  ➜  Network: use --host to expose
  ➜  press h + enter to show help
✘ [ERROR] Could not resolve "@mui/material/DefaultPropsProvider"

    node_modules/@mui/lab/LoadingButton/LoadingButton.js:12:32:
      12 │ import { useDefaultProps } from '@mui/material/DefaultPropsProvider';
         ╵                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  You can mark the path "@mui/material/DefaultPropsProvider" as external to exclude it from the
  bundle, which will remove this error and leave the unresolved path in the bundle.

/Users/alicanerdurmaz/Desktop/refine-form-demo/node_modules/esbuild/lib/main.js:1651
  let error = new Error(text);
              ^

Error: Build failed with 1 error:
node_modules/@mui/lab/LoadingButton/LoadingButton.js:12:32: ERROR: Could not resolve "@mui/material/DefaultPropsProvider"
    at failureErrorWithLog (/Users/alicanerdurmaz/Desktop/refine-form-demo/node_modules/esbuild/lib/main.js:1651:15)
    at /Users/alicanerdurmaz/Desktop/refine-form-demo/node_modules/esbuild/lib/main.js:1059:25
    at /Users/alicanerdurmaz/Desktop/refine-form-demo/node_modules/esbuild/lib/main.js:1527:9
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  errors: [Getter/Setter],
  warnings: [Getter/Setter]
}

Node.js v20.11.0

This error was resolved when I updated @mui/material to the latest version.

Could you please recheck the project you shared? I couldn't reproduce the error you got.

@ritute
Copy link
Contributor Author

ritute commented Jul 18, 2024

@alicanerdurmaz @aliemir fixed the syntax error for the commented Edit wrapper, my bad. I don't get that MUI error. Can you make sure you're using the yarn lock file that's there with yarn install? I just pulled the repo from scratch and ran into the issue I posted after a yarn install. It appears in the browser, not in the terminal output. Here's a screenshot.

Screen Shot 2024-07-18 at 12 19 41 PM

@aliemir
Copy link
Member

aliemir commented Jul 18, 2024

Hey @ritute thank you for the update. I cloned the repo and used yarn v1 to install. Now I can reproduce the same issue 🙌

I'll try to investigate on what caused the issue but when I replace the react-hook-form dependency version from 7.30.0 to ^7.30.0 and that installed 7.52.1 (latest) and the error is gone and rendering is done properly. 🚀

Can you check if updating the dependency also works for you? Do you have any limitations to keep the react-hook-form pinned at 7.30.0? 🤔

@ritute
Copy link
Contributor Author

ritute commented Jul 18, 2024

@aliemir I noticed that the Refine library was using this version 7.30.0 for react-hook-form, so I decided to go with that one. But I don't need to, I'll try updating it, thanks! If the root cause can't be fixed, would be great to update the docs to ensure people don't use that version 👍

@ritute
Copy link
Contributor Author

ritute commented Jul 19, 2024

@aliemir it worked with version 7.52.1 (but needed to add it to resolutions since Refine is using 7.30.0)

@aliemir
Copy link
Member

aliemir commented Jul 19, 2024

The version bump of react-hook-form to ^7.30.0 is done when we're moving to React 18. react-hook-form added React 18 support with v7.29.0.

I found the issue in the react-hook-form package, which was resolved with PR react-hook-form/react-hook-form#10052 and released with 7.43.5. I didn't check our code for the useForm but looks like one of the changes made since the last year doesn't work with versions prior 7.43.5 🤔

Happy to see the issue is resolved after the update. Since we've found the exact version that resolves the issue, I think we can mark our react-hook-form dependency as ^7.43..5 🙌

@aliemir aliemir added the good first issue Good for newcomers label Jul 19, 2024
@aliemir
Copy link
Member

aliemir commented Jul 19, 2024

If anyone is interested on working on the fix, it should be as simple as replacing all "react-hook-form": "^7.30.0" with "react-hook-form": "^7.43.5" in the repo. @ritute I can assign this to you if you can work on it. We'll be happy to see your PR 🙏

@ritute
Copy link
Contributor Author

ritute commented Jul 19, 2024 via email

@ritute
Copy link
Contributor Author

ritute commented Jul 19, 2024

@aliemir is there any reason you guys don't expose Controller in your @refinedev/react-hook-form package, so that we don't need those two separate dependencies?

@aliemir
Copy link
Member

aliemir commented Jul 19, 2024

@ritute, @refinedev/react-hook-form is an extension package that extends useForm of @refinedev/core with abilities of react-hook-form. We try to keep our packages clean and don't export anything that we don't modify/extend. We take the useForm from react-hook-form and add all existing features of @refinedev/core's useForm such as API interactions, error propagation, redirections, invalidation etc.

For the previous question, check out the source code for the useForm from @refinedev/react-hook-form:

https://github.com/refinedev/refine/blob/master/packages/react-hook-form/src/useForm/index.ts

We're not just using or re-implementing the useForm hook from react-hook-form but we're extending it. Looks like watch (useSubscribe under the hood -the one causing the errors-) is used for notifying the user on unsaved changes and for the auto-save feature. Since those are not present in react-hook-form's useForm, we don't get any errors if we use it directly.

I hope I clarified the difference and the reason behind two packages and two hooks ✌️

@ritute
Copy link
Contributor Author

ritute commented Jul 19, 2024

@aliemir getting this when running pnpm install in latest master.

Screen Shot 2024-07-19 at 12 59 12 PM

@aliemir
Copy link
Member

aliemir commented Jul 19, 2024

@ritute I checked but couldn't find the same version code. Can you check blog-material-ui-datagrid/package.json to see if there's a broken version tag in react dependency?

@ritute
Copy link
Contributor Author

ritute commented Jul 19, 2024

@aliemir ah, oh my, I somehow added that... 😂. All good! BTW, there are several tests that fail in master for me - looks like a timezone related issue.

Screen Shot 2024-07-19 at 2 11 55 PM Screen Shot 2024-07-19 at 2 11 40 PM

ritute added a commit to ritute/refine that referenced this issue Jul 19, 2024
@ritute
Copy link
Contributor Author

ritute commented Jul 19, 2024

#6161

@aliemir aliemir linked a pull request Jul 19, 2024 that will close this issue
5 tasks
@aliemir aliemir added this to the August Release milestone Jul 19, 2024
ritute added a commit to ritute/refine that referenced this issue Jul 19, 2024
@aliemir aliemir linked a pull request Jul 26, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
3 participants