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

Working with react-hook-form #18269

Closed
2 tasks done
leolozes opened this issue Nov 8, 2019 · 40 comments
Closed
2 tasks done

Working with react-hook-form #18269

leolozes opened this issue Nov 8, 2019 · 40 comments
Labels
external dependency Blocked by external dependency, we can’t do anything about it new feature New feature or request

Comments

@leolozes
Copy link

leolozes commented Nov 8, 2019

I'm trying to do a simple form with react-hook-form and material-ui. I'd like to be able to:

  1. submit / validate a form
  2. reset the form correctly
  3. load data correctly

Sorry, bad issue search, it looks like a duplicate of #17018. Feel free to close this.

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The first step works correctly.
The reset function clears the value, but the state of the material-ui TextField seems to stay at "filled", and the styles don't match with the cleared data.
The reset function with initial values set correctly the value, but the behavior is the same as with reset (well, the opposite in this case), the data is loaded but the internal state of the TextField is not "filled".

Expected Behavior 🤔

The material-ui input should refresh its internal state when data is loaded / cleared

Steps to Reproduce 🕹

You can try the three buttons on this CodePen:
https://codesandbox.io/s/material-demo-ywutu

  1. Submit: works correctly validating the data.
  2. Correctly resets but the state of the TextFields is not empty.
  3. Correctly loads data but the state of the TextFields is not filled.

I filed an issue at the react-hook-form project but the owner told me to open one here as well, so here we are :)

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 8, 2019

I have added the waiting for users upvotes tag. I'm closing the issue as we are not sure people are looking for such support. So please upvote this issue if you are. We will prioritize our effort based on the number of upvotes.

https://npm-stat.com/charts.html?package=formik&package=react-hook-form&package=react-final-form

@stunaz
Copy link
Contributor

stunaz commented Nov 9, 2019

much needed 🙏🙏🙏 any workaround?

@sessionboy
Copy link

much needed!

@jgb-solutions
Copy link

Definitely needed.

@oliviertassinari oliviertassinari added the new feature New feature or request label Nov 19, 2019
@oliviertassinari
Copy link
Member

Any idea of what the solution would be?

@jgb-solutions
Copy link

jgb-solutions commented Nov 19, 2019

I'm not sure about something specific but I know I started using both libraries and I really would like to see tighter integrations between the two. React Hook Form and Material UI are just great together. Their support for TypeScript is also nice.

@bluebill1049
Copy link

Hey, @oliviertassinari thanks for taking a look at it. Looks like MUI TextField is working better with native input's API now! That's really nice. We used to have the place holder text overlay on top of the text when you invoke e.target.value = 'xxx'. 👍

Screen Shot 2019-11-20 at 10 06 26 am

https://codesandbox.io/s/react-hook-form-conditional-fields-delete-1frsm
In the example above <Switch /> didn't reset after reset API invoked.

It would be really nice if we can have all form related components to support native form input API (set and reset value via input's reference ) I do understand it's a large piece of work and to support a small lib like react-hook-form is something to be considered ( maybe until if the lib getting more popular ❤️)

again I do appreciate your time and effort to look into this issue (maintain a HUGE open source project like MUI is crazy to me and you are doing an amazing job to the React community). On top of that, let me know if anything I can work on my end to work with MUI too.

Cheers
Bill

@oliviertassinari
Copy link
Member

@bluebill1049 Thanks for looking at it. The library seems to get a good traction, but it's still far behind formik. Is the switch reset the only issue we have?

@bluebill1049
Copy link

thanks, @oliviertassinari for reopening and looking into this issue. 🙏 yes, you are right react-hook-form is still a new boy in the town and you don't have act if you would like to wait some more time to see if more users adopting react-hook-form, which I think it's totally reasonable (even tho inside my heart I am desperate to want it working with MUI, hahaha been selfish😼). In the meantime, I will create a table of inputs that is compatible with react-hook-form out the box (maybe even with a codesandbox too) I will post it in here as well.

If things work well with react-hook-form and usage growing bigger, maybe we can consider to working with native input API (which is what react hook form use behind the scenes) it would be great to get input, select, radio and checkbox working as those are the primary usage for form.

@bluebill1049
Copy link

FYI @oliviertassinari I am working on something to close the gap between MUI and react-hook-form. https://github.com/react-hook-form/react-hook-form-input

@raikusy
Copy link

raikusy commented Nov 23, 2019

Also facing issue with reset and loading default data. Please look into this issue. Or at least provide a work around for now.

@bluebill1049
Copy link

bluebill1049 commented Nov 23, 2019

Hey @raikusy please take a look the link which I have posted above, i think it will help you out. In the long run it would would be great for MUI component to support native form api ‘reset’ and update value via component ‘ref’, but this is a large price of work which I am sure MUI have a lot features and ideas lined up which are more important. Just as we discussed above until react hook form gets to be the mainstream lib or become popular then MUI will start investigating for solutions. Hope these make sense :) thanks for using react hook form too ❤️🤩🤟🏻

FYI that wrapper component will still enable ur form to have 0 re-render. Because input state update is isolated within the wrapper component.🤩🤟🏻💃

@raikusy
Copy link

raikusy commented Nov 23, 2019

Thank you @bluebill1049 this looks great. I have a condition where form fields will have default value when data is passed from props. So will the TextField component work with the above solution too? Also I was looking for any better example of passing default values with props to the form. The props might sometime come from a api call after the form has already been mounted.

@bluebill1049
Copy link

@raikusy you should be able to use reset/setValue API with the wrapper compoent.
reset: for entire form -> https://react-hook-form.com/api#setValue
setValue: for individual - > https://react-hook-form.com/api#reset

@hbarcelos
Copy link

I cannot get <Slider /> to work with RHF.

Using it like:

      <FormControl fullWidth component="fieldset" margin="normal">
        <FormLabel component="legend">Palavras</FormLabel>
        <RHFInput
          as={<Slider min={100} max={1200} step={100} valueLabelDisplay="auto" marks={marks} />}
          type="input"
          name="words"
          register={register}
          setValue={setValue}
        />
      </FormControl>

I get the following error:

Warning: Failed prop type: Invalid prop `value` supplied to `ForwardRef(Slider)`.
    in ForwardRef(Slider) (created by WithStyles(ForwardRef(Slider)))
    in WithStyles(ForwardRef(Slider)) (created by SetupAccountForm)
    in Unknown (created by SetupAccountForm)
    in fieldset (created by ForwardRef(FormControl))
    in ForwardRef(FormControl) (created by WithStyles(ForwardRef(FormControl)))
    in WithStyles(ForwardRef(FormControl)) (created by SetupAccountForm)
    in form (created by SetupAccountForm)
    in SetupAccountForm (created by SetupAccountPage)
    in div (created by ForwardRef(CardContent))
    in ForwardRef(CardContent) (created by WithStyles(ForwardRef(CardContent)))
    in WithStyles(ForwardRef(CardContent)) (created by SetupAccountPage)
    in div (created by ForwardRef(Paper))
    in ForwardRef(Paper) (created by WithStyles(ForwardRef(Paper)))
    in WithStyles(ForwardRef(Paper)) (created by ForwardRef(Card))
    in ForwardRef(Card) (created by WithStyles(ForwardRef(Card)))
    in WithStyles(ForwardRef(Card)) (created by SetupAccountPage)
    in div (created by ForwardRef(Grid))
    in ForwardRef(Grid) (created by WithStyles(ForwardRef(Grid)))
    in WithStyles(ForwardRef(Grid)) (created by Layout)
    in Layout (created by SetupAccountPage)
    in SetupAccountPage (created by App)
    in Route (created by App)
    in Switch (created by App)
    in Router (created by HashRouter)
    in HashRouter (created by App)
    in App (created by Root)
    in div (created by ForwardRef(Container))
    in ForwardRef(Container) (created by WithStyles(ForwardRef(Container)))
    in WithStyles(ForwardRef(Container)) (created by Root)
    in div (created by Styled(MuiBox))
    in Styled(MuiBox) (created by Root)
    in Provider (created by Root)
    in Root Button.js:233:15

