Feat: adding text input, address input, select input, checkbox, number input

[mcgingras]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
ui-components	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 25, 2023 4:28pm

[gelicamarie]

I don’t think we should add react hook form here - this is a UI library and we should add a stateless input component

[mcgingras]

I don’t think we should add react hook form here - this is a UI library and we should add a stateless input component

I disagree -- it's our UI library so we can be opinionated about it if we want to. Tying react-hook-form to the form components lets the component accept validation + is controlled through the form library we are using. It's possible we could write generic input components, then wrap each component with some shell that you pass validation + react-hook-form props to, then we destructure props in each of our own UI components like {...props} to pass validation + controlled input state from react-hook-form. But, I also feel like its fine if we just take an opinionated stance about using react-hook-form to begin with.

I can give a stab at the generic form component that passes all of react-hook-form down through the input element but don't want to spend too much time on it. I'll see how it looks and push it here

[gelicamarie]

I don’t think we should add react hook form here - this is a UI library and we should add a stateless input component

I disagree -- it's our UI library so we can be opinionated about it if we want to. Tying react-hook-form to the form components lets the component accept validation + is controlled through the form library we are using. It's possible we could write generic input components, then wrap each component with some shell that you pass validation + react-hook-form props to, then we destructure props in each of our own UI components like {...props} to pass validation + controlled input state from react-hook-form. But, I also feel like its fine if we just take an opinionated stance about using react-hook-form to begin with.

I can give a stab at the generic form component that passes all of react-hook-form down through the input element but don't want to spend too much time on it. I'll see how it looks and push it here

UI Library is for presentational components - it is best practice to not have business logic within it. We can wrap the inputs we have here with react hook form in the membership rep. I know its a bit of extra work and making this opinionated may seem efficient right now but it does defeat the whole purpose of having a component library.

We should keep it separate for scalability and enforcing good practices for ui libraries.

[mcgingras]

We can wrap the inputs we have here with react hook form in the membership rep

this is quite tedious if you have to do this each time -- if we are okay with an opinionated ui library that takes the opinion that all forms should run through RHF, then you can put the RHF control logic in the component itself. It's not "business logic" -- its just using RHF behind the scenes to handle validation, controlling the input etc. It's still completely presentational.

does defeat the whole purpose of having a component library

IMO the whole purpose of a component library is so we can "drag and drop" components into membership product and all instances of that component will be uniformly styled. We can implement the component once, then be done with it, and any changes in the future will be implemented within the component then automatically updated anywhere we used that component. I agree its a good idea to keep components free of business logic, because introducing business logic means you have a component that only fits the domain of the business logic in question. However, I don't think using RHF qualifies as business logic in this case. It's just a tool for managing the form.

We should keep it separate for scalability and enforcing good practices for ui libraries.

I'd encourage you to argue your case with more substance as to what you mean by scalability and good practices. Scalability and good practices could apply to just about anything :) There are a few additional reasons I could see why you might want to omit RHF from the component (what if we want to use a different form library? If the input it free from RHF, we can swap the form library without affecting the input component itself) but I don't really buy this argument because the point of having an opinionated component library is that you are okay with such opinions like "we should be using RHF", and it seems like an premature optimization to be concerned about swapping out form libraries.

shadCN has a decent approach of keeping the input component itself free of RHF but still using RHF through a Form component that wraps each input. We could take this approach. I don't mind copying the rest of the shadCN components over into our repo and restyling them. I do think it comes at the cost of the form being a bit more verbose -- and out of the box the shadCN component does not offer a way to pass the error to the input itself, so we'd need to do a bit of modification to allow for that in order to style the input a certain way when it contains errors. I'm fine with this approach if you are more comfortable with it, but I stand by the original approach serving our needs as well.

[gelicamarie]

It's not "business logic"

It is business logic you are mixing presentation logic with form handling logic

IMO the whole purpose of a component library is so we can "drag and drop" components into membership product and all instances of that component will be uniformly styled.

It is still drag and drop — im just opposed to having RHF because it then forces us to always use RHF in the future. The primary goal of a component library is to promote reusability. By tying RHF directly into the membership component, you limit its reusability. What if you want to use the same component in another project that doesn't use RHF? It becomes less portable and flexible. And yes right now we are using RHF but we're assuming we'll always be using it.

From experience and conversing with other coworkers who have used RHF there’s always that point where we've ran into building more complex forms that RHF is just not the easiest to work with DX wise more notably with their Typing - and have ended up migrating away from it. I don't believe these reasons and trying to avoid it is "premature optimization".

But yes it is best practice to avoid forcing engineers into specific methods of hooking up components and managing state. I suggest exploring other projects for reference on how this can be achieved more effectively.
• https://spectrum.adobe.com/page/text-field
• https://www.radix-ui.com/docs/primitives/components/form
• https://degen-xyz.vercel.app/components/Field
• https://universal.showtime.xyz/?path=/story/components-fieldset--primary

Another scalability point is handling dependency management. Incorporating RHF directly into the component makes it more challenging to manage dependencies. I know its just one package right now but if we keep adding things to make it out "own opinionated library" that will just add more bloat to this package and increase the size of the bundle unreasonably.