-
Notifications
You must be signed in to change notification settings - Fork 7
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
Thomas/update conform #499
Conversation
8c0083f
to
721c58f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my culture I think I'm still interested in an oral explanation of the changes to the components/Form files - otherwise LGTM
"@vitest/coverage-v8": "^1.6.0", | ||
"@vitest/ui": "1.6.0", | ||
"prettier": "^3.3.2", | ||
"@vitest/coverage-v8": "^2.0.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to be sure and since those are major version changes, can you please double check the license type didn't change ?
"@conform-to/dom": "^0.9.1", | ||
"@conform-to/react": "^0.9.1", | ||
"@conform-to/zod": "^0.9.1", | ||
"@conform-to/dom": "^1.1.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the comment is not at the good position
const name = useFieldName(); | ||
const [meta] = useField<boolean>(name); | ||
|
||
const control = unstable_useControl(meta); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sanity check: what should I make of "unstable_" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing 🤷🏽
This is a common convention for pkg mainteners to tell the public API is unstable and may be changed with breaking changes in the future; still the hook is introduced for more than 4 months now, w\ any changes. This was created in order to fix issues with the initial attempt of handling form custom inputs useInputControl
.
More info here
721c58f
to
d7705a4
Compare
d7705a4
to
028c727
Compare
Finally did it, it was painfull but here we are !
In short: I updated to the last major of
conformr
(= the form lib I want to use in the app, that should replace entirelyreact-hook-form
in the end).NB: I tested all the forms to make sure no regressions are introduced + improved existing one (ex: when creating a scenario, error messages are displayed now).
Another UX improvment, when an error occure at first submit, each fields is validated "on change", so no more "dangling" error messages above fields (when the validation can be done frontend side, of course).
I honnestly doubt you can review it... reading some forms update should be a good way to catch the main changes :
FormProvider
to access the state in order to give progressive enhancementGlobally the same interface we already used (for
FormField,
FormInput`....)