@bluebill1049
Copy link

@hbarcelos looks like slider doesn't support value props :( I will do some investigation on it. in the meantime, you may have to use custom register, which is register at useEffect

@hbarcelos
Copy link

hbarcelos commented Nov 27, 2019

Thanks a lot @bluebill1049

I ended up making something like this:

import React, { useEffect, useCallback, useState, useMemo } from 'react';
import t from 'prop-types';
import clsx from 'clsx';
import { makeStyles } from '@material-ui/core/styles';
import Box from '@material-ui/core/Box';
import Slider from '@material-ui/core/Slider';

const useStyles = makeStyles(theme => ({
  sliderWrapper: {},
  sliderLabel: {
    marginTop: theme.spacing(1),
    marginBottom: theme.spacing(0.5),
    color: theme.palette.primary.main,
  },
}));

function CustomSlider({
  register,
  unregister,
  setValue,
  name,
  rules,
  defaultValue,
  valueLabelAs,
  formatLabel,
  ...rest
}) {
  const cl = useStyles();

  const [currentValue, setCurrentValue] = useState(defaultValue);

  useEffect(() => {
    register({ name });
    return () => unregister(name);
  }, [defaultValue, name, register, setValue, unregister]);

  const valueLabel = useMemo(() => {
    if (!valueLabelAs) {
      return null;
    }

    return React.cloneElement(
      valueLabelAs,
      { className: clsx(valueLabelAs.props.className, cl.sliderLabel) },
      formatLabel(currentValue)
    );
  }, [cl.sliderLabel, currentValue, formatLabel, valueLabelAs]);

  const handleChange = useCallback(
    (_, value) => {
      setValue(name, value);
      setCurrentValue(value);
    },
    [name, setValue]
  );

  return (
    <React.Fragment>
      {valueLabel}
      <Box className={cl.sliderWrapper}>
        <Slider {...rest} onChange={handleChange} defaultValue={defaultValue} />
      </Box>
    </React.Fragment>
  );
}

CustomSlider.defaultProps = {
  rules: {},
  defaultValue: '',
  valueLabelAs: null,
  formatLabel: v => v,
};

CustomSlider.propTypes = {
  register: t.func.isRequired,
  unregister: t.func.isRequired,
  setValue: t.func.isRequired,
  name: t.string.isRequired,
  rules: t.object,
  defaultValue: t.oneOfType([t.number, t.string]),
  valueLabelAs: t.node,
  formatLabel: t.func,
};

export default CustomSlider;

It works as expected.

@oliviertassinari oliviertassinari removed the waiting for 👍 Waiting for upvotes label Nov 30, 2019
@lednhatkhanh
Copy link

Much needed this also

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 7, 2019

In the long run it would would be great for MUI component to support native form api ‘reset’ and update value via component ‘ref’.

@bluebill1049 Isn't this an issue with React or react-hook-form itself? How is a developer supposed to "react" to an input state change with react-hook-form?
Let's say I'm a React developer building a custom input, and I want to style the element differently when it's filled/empty, how should I implement this? It seems that no change event is ever fired.

Have you considered firing a change event when your library changes an input value "outside" React's declarative model? Like it's done in @test-library/react?

This would enable the support of Material-UI components that rely on native input elements, for the others, like the Slider, a custom wrapper would still be needed. The slider has a value prop but the signature of onChange is different from a native input. I think that it would be great to see an integration library as we have for the other form libraries: #15585.


I was wondering why this issue got so many upvotes, I think that I have found out 🙃:

Capture d’écran 2019-12-07 à 20 41 31
https://react-hook-form.com/advanced-usage#ControlledmixedwithUncontrolledComponents

@bluebill1049
Copy link

bluebill1049 commented Dec 7, 2019

hey @oliviertassinari, thanks for looking closer to this issue again 👍

Let's say I'm a React developer building a custom input, and I want to style the element differently when it's filled/empty, how should I implement this? It seems that no change event is ever fired.

