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

Fix schema type error mentioned in issue 549 #563

Merged
merged 2 commits into from
Jun 12, 2023

Conversation

henrikvolmer
Copy link
Contributor

Fixes issue #549

@bluebill1049
Copy link
Member

Any chance you could add some test coverage on the fix? thank 🙏

@henrikvolmer
Copy link
Contributor Author

Do you have any suggestions, how to test a typescript type change? In my opinion, typescript will test it automatically on compile time.

@bluebill1049
Copy link
Member

If you can include usage in the test case would be good, that proof why the issue was there in the first place.

Improves yup type inference to make generic casting for useForm unnecessary
@henrikvolmer
Copy link
Contributor Author

I changed some more, to improve/fix the type inference for useForm. Now the generic in useForm is unnecessary because it'll be inferred from the schema.

@bluebill1049 bluebill1049 requested a review from jorisre May 30, 2023 22:06
@henrikvolmer
Copy link
Contributor Author

Any news here? Do you need something?

Copy link
Member

@jorisre jorisre left a comment

Choose a reason for hiding this comment

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

LGTM! thanks for the contribution

@jorisre jorisre merged commit 420e862 into react-hook-form:master Jun 12, 2023
3 checks passed
@github-actions
Copy link
Contributor

🎉 This PR is included in version 3.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@henrikvolmer
Copy link
Contributor Author

@jorisre @bluebill1049 : I think this should have been better at least a minor release, since the type was any before and is now specialized. This is probably why some users have a problem with their types. Because of any the type check was disabled before and therefore wrong types didn't cause an error.

@bluebill1049
Copy link
Member

Thanks @henrikvolmer for supporting on this issue. Yes, I think a minor version would probably be the wiser release.

@henrikvolmer
Copy link
Contributor Author

@bluebill1049 : So how we move on? Can you change the patch version to a minor version?

@bluebill1049
Copy link
Member

It's a little bit of a difficult situation here We can't overwrite the release or re-release. perhaps @jorisre maybe we just added to the release note on this change which may cause breaking changes to existing apps.

@jorisre
Copy link
Member

jorisre commented Jun 14, 2023

We can:

  • Update the release note
  • Re-release a minor version with a release note

@bluebill1049 What do you prefer?

@bluebill1049
Copy link
Member

I think we just update the release note would be the better choice, cause the patch is already release. thanks @jorisre 🙏

@jorisre
Copy link
Member

jorisre commented Jun 14, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants