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

General - Research libraries for redesigning our form architecture #45

Closed
jpwaves opened this issue Feb 21, 2022 · 13 comments
Closed

General - Research libraries for redesigning our form architecture #45

jpwaves opened this issue Feb 21, 2022 · 13 comments
Assignees
Labels
feature enhancement Improvements to an existing feature spike Research technical improvement Improvements a user might not notice

Comments

@jpwaves
Copy link

jpwaves commented Feb 21, 2022

Desired Changes

As of now, we currently use states and pass those down to the view components to get the data from our forms and then send them back up to our container component to do business logic with them/pass the data to our custom react hooks. We also manually do form validation, and our current validation is the bare minimum at best. We want to utilize existing libraries that provide the tools and architecture that would allow us to create flexible forms with proper validation and are easy to build. Some potential options we have right now are Formik and React Hook Forms.

Screenshots (as needed)

No response

@jpwaves jpwaves self-assigned this Feb 21, 2022
@jpwaves jpwaves added feature enhancement Improvements to an existing feature technical improvement Improvements a user might not notice labels Feb 21, 2022
@jamescd18 jamescd18 changed the title Spike - Research libraries for redesigning our form architecture General - Research libraries for redesigning our form architecture Feb 21, 2022
@jamescd18 jamescd18 added the spike Research label Feb 21, 2022
@jpwaves
Copy link
Author

jpwaves commented Feb 22, 2022

A few blogs I've read comparing Formik and React Hook Forms make it seem that React Hook Forms is the better option because it is more performant and has a smaller package size. However, the two are fairly similar, so it may come to deciding which seems easiest for us to implement.

Sources:
https://blog.logrocket.com/react-hook-form-vs-formik-comparison/
https://blog.bitsrc.io/react-hook-form-vs-formik-form-builder-library-for-react-23ed559fdae

@jamescd18
Copy link
Member

This is awesome! I want your opinion: which package do you think is more intuitive? Which do you think will provide the functionality we will later need?

I personally don't care a lot about bundle size. As long as we're avoiding bloated packages, we generally should be fine. Bundle size reduction can be handled later down the line. Usually differences in bundle size are negligible, especially at this stage of our application.

Things I do care about:

  • intuitive experience for developers
  • flexible design patterns to fit with what we've got
  • sufficiently powerful to fit our needs
  • actually going to help make devs' lives easier

Forms things I would consider:

  • integration with react-bootstrap form components (not actually super important, but a nice to have since it would keep the app looking consistent)
  • how does it handle form state?
  • can we disable submit button and show the user a loading spinner?
  • how does it handle validation of specific fields and showing error messages on only the offending fields?
  • can it integrate properly with our mutate async data sending design pattern?
  • can it integrate with our existing input payload schemas?
  • can it handle passing in default values for the form fields to do pre-filled forms in the future?

@jpwaves
Copy link
Author

jpwaves commented Feb 26, 2022

As far as validation goes, I was thinking of utilizing yup. I've seen examples of yup being integrated into both React Hook Forms and Formik, so with either option, I think we should consider yup to standardize and abstract away our form validation. I'll keep looking through React Hook Forms, but from what I've seen so far, React Hook Forms is fairly easy to use as it doesn't require us to use new components and there are a lot of usage examples we can reference from. Not sure how well it fits our mutate async pattern yet though, so I'll have to look into that more but right now I'm leaning towards using React Hook Forms.

@jpwaves
Copy link
Author

jpwaves commented Feb 26, 2022

I think before we create an epic for integrating any of these packages though, we should have one test ticket to try implementing the new package for conforming our current forms to the new package to see how feasible it is. I wouldn't mind taking that up if we get to that point.

@jpwaves
Copy link
Author

jpwaves commented Feb 27, 2022

Here are my findings for React Hook Forms so far:

  • We can integrate react-bootstrap components with the form using Controller's
  • States for the form are handled internally and are accessible for validation and submission. At least within the handleSubmit function, you can access the data of the form, which has key-pair values with the name attribute of the input as the key and the value of the associated input as the value. Validation of the data can happen in two stages: the initial enforcement of the data to specific types and constraints, and then the validation of the data that fits those types and constraints. The latter could be done within handleSubmit or have the data passed to a separate service that does the validation; the bottom line, though, is that we can easily integrate our current handleSubmit functions to match what they expect, with some minor tweaking.
  • Submit button can still be disabled and loading spinner can still show before form is rendered (we can also have the loading spinner occur on submission by utilizing the isSubmitting property from useFormState)
  • As I mentioned above, I think we should try to split our validation into two parts: first, enforcing our data to specific constraints, then validating that data to see if it maps to existing records in our database. The first part can be enforced with the help of the built-in validation features (or yup), the second part can take place within handleSubmit.
  • React Hook Forms allows for asynchronous submit callbacks, so it should be able to fit our mutateAsync form submission pattern
  • It should match our current input payload schemas since we get to define the handleSubmit callback ourselves and React Hook Forms simply calls that function when the form is submitted
  • Yes, it can handle adding in default values to the form since it doesn't require using custom/new components for the form

@jpwaves
Copy link
Author

jpwaves commented Mar 4, 2022