I would seek CSS selector for solutions in my own project which is using native inputs, eg: select an element has an empty value and display a style. An alternative I would use react-hook-form watch API to detect empty value and pass down as prop (with my wrapped input component).

Have you considered firing a change event when your library changes an input value "outside" React's declarative model? Like it's done in @test-library/react?

Yeah, we are pretty much doing it under react-hook-form-input. 😄

I believe in uncontrolled components for building forms after many years of building form controlled, it really makes things much easier. It may not solve every edge case (if you working at Facebook LOL) but it certainly makes my work life and others around me easier when it comes to building forms. HTML inputs are stateful themselves and I would love to embrace them in this lib (not saying it's right or wrong, but an alternative solution).

I love building React application so much, and that's why building a lot of packages around it. I understand React to embrace controlled components. However, react-hook-form is not blocking developers from building form controlled, because you can still do custom register at useEffect.

Conclusion:

I think we can close this issue and I will remove the link on my website related to this issue. On top of that, could I update this page to include react-hook-form-input?

https://material-ui.com/components/text-fields/#complementary-projects

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 7, 2019

Yeah, we are pretty much doing it under react-hook-form-input.

@bluebill1049 It sounds like the opposite approach to my suggestion. I understand the following:

  • react-hook-form:
    1. It keeps track of the input DOM nodes. When it needs to change the value, it does input.value = 'x'. This is problematic for React, as it doesn't have any way to know that the input value has changed. For instance, no change event is triggered.
    2. Because react-hook-form needs to listen for input value changes, it sets an "input" event listener on the input. This is problematic for your library as changes by React doesn't trigger any event when the value changes from a controlled input.
  • react-hook-form-input: the library controls the input to work around the two previous limitations of react-hook-form (i and ii).

Given that 1. it's very unlikely that React will play nicely to support react-hook-form approach and 2. that react-hook-form doesn't work outside of the box with controlled and pseudo-uncontrolled inputs (the ones that change the default value and the ones that only listen to onChange), I would propose that:

  1. react-hook-form patches the controlled and pseudo uncontrolled problem internally. Basically, you need to solve i. and ii. (see the proposed solutions in the linked codesandboxes: dispatch + defineProperty).
  2. Material-UI seeks assistance from the community to provide adapters for react-hook-form, when needed (related to Improve form library integration #15585).
  3. We close this issue 👌

@oliviertassinari oliviertassinari added the external dependency Blocked by external dependency, we can’t do anything about it label Dec 7, 2019
@bluebill1049
Copy link

bluebill1049 commented Dec 7, 2019

thanks, @oliviertassinari for the detailed reply. I will do some further investigation with your proposed solutions :)

I believe most of the issue you mentioned above with React is when you switch controlled components which is the part I want to improve on.

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 7, 2019

Just to be clear:

So I'm pretty sure you can solve that in react-hook-form. We would appreciate it if you can try/deploy these fixes and remove the blame on our side from your documentation 😛.

These changes should really help with the traction of your form library, I hope you will appreciate it :D.

@oliviertassinari
Copy link
Member

I believe most of the issue you mentioned above with React is when you switch from uncontrolled to controlled or controlled components which is the part I want to improve on.

React warns when the user switches between uncontrolled and controlled. I don't understand your point.

@bluebill1049
Copy link

bluebill1049 commented Dec 7, 2019

So I'm pretty sure you can solve that in react-hook-form. We would appreciate it if you can try/deploy these fixes and remove the blame on our side from your documentation 😛.

NO WAY I am blaming MUI haha, I love MUI (with my little star ⭐️). As you would know, react-hook-form is a new boy in the town, I was trying to get some attention or momentum on the particular issue. I apologize if you find that way (blame). again thanks very much for your help and investigation.

image
(Not Blaming)

@bluebill1049
Copy link

I believe most of the issue you mentioned above with React is when you switch from uncontrolled to controlled or controlled components which is the part I want to improve on.

React warns when the user switches between uncontrolled and controlled. I don't understand your point.

sorry I meant controlled component :) fixed my comment

