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

React Hook Form: Update to v 7.49.2 #79493

Merged
merged 21 commits into from Jan 5, 2024
Merged

React Hook Form: Update to v 7.49.2 #79493

merged 21 commits into from Jan 5, 2024

Conversation

Clarity-89
Copy link
Contributor

What is this feature?

Update the react-hook-form package to the latest version.

Why do we need this feature?

The version of RHF we have is a few years old. Updating to the new version has a few benefits:

  • We get to use the new methods (like resetField)
  • In the latest versions of RHF the types were significantly improved, providing better type safety, which allowed to catch a few bugs already.

Special notes for your reviewer:

I see that RHF is included in the renovate ignore list to avoid breaking the Form component API. I think we'd just deprecate the Form component from grafana/ui, which would remove the extra non ui-related dependency we need to maintain.

@Clarity-89 Clarity-89 added this to the 10.3.x milestone Dec 14, 2023
@Clarity-89 Clarity-89 self-assigned this Dec 14, 2023
@Clarity-89 Clarity-89 requested review from a team as code owners December 14, 2023 06:54
@Clarity-89 Clarity-89 requested review from kaydelaney and joshhunt and removed request for a team December 14, 2023 06:54
Copy link
Member

@gillesdemey gillesdemey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one change required, everything else LGTM from the Alerting side :)

public/app/features/alerting/unified/utils/amroutes.ts Outdated Show resolved Hide resolved
Copy link
Member

@gillesdemey gillesdemey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed the Alerting-related concern in f24bf2f – LGTM from our side

Copy link
Contributor

@juanicabanas juanicabanas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just a nit

@gelicia gelicia requested review from Elfo404 and removed request for VikaCep January 2, 2024 14:05
Copy link
Contributor

@ashharrison90 ashharrison90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from a code standpoint this lgtm 👍

haven't tested behaviour in alerting/explore etc - i'll leave that to the relevant affected teams

think it would be useful to confirm that this won't be a breaking change for e.g. a plugin using our form components

Copy link
Member

@Elfo404 Elfo404 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explore/Correlations looks good! 👍

@Clarity-89
Copy link
Contributor Author

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with rhf-update oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot
Copy link

@grafana-pr-automation grafana-pr-automation bot requested review from a team and academo and removed request for a team January 5, 2024 06:20
@Clarity-89 Clarity-89 merged commit 99f7110 into main Jan 5, 2024
18 checks passed
@Clarity-89 Clarity-89 deleted the rhf-update branch January 5, 2024 10:41
zserge pushed a commit that referenced this pull request Jan 22, 2024
* Update RHF to latest

* Update Form types

* Fix alerting types

* Fix correlations types

* Update tests

* Fix tests

* Update LabelsField.tsx to use InputControl

* Update RuleEditorGrafanaRules.test.tsx

* Update RuleEditorCloudRules.test.tsx

* Only require one label

* Update RuleEditorRecordingRule.test.tsx

* Fix labels rules

* Revert

* Remove RHF from ignore rules

* Revert

* update form validation for overriding group timings

* Fix changes to correlations

* Fix auth type errors

---------

Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>
Co-authored-by: Kristina Durivage <kristina.durivage@grafana.com>
@summerwollin summerwollin modified the milestones: 10.3.x, 10.3.0 Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants