-
Notifications
You must be signed in to change notification settings - Fork 41
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
feat: use setInitialState
for async initialization
#84
Conversation
Thanks, this makes a lot of sense! |
packages/bs-reform/src/ReFormNext.re
Outdated
@@ -127,6 +147,7 @@ module Make = (Config: Config) => { | |||
arrayRemoveBy: 'a. (Config.field(array('a)), 'a => bool) => unit, | |||
submit: unit => unit, | |||
resetForm: unit => unit, | |||
setInitialState: Config.state => unit, |
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 we can actually create a helper called setValues
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 that setValues means set all values passed in, and setInitialState means won't set the value when its fieldState is not Pristine, maybe we can call it setinitialValues
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.
that makes sense, I'll take a better look soon so we can get it merged
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.
hello @fakenickels would you have time to merge this
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.
Sorry about the delay, the past few days have we have been a bit busy. I'll gonna finish the review today and give you a better feedback soon
packages/bs-reform/src/ReFormNext.re
Outdated
@@ -144,6 +165,7 @@ module Make = (Config: Config) => { | |||
let (state, send) = | |||
ReactUpdate.useReducer( | |||
{ | |||
initValues: initialState, |
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.
With setValues I don't think this is necessary
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.
hmm looking the ResetForm branch changes I got it now
@amythos This looks good, but for the next releases we are going to try to make ReForm as similar as possible to Formik to ease adoption. So in Formik there is no setInitialValues, we just have a setValues. So before merging this PR we would like to have the handlers also named setValues and no |
packages/bs-reform/src/ReFormNext.re
Outdated
@@ -86,6 +86,24 @@ module Make = (Config: Config) => { | |||
->Belt.Array.get(0); | |||
}; | |||
|
|||
let setInitialValues = |
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.
let's call this setValues now
…ix Js.Re.test for Deprecated
@fakenickels now we have setValues for set the whole state, and setFieldValue for the single field |
Thank you for your time and effort @amythos , we'll be releasing this to NPM soon |
@all-contributors please add @amythos for code |
I've put up a pull request to add @amythos! 🎉 |
usage