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

Iss 139 upgrade hooks form #167

Closed

Conversation

karlrez
Copy link
Contributor

@karlrez karlrez commented Jul 13, 2022

Related Issue

Resolves #139

Description

  • Made code changes to update react-hooks-form from v6 to v7.

Impacted Areas in Application

Packages:

  • Updated react-hooks-form to latest version
  • Updated TypeScript to latest version (TS 4.3 or above was required)

Forms:

  • Updated syntax for register method
  • Updated syntax for error object

Steps to Test or Reproduce

  • Tested form submits correctly.
  • Tested error checking and error messages show correctly.

Related PRs

If you PR is related to other part of PR, list them here


PR Checklist:

  • My code follows the style guidelines of this project and I have double check my own code
  • I have made corresponding changes to the documentation, if any
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have run npm run test and be sure all test pass
  • I have run npm run build:dev and be sure no error blocks the build
  • I have test locally that everything run as expected
  • I have properly fill in the PR template
  • I have ask for reviews

@gsambrotta Were you hoping to have tests added for the Contact and Subscribe forms? I don't mind adding those.

@karlrez karlrez requested a review from a team as a code owner July 13, 2022 05:17
@karlrez karlrez requested review from qweqsbrako and removed request for a team July 13, 2022 05:17
@gsambrotta
Copy link
Collaborator

Thank you very much @karlrez!
I'm checking the PR in these days but looks great.

We are upgrading to react 18 and there seems to be an issue with enzyme.
We probably need to remove it.
Any chance you have some free time to help with migrating all the tests from enzyme to react test library?

1 similar comment
@gsambrotta
Copy link
Collaborator

Thank you very much @karlrez!
I'm checking the PR in these days but looks great.

We are upgrading to react 18 and there seems to be an issue with enzyme.
We probably need to remove it.
Any chance you have some free time to help with migrating all the tests from enzyme to react test library?

@karlrez
Copy link
Contributor Author

karlrez commented Jul 30, 2022

Hey @gsambrotta! For sure, I would be interested in migrating the tests. I do have a few prior commitments, so I might be about two weeks or so until I have a PR up. If you still want me to go ahead, can you let me know which test files you guys want migrated over?

@gsambrotta
Copy link
Collaborator

@karlrez that would be very very helpful! 🎉
I finally manage to fix all the dependencies to React 17.0.2 so we should be ok with waiting a couple of weeks before upgrading to react 18.

The tests are the ones that still use Enzyme.
It should be:

  • AllResultsView
  • Footer
  • SubscribeForm
  • ImagesView
  • ButtonPrimary
  • Header
  • Hero
  • Map
  • NewsView
  • SearchBar
  • Toast
  • Tooltip
  • app

Not all the test cases inside those files use Enzyme. Most of the time is just 1 or 2 test cases for each file.

I will write an Issue for this task

@karlrez
Copy link
Contributor Author

karlrez commented Jul 31, 2022

@gsambrotta sounds good! Feel free to assign me to the issue once it is up.

@gsambrotta gsambrotta removed the request for review from qweqsbrako August 1, 2022 13:43
@gsambrotta
Copy link
Collaborator

I close this in favor of pr #172 which is the same just with a merge conflict from the last push from develop branch.

@gsambrotta gsambrotta closed this Aug 1, 2022
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

Successfully merging this pull request may close these issues.

Upgrade react-hooks-form to v7
2 participants