@oliviertassinari
Copy link
Member

@bluebill1049 Awesome :)

@bluebill1049
Copy link

thanks a lot, @oliviertassinari (you are very kind)

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 7, 2019

Note that Material-UI could apply the same proposed fixes but I don't think that we should, it would have two drawbacks: 1. it would only work with Material-UI, the same effort would need to be done over and over. 2. the ownership of these "hacks" should stay in react-hook-form as it originates from the tradeoff the library took (bypassing the idiomatic React API).

@bluebill1049
Copy link

bluebill1049 commented Dec 20, 2019

Hey guys, in case someone steps into this issue. (We have been secretly working over this for a while 😓)

We are working on the next major version for RHF, which has some quality of life updates, especially around the controlled component. We will have better support for the UI library. The following code will resolve setValue and defaultValue for controlled UI library while still maintain minimum re-render at your App/Form level, re-render will be isolated at your Input component level.

The following will be the syntax when you are using V4 of RHF.

import TextField from '@material-ui/core/TextField';

const { control } = useForm();

<Controller as={TextField} control={control} name="firstName" rules={{ required: true }} />

@albert-medcheck
Copy link

albert-medcheck commented Feb 10, 2020

Need help.
I'm having an issue with material's textfield (probably other mui as well).
When the textfield is empty the first keystore is slow, same goes when you change the value to empty.
For some reason, it re-renders the whole form.

Screen Shot 2020-02-11 at 3 21 25 AM

ezgif com-video-to-gif

QUICK NOTE: I'm typing fast, without any delay. they delay part is due to the reason mentioned above.

@Luccasoli
Copy link

Luccasoli commented Apr 14, 2020

Hi guys, this problem persist even if i use the Controller from react-hook-form :(
Can this issue be reopened? Or am i doing it wrong?

Example: https://codesandbox.io/s/example-muitextfield-setvalue-with-react-hook-form-kqwq0?file=/index.js

Edit: A solution that i'm using is the InputLabelProps={isEdition && { shrink: isEdition }}
where isEdition is a flag that i'm using at edit screen.

@bluebill1049
Copy link

@Luccasoli plz refer to the doc: https://react-hook-form.com/api#Controller

There is an example with MUI examples: https://codesandbox.io/s/react-hook-form-controller-079xx

@Luccasoli
Copy link

Sorry @bluebill1049, but i dont found any similar situation in this example, and in the documentation I couldn't find any prop that could solve this problem properly :(

@bluebill1049
Copy link

Screen Shot 2020-04-15 at 9 21 56 am

@Luccasoli take a look at defaultValue

@Luccasoli
Copy link

I tried:

  • I tried turn the defaultValues inside useForm() a state value, and set that state inside useEffect()
  • i tried use the same idea, but in defaultValue prop directly each Controller

In both ways, the textField does not recognize that input is filled, and label keep over the input

@bluebill1049
Copy link

https://codesandbox.io/s/example-muitextfield-setvalue-with-react-hook-form-ijxfp

@Luccasoli
Copy link

Luccasoli commented Apr 14, 2020

Yes, but its strange a keep a defaultValue like that, i always try starting with a empty string value to not show a strange value to the user.

I could even display a "Loading" defaultValue, but the empty initial value still seems more interesting, do you think it is possible?

Edit: yeah, works with empty string too... hahaha, thank you, I swore I had tried this before, but it seems that.

@tripleTriangles
Copy link

@bluebill1049 I tweaked your demo to show the issue I'm seeing and hopefully you have a good solution.
https://codesandbox.io/s/react-hook-form-controller-n196b?file=/src/index.js
Trying to validate a checkbox is checked before enabling a button. All of the other fields I've used work as expected except for the checkbox.

@olddustysocksunderthecouch

I found this sand box really really useful for Material UI - There are examples for sliders, Select and a few others.
https://codesandbox.io/s/react-hook-form-controller-079xx?file=/src/index.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external dependency Blocked by external dependency, we can’t do anything about it new feature New feature or request
Projects
None yet
Development

No branches or pull requests