Here are my findings for Formik so far:

  • Formik can integrate with react-bootstrap components (there are even some examples on the react-bootstrap docs in the form validation section)
  • Form states are handled by Formik: they are defined in the initialValues prop on the Formik wrapper component as a hash where the key labels are the name property assigned to specific components within our actual form
  • Similar to React Hook Forms, there is a isSubmitting property that allows us to add spinners during submission (and for validation too with isValidating)
  • The validation process would be similar to what I recommended in the React Hook Forms findings where we do both field-level validation (validation on each individual input) and validation to enforce the inputs to conform to a specific schema (likely through the use of yup)
  • Async submissions are allowed, which should sync with our mututeAsync form submission pattern
  • Should be able to match our current input payload schemas since we define what the handleSubmit callback is
  • I believe the initialValues will set the default values of the form inputs, so yes there can be default values set and those will also be the default values of the states for the form as well.

Note: Formik has its own set of components for creating forms (you could use the hook version of Formik to eliminate the need to learn new components but there would be a lot of boilerplate code) that developers would need to learn, it doesn't look too complicated to understand, but it will take time. This is one of the major differences between React Hook Forms and Formik, and one of the reasons I'm currently leaning towards React Hook Forms.

@jpwaves
Copy link
Author

jpwaves commented Mar 4, 2022

My current standing on this is that we go with React Hook Forms and do a trial run by converting one of our forms to use React Hook Forms (with all the built out validation using yup and other necessary validating helper functions) to see how it feels and if it looks somewhat straightforward for other devs to convert our forms to use React Hook Forms. My main points for React Hook Forms are that devs don't need to learn how to use new components, just how to integrate React Hook Forms to be used in our current components (which with an example form shouldn't be too difficult) and that it seems to be used in other actual products as well (here I'm purely going off the fact that my co-op uses React Hook Forms over Formik) so it would be good to learn now before going on co-op. In both cases though, we would use yup to do schema validation on the form and then have separate validation (probably within the API calls/services we have that do our API calls to ensure our form inputs are actually valid and map to existing records in our database) and can continue with our mutateAsync pattern. I'd appreciate any feedback on any of this -- I feel like my opinions are slightly skewed because my co-op uses React Hook Forms over Formik and so I see that package in a brighter light.

NOTE: the integration for React Hook Forms with react-bootstrap isn't as easy to find/straightforward as Formik is, but I can look into finding those resources so devs can easily figure that out. It seems React Hook Forms is more set up to interact with MUI rather than React Bootstrap, but I think it may be possible to do the registering of the form inputs in the same way as with normal HTML elements since React Bootstrap components should just be HOC's or wrapper component of the normal HTML elements.

@jamescd18
Copy link
Member

This is amazing to see! Yes let’s definitely spin up a ticket for doing a trial run migration of a form to this new package. I want to say that this can also be a chance for us to trial having a feature branch for integrating this new package.

The one thing I’m still wondering is why yup when we have json schema and json-schema-to-ts? If possible, I’d like to avoid having two completely different design patterns for describing object validation stuff. I don’t have a clue what the details of the differences and implications are here, just posing the question. If you could figure out what the reasoning / justification here is that’d be amazing.

@jpwaves
Copy link
Author

jpwaves commented Mar 5, 2022

This is amazing to see! Yes let’s definitely spin up a ticket for doing a trial run migration of a form to this new package. I want to say that this can also be a chance for us to trial having a feature branch for integrating this new package.

The one thing I’m still wondering is why yup when we have json schema and json-schema-to-ts? If possible, I’d like to avoid having two completely different design patterns for describing object validation stuff. I don’t have a clue what the details of the differences and implications are here, just posing the question. If you could figure out what the reasoning / justification here is that’d be amazing.

From my understanding, yup will perform validation on the form inputs themselves before they are submitted. We already do this on some of our forms when we limit specific inputs to numbers or to a specific range of numbers -- yup will replace that and allow us to nicely organize all of those validations into a single schema which we can integrate into our forms. I also looked into the difference between yup and json-schema-to-ts and the former is a library more suited to run-time validation (which is what we need for form input validation) and the latter seems to be better suited for API input validation. Beyond this, it's also used in a number of other projects (and I've seen it used in the codebase for my co-op), so I feel this is a library worth getting exposure to for our devs, especially those who will be going on co-op soon. yup includes a lot more in-depth features for specific validation as well (regex, constraining the length of a string var, etc.) than what I could find when briefly looking through json-schema-to-ts, which I think would be incredibly useful (e.g. using regex to enforce the formatting for WBS numbers). I hope this blurb kinda clears up the difference between yup and json-schema-to-ts and why I think we should use it.

@jamescd18
Copy link
Member

former is a library more suited to run-time validation (which is what we need for form input validation) and the latter seems to be better suited for API input validation.

Thank you Justin! That clarifies it a ton! If you feel as though yup and json-schema-to-ts make most sense living together serving their unique purposes, then by all means let's go for that.

@jpwaves
Copy link
Author

jpwaves commented Mar 5, 2022

Note to self: Make a ticket to test converting one of our forms to use React Hook Forms

@jpwaves
Copy link
Author

jpwaves commented Mar 11, 2022

Note to self: close this issue only after a successful trial is done for converting one of our forms to whichever forms library we end up using.

@anthonybernardi anthonybernardi transferred this issue from Northeastern-Electric-Racing/PM-Dashboard-v2 Aug 11, 2022
@anthonybernardi
Copy link
Collaborator

it works so this ticket is done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature enhancement Improvements to an existing feature spike Research technical improvement Improvements a user might not notice
Projects
None yet
Development

No branches or pull requests

3